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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180730164247.GN2494@hirez.programming.kicks-ass.net>
Date:   Mon, 30 Jul 2018 18:42:47 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
Cc:     linux-kernel@...r.kernel.org
Subject: Re: [PATCH RFC tip/core/rcu] Avoid resched_cpu() when rescheduling
 the current CPU

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.