[PATCH] sched/fair: use RETRY_TASK in idle

LKML Archive on lore.kernel.org
help / color / mirror / Atom feed

* [PATCH] sched/fair: use RETRY_TASK in idle_balance
@ 2018-04-26 10:37 Vincent Guittot
  2018-04-26 11:49 ` Peter Zijlstra
  2018-04-27 22:49 ` kbuild test robot
  0 siblings, 2 replies; 4+ messages in thread
From: Vincent Guittot @ 2018-04-26 10:37 UTC (permalink / raw)
  To: peterz, mingo, linux-kernel; +Cc: Vincent Guittot

Use RETRY_TASK in idle_balance when we want select_task_rq() function to 
rerun the complete task selection path

Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
---
 kernel/sched/fair.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 0951d1c..ba49f83 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -9862,7 +9862,7 @@ static int idle_balance(struct rq *this_rq, struct rq_flags *rf)
 
 	/* Is there a task of a high priority class? */
 	if (this_rq->nr_running != this_rq->cfs.h_nr_running)
-		pulled_task = -1;
+		pulled_task = RETRY_TASK;
 
 	if (pulled_task)
 		this_rq->idle_stamp = 0;
-- 
2.7.4

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] sched/fair: use RETRY_TASK in idle_balance
  2018-04-26 10:37 [PATCH] sched/fair: use RETRY_TASK in idle_balance Vincent Guittot
@ 2018-04-26 11:49 ` Peter Zijlstra
  2018-04-26 13:11   ` Vincent Guittot
  2018-04-27 22:49 ` kbuild test robot
  1 sibling, 1 reply; 4+ messages in thread
From: Peter Zijlstra @ 2018-04-26 11:49 UTC (permalink / raw)
  To: Vincent Guittot; +Cc: mingo, linux-kernel

On Thu, Apr 26, 2018 at 12:37:02PM +0200, Vincent Guittot wrote:
> Use RETRY_TASK in idle_balance when we want select_task_rq() function to 
> rerun the complete task selection path
> 
> Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
> ---
>  kernel/sched/fair.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 0951d1c..ba49f83 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -9862,7 +9862,7 @@ static int idle_balance(struct rq *this_rq, struct rq_flags *rf)
>  
>  	/* Is there a task of a high priority class? */
>  	if (this_rq->nr_running != this_rq->cfs.h_nr_running)
> -		pulled_task = -1;
> +		pulled_task = RETRY_TASK;

This doesn't make sense, pulled_task is an integer, not a task pointer.
Also see how the caller in select_task_rq_fair() converts a negative
into RETRY_TASK.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] sched/fair: use RETRY_TASK in idle_balance
  2018-04-26 11:49 ` Peter Zijlstra
@ 2018-04-26 13:11   ` Vincent Guittot
  0 siblings, 0 replies; 4+ messages in thread
From: Vincent Guittot @ 2018-04-26 13:11 UTC (permalink / raw)
  To: Peter Zijlstra; +Cc: Ingo Molnar, linux-kernel

On 26 April 2018 at 13:49, Peter Zijlstra <peterz@infradead.org> wrote:
> On Thu, Apr 26, 2018 at 12:37:02PM +0200, Vincent Guittot wrote:
>> Use RETRY_TASK in idle_balance when we want select_task_rq() function to
>> rerun the complete task selection path
>>
>> Signed-off-by: Vincent Guittot <vincent.guittot@linaro.org>
>> ---
>>  kernel/sched/fair.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>> index 0951d1c..ba49f83 100644
>> --- a/kernel/sched/fair.c
>> +++ b/kernel/sched/fair.c
>> @@ -9862,7 +9862,7 @@ static int idle_balance(struct rq *this_rq, struct rq_flags *rf)
>>
>>       /* Is there a task of a high priority class? */
>>       if (this_rq->nr_running != this_rq->cfs.h_nr_running)
>> -             pulled_task = -1;
>> +             pulled_task = RETRY_TASK;
>
> This doesn't make sense, pulled_task is an integer, not a task pointer.
> Also see how the caller in select_task_rq_fair() converts a negative
> into RETRY_TASK.

My goal was to try to make it a bit more readable about the meaning of
-1 but it fails with the type returned by idle_balance

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] sched/fair: use RETRY_TASK in idle_balance
  2018-04-26 10:37 [PATCH] sched/fair: use RETRY_TASK in idle_balance Vincent Guittot
  2018-04-26 11:49 ` Peter Zijlstra
@ 2018-04-27 22:49 ` kbuild test robot
  1 sibling, 0 replies; 4+ messages in thread
From: kbuild test robot @ 2018-04-27 22:49 UTC (permalink / raw)
  To: Vincent Guittot; +Cc: kbuild-all, peterz, mingo, linux-kernel, Vincent Guittot

[-- Attachment #1: Type: text/plain, Size: 11217 bytes --]

Hi Vincent,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on tip/sched/core]
[also build test WARNING on v4.17-rc2 next-20180426]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Vincent-Guittot/sched-fair-use-RETRY_TASK-in-idle_balance/20180428-050824
config: x86_64-randconfig-x012-04271426 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   kernel/sched/fair.c: In function 'idle_balance':
>> kernel/sched/fair.c:9865:15: warning: assignment makes integer from pointer without a cast [-Wint-conversion]
      pulled_task = RETRY_TASK;
                  ^
   Cyclomatic Complexity 5 include/linux/compiler.h:__read_once_size
   Cyclomatic Complexity 5 include/linux/compiler.h:__write_once_size
   Cyclomatic Complexity 2 arch/x86/include/asm/bitops.h:clear_bit
   Cyclomatic Complexity 1 arch/x86/include/asm/bitops.h:constant_test_bit
   Cyclomatic Complexity 1 arch/x86/include/asm/bitops.h:variable_test_bit
   Cyclomatic Complexity 1 arch/x86/include/asm/bitops.h:fls
   Cyclomatic Complexity 1 arch/x86/include/asm/arch_hweight.h:__arch_hweight64
   Cyclomatic Complexity 2 include/linux/bitops.h:hweight_long
   Cyclomatic Complexity 1 include/linux/log2.h:__ilog2_u32
   Cyclomatic Complexity 1 arch/x86/include/asm/current.h:get_current
   Cyclomatic Complexity 1 include/linux/list.h:INIT_LIST_HEAD
   Cyclomatic Complexity 1 include/linux/list.h:__list_add_valid
   Cyclomatic Complexity 1 include/linux/list.h:__list_del_entry_valid
   Cyclomatic Complexity 2 include/linux/list.h:__list_add
   Cyclomatic Complexity 1 include/linux/list.h:list_add
   Cyclomatic Complexity 1 include/linux/list.h:__list_del
   Cyclomatic Complexity 2 include/linux/list.h:__list_del_entry
   Cyclomatic Complexity 1 include/linux/list.h:list_del_init
   Cyclomatic Complexity 1 include/linux/list.h:list_move
   Cyclomatic Complexity 1 include/linux/list.h:list_empty
   Cyclomatic Complexity 1 arch/x86/include/asm/atomic.h:atomic_read
   Cyclomatic Complexity 1 arch/x86/include/asm/atomic64_64.h:atomic64_add
   Cyclomatic Complexity 1 include/linux/jump_label.h:static_key_count
   Cyclomatic Complexity 3 include/linux/string.h:memset
   Cyclomatic Complexity 3 include/linux/bitmap.h:bitmap_and
   Cyclomatic Complexity 3 include/linux/bitmap.h:bitmap_intersects
   Cyclomatic Complexity 3 include/linux/bitmap.h:bitmap_subset
   Cyclomatic Complexity 3 include/linux/bitmap.h:bitmap_weight
   Cyclomatic Complexity 1 include/linux/cpumask.h:cpumask_check
   Cyclomatic Complexity 1 include/linux/cpumask.h:cpumask_first
   Cyclomatic Complexity 1 include/linux/cpumask.h:cpumask_clear_cpu
   Cyclomatic Complexity 1 include/linux/cpumask.h:cpumask_test_cpu
   Cyclomatic Complexity 1 include/linux/cpumask.h:cpumask_and
   Cyclomatic Complexity 1 include/linux/cpumask.h:cpumask_intersects
   Cyclomatic Complexity 1 include/linux/cpumask.h:cpumask_subset
   Cyclomatic Complexity 1 include/linux/cpumask.h:cpumask_weight
   Cyclomatic Complexity 1 arch/x86/include/asm/paravirt.h:arch_local_irq_restore
   Cyclomatic Complexity 1 arch/x86/include/asm/paravirt.h:arch_local_irq_enable
   Cyclomatic Complexity 1 include/linux/math64.h:div_u64_rem
   Cyclomatic Complexity 1 include/linux/math64.h:div_u64
   Cyclomatic Complexity 1 include/linux/math64.h:mul_u64_u32_shr
   Cyclomatic Complexity 2 include/linux/thread_info.h:test_ti_thread_flag
   Cyclomatic Complexity 1 include/linux/lockdep.h:lock_is_held
   Cyclomatic Complexity 1 include/linux/rcupdate.h:rcu_lock_acquire
   Cyclomatic Complexity 1 include/linux/rcupdate.h:rcu_lock_release
   Cyclomatic Complexity 1 include/linux/rcupdate.h:rcu_read_lock
   Cyclomatic Complexity 1 include/linux/rcupdate.h:rcu_read_unlock
   Cyclomatic Complexity 1 include/linux/spinlock.h:spin_trylock
   Cyclomatic Complexity 1 include/linux/spinlock.h:spin_unlock
   Cyclomatic Complexity 1 include/linux/jiffies.h:_msecs_to_jiffies
   Cyclomatic Complexity 3 include/linux/jiffies.h:msecs_to_jiffies
   Cyclomatic Complexity 1 include/linux/rbtree.h:rb_link_node
   Cyclomatic Complexity 1 include/linux/sched.h:task_thread_info
   Cyclomatic Complexity 1 include/linux/sched.h:test_tsk_thread_flag
   Cyclomatic Complexity 1 include/linux/sched.h:test_tsk_need_resched
   Cyclomatic Complexity 1 include/linux/sched.h:task_cpu
   Cyclomatic Complexity 1 include/linux/sched/clock.h:local_clock
   Cyclomatic Complexity 1 include/linux/sched/cputime.h:get_running_cputimer
   Cyclomatic Complexity 2 include/linux/sched/cputime.h:account_group_exec_runtime
   Cyclomatic Complexity 1 include/linux/tick.h:tick_nohz_tick_stopped
   Cyclomatic Complexity 1 include/linux/sched/topology.h:sched_domain_span
   Cyclomatic Complexity 1 include/linux/cgroup.h:task_css_set
   Cyclomatic Complexity 1 include/linux/cgroup.h:task_dfl_cgroup
   Cyclomatic Complexity 3 include/linux/cgroup.h:cgroup_parent
   Cyclomatic Complexity 1 include/linux/cgroup.h:cpuacct_charge
   Cyclomatic Complexity 2 include/linux/cgroup.h:cgroup_account_cputime
   Cyclomatic Complexity 1 kernel/sched/sched.h:sched_asym_prefer
   Cyclomatic Complexity 1 kernel/sched/sched.h:cpu_of
   Cyclomatic Complexity 1 kernel/sched/sched.h:__rq_clock_broken
   Cyclomatic Complexity 4 kernel/sched/sched.h:assert_clock_updated
   Cyclomatic Complexity 4 kernel/sched/sched.h:rq_clock
   Cyclomatic Complexity 4 kernel/sched/sched.h:rq_clock_task
   Cyclomatic Complexity 5 kernel/sched/sched.h:rq_clock_skip_update
   Cyclomatic Complexity 1 kernel/sched/sched.h:rq_pin_lock
   Cyclomatic Complexity 2 kernel/sched/sched.h:rq_unpin_lock
   Cyclomatic Complexity 1 kernel/sched/sched.h:rq_repin_lock
   Cyclomatic Complexity 1 kernel/sched/sched.h:sched_group_span
   Cyclomatic Complexity 1 kernel/sched/sched.h:group_balance_mask
   Cyclomatic Complexity 1 kernel/sched/sched.h:task_group
   Cyclomatic Complexity 1 kernel/sched/sched.h:task_running
   Cyclomatic Complexity 1 kernel/sched/sched.h:task_on_rq_queued
   Cyclomatic Complexity 1 kernel/sched/sched.h:put_prev_task
   Cyclomatic Complexity 4 kernel/sched/sched.h:idle_get_state
   Cyclomatic Complexity 1 kernel/sched/sched.h:sched_update_tick_dependency
   Cyclomatic Complexity 4 kernel/sched/sched.h:add_nr_running
   Cyclomatic Complexity 1 kernel/sched/sched.h:sub_nr_running
   Cyclomatic Complexity 1 kernel/sched/sched.h:sched_avg_period
   Cyclomatic Complexity 1 kernel/sched/sched.h:hrtick_enabled
   Cyclomatic Complexity 1 kernel/sched/sched.h:arch_scale_freq_capacity
   Cyclomatic Complexity 4 kernel/sched/sched.h:arch_scale_cpu_capacity
   Cyclomatic Complexity 1 kernel/sched/sched.h:rq_lock_irqsave
   Cyclomatic Complexity 1 kernel/sched/sched.h:rq_lock_irq
   Cyclomatic Complexity 1 kernel/sched/sched.h:rq_lock
   Cyclomatic Complexity 1 kernel/sched/sched.h:rq_unlock_irqrestore
   Cyclomatic Complexity 1 kernel/sched/sched.h:rq_unlock
   Cyclomatic Complexity 1 kernel/sched/sched.h:cpufreq_update_util
   Cyclomatic Complexity 1 include/trace/events/sched.h:trace_sched_stat_runtime
   Cyclomatic Complexity 1 kernel/sched/fair.c:update_load_add

vim +9865 kernel/sched/fair.c

  9757	
  9758	/*
  9759	 * idle_balance is called by schedule() if this_cpu is about to become
  9760	 * idle. Attempts to pull tasks from other CPUs.
  9761	 */
  9762	static int idle_balance(struct rq *this_rq, struct rq_flags *rf)
  9763	{
  9764		unsigned long next_balance = jiffies + HZ;
  9765		int this_cpu = this_rq->cpu;
  9766		struct sched_domain *sd;
  9767		int pulled_task = 0;
  9768		u64 curr_cost = 0;
  9769	
  9770		/*
  9771		 * We must set idle_stamp _before_ calling idle_balance(), such that we
  9772		 * measure the duration of idle_balance() as idle time.
  9773		 */
  9774		this_rq->idle_stamp = rq_clock(this_rq);
  9775	
  9776		/*
  9777		 * Do not pull tasks towards !active CPUs...
  9778		 */
  9779		if (!cpu_active(this_cpu))
  9780			return 0;
  9781	
  9782		/*
  9783		 * This is OK, because current is on_cpu, which avoids it being picked
  9784		 * for load-balance and preemption/IRQs are still disabled avoiding
  9785		 * further scheduler activity on it and we're being very careful to
  9786		 * re-start the picking loop.
  9787		 */
  9788		rq_unpin_lock(this_rq, rf);
  9789	
  9790		if (this_rq->avg_idle < sysctl_sched_migration_cost ||
  9791		    !this_rq->rd->overload) {
  9792	
  9793			rcu_read_lock();
  9794			sd = rcu_dereference_check_sched_domain(this_rq->sd);
  9795			if (sd)
  9796				update_next_balance(sd, &next_balance);
  9797			rcu_read_unlock();
  9798	
  9799			nohz_newidle_balance(this_rq);
  9800	
  9801			goto out;
  9802		}
  9803	
  9804		raw_spin_unlock(&this_rq->lock);
  9805	
  9806		update_blocked_averages(this_cpu);
  9807		rcu_read_lock();
  9808		for_each_domain(this_cpu, sd) {
  9809			int continue_balancing = 1;
  9810			u64 t0, domain_cost;
  9811	
  9812			if (!(sd->flags & SD_LOAD_BALANCE))
  9813				continue;
  9814	
  9815			if (this_rq->avg_idle < curr_cost + sd->max_newidle_lb_cost) {
  9816				update_next_balance(sd, &next_balance);
  9817				break;
  9818			}
  9819	
  9820			if (sd->flags & SD_BALANCE_NEWIDLE) {
  9821				t0 = sched_clock_cpu(this_cpu);
  9822	
  9823				pulled_task = load_balance(this_cpu, this_rq,
  9824							   sd, CPU_NEWLY_IDLE,
  9825							   &continue_balancing);
  9826	
  9827				domain_cost = sched_clock_cpu(this_cpu) - t0;
  9828				if (domain_cost > sd->max_newidle_lb_cost)
  9829					sd->max_newidle_lb_cost = domain_cost;
  9830	
  9831				curr_cost += domain_cost;
  9832			}
  9833	
  9834			update_next_balance(sd, &next_balance);
  9835	
  9836			/*
  9837			 * Stop searching for tasks to pull if there are
  9838			 * now runnable tasks on this rq.
  9839			 */
  9840			if (pulled_task || this_rq->nr_running > 0)
  9841				break;
  9842		}
  9843		rcu_read_unlock();
  9844	
  9845		raw_spin_lock(&this_rq->lock);
  9846	
  9847		if (curr_cost > this_rq->max_idle_balance_cost)
  9848			this_rq->max_idle_balance_cost = curr_cost;
  9849	
  9850		/*
  9851		 * While browsing the domains, we released the rq lock, a task could
  9852		 * have been enqueued in the meantime. Since we're not going idle,
  9853		 * pretend we pulled a task.
  9854		 */
  9855		if (this_rq->cfs.h_nr_running && !pulled_task)
  9856			pulled_task = 1;
  9857	
  9858	out:
  9859		/* Move the next balance forward */
  9860		if (time_after(this_rq->next_balance, next_balance))
  9861			this_rq->next_balance = next_balance;
  9862	
  9863		/* Is there a task of a high priority class? */
  9864		if (this_rq->nr_running != this_rq->cfs.h_nr_running)
> 9865			pulled_task = RETRY_TASK;
  9866	
  9867		if (pulled_task)
  9868			this_rq->idle_stamp = 0;
  9869	
  9870		rq_repin_lock(this_rq, rf);
  9871	
  9872		return pulled_task;
  9873	}
  9874	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 22105 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2018-04-27 22:49 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-26 10:37 [PATCH] sched/fair: use RETRY_TASK in idle_balance Vincent Guittot
2018-04-26 11:49 ` Peter Zijlstra
2018-04-26 13:11   ` Vincent Guittot
2018-04-27 22:49 ` kbuild test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).