Re: [PATCH] sched/nohz: Skip remote tick on idle task entirely

[Paul E. McKenney]

On Thu, Jun 28, 2018 at 06:29:41PM +0200, Frederic Weisbecker wrote:
> Some people have reported that the warning in sched_tick_remote()
> occasionally triggers, especially in favour of some RCU-Torture
> pressure:
> 
> 	WARNING: CPU: 11 PID: 906 at kernel/sched/core.c:3138 sched_tick_remote+0xb6/0xc0
> 	Modules linked in:
> 	CPU: 11 PID: 906 Comm: kworker/u32:3 Not tainted 4.18.0-rc2+ #1
> 	Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014
> 	Workqueue: events_unbound sched_tick_remote
> 	RIP: 0010:sched_tick_remote+0xb6/0xc0
> 	Code: e8 0f 06 b8 00 c6 03 00 fb eb 9d 8b 43 04 85 c0 75 8d 48 8b 83 e0 0a 00 00 48 85 c0 75 81 eb 88 48 89 df e8 bc fe ff ff eb aa <0f> 0b eb
> 	+c5 66 0f 1f 44 00 00 bf 17 00 00 00 e8 b6 2e fe ff 0f b6
> 	Call Trace:
> 	 process_one_work+0x1df/0x3b0
> 	 worker_thread+0x44/0x3d0
> 	 kthread+0xf3/0x130
> 	 ? set_worker_desc+0xb0/0xb0
> 	 ? kthread_create_worker_on_cpu+0x70/0x70
> 	 ret_from_fork+0x35/0x40
> 
> This happens when the remote tick applies on an idle task. Usually the
> idle_cpu() check avoids that, but it is performed before we lock the
> runqueue and it is therefore racy. It was intended to be that way in
> order to prevent from useless runqueue locks since idle task tick
> callback is a no-op.
> 
> Now if the racy check slips out of our hands and we end up remotely
> ticking an idle task, the empty task_tick_idle() is harmless. Still
> it won't pass the WARN_ON_ONCE() test that ensures rq_clock_task() is
> not too far from curr->se.exec_start because update_curr_idle() doesn't
> update the exec_start value like other scheduler policies. Hence the
> reported false positive.
> 
> So let's have another check, while the rq is locked, to make sure we
> don't remote tick on an idle task. The lockless idle_cpu() still applies
> to avoid unecessary rq lock contention.
> 
> Reported-by: Jacek Tomaka <jacekt@xxxxxxx>
> Reported-by: Paul E. McKenney <paulmck@xxxxxxxxxxxxxxxxxx>
> Reported-by: Anna-Maria Gleixner <anna-maria@xxxxxxxxxxxxx>
> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Frederic Weisbecker <frederic@xxxxxxxxxx>

Tested-by: Paul E. McKenney <paulmck@xxxxxxxxxxxxxxxxxx>

> ---
>  kernel/sched/core.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 78d8fac..da8f121 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -3127,16 +3127,18 @@ static void sched_tick_remote(struct work_struct *work)
>  		u64 delta;
> 
>  		rq_lock_irq(rq, &rf);
> -		update_rq_clock(rq);
>  		curr = rq->curr;
> -		delta = rq_clock_task(rq) - curr->se.exec_start;
> +		if (!is_idle_task(curr)) {
> +			update_rq_clock(rq);
> +			delta = rq_clock_task(rq) - curr->se.exec_start;
> 
> -		/*
> -		 * Make sure the next tick runs within a reasonable
> -		 * amount of time.
> -		 */
> -		WARN_ON_ONCE(delta > (u64)NSEC_PER_SEC * 3);
> -		curr->sched_class->task_tick(rq, curr, 0);
> +			/*
> +			 * Make sure the next tick runs within a reasonable
> +			 * amount of time.
> +			 */
> +			WARN_ON_ONCE(delta > (u64)NSEC_PER_SEC * 3);
> +			curr->sched_class->task_tick(rq, curr, 0);
> +		}
>  		rq_unlock_irq(rq, &rf);
>  	}
> 
> -- 
> 2.7.4
>