Re: [PATCH RFC tip/core/rcu] Avoid resched_cpu() when rescheduling the current CPU

[Peter Zijlstra]

On Mon, Jul 30, 2018 at 07:59:33AM -0700, Paul E. McKenney wrote:

> > Something that might be more in line with
> > resched_curr(smp_processor_id()) would be:
> > 
> > 	preempt_disable();
> > 	if (!test_tsk_need_resched(current)) {
> > 		set_tsk_need_resched(current);
> > 		set_preempt_need_resched();
> > 	}
> > 	preempt_enable();
> > 
> > Where the preempt_enable() could of course instantly trigger the
> > reschedule if it was the outer most one.
> 
> Ah.  So should I use resched_curr() from rcu_check_callbacks(), which
> is invoked from the scheduling-clock interrupt?  Right now I have calls
> to set_tsk_need_resched() and set_preempt_need_resched().
> 
> > > @@ -2674,10 +2675,12 @@ static __latent_entropy void rcu_process_callbacks(struct softirq_action *unused
> > 
> > > -		resched_cpu(rdp->cpu); /* Provoke future context switch. */
> > 
> > > +		set_tsk_need_resched(current);
> > > +		set_preempt_need_resched();
> > 
> > That's not obviously correct. rdp->cpu had better be smp_processor_id().
> 
> At the beginning of the function, we have:
> 
> 	struct rcu_data *rdp = raw_cpu_ptr(&rcu_data);
> 
> And this is in a softirq handler, so we are OK.

Agreed.

> > > @@ -672,7 +672,8 @@ static void sync_rcu_exp_handler(void *unused)
> > >  			rcu_report_exp_rdp(rdp);
> > >  		} else {
> > >  			rdp->deferred_qs = true;
> > > -			resched_cpu(rdp->cpu);
> > > +			set_tsk_need_resched(t);
> > > +			set_preempt_need_resched();
> > 
> > That only works if @t == current.
> 
> At the beginning of the function, we have:
> 
> 	struct task_struct *t = current;
> 
> So we should be OK.

Ah, the scheduler and locking code typically use to call that curr, to
be more explicit that it is the current task.


> Should I be instead using resched_curr() on some or all of these?

If, as it seems is the case, they are all targeting the current cpu and
have (soft) interrupts disabled, then what you propose is indeed fine.