diff mbox series

sched/fair: use RETRY_TASK in idle_balance

Message ID 1524739022-7788-1-git-send-email-vincent.guittot@linaro.org
State New
Headers show
Series sched/fair: use RETRY_TASK in idle_balance | expand

Commit Message

Vincent Guittot April 26, 2018, 10:37 a.m. UTC 
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(-)

-- 
2.7.4

Comments

Peter Zijlstra April 26, 2018, 11:49 a.m. UTC | #1 
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.
Vincent Guittot April 26, 2018, 1:11 p.m. UTC | #2 
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
kernel test robot April 27, 2018, 10:49 p.m. UTC | #3 
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
diff mbox series

Patch

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;