linux-kernel - Re: [RFC][PATCH 4/7] smp: Optimize send_call_function_single

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]

Date:   Fri, 22 Jan 2021 09:31:37 +0100
From:   Peter Zijlstra <peterz@...radead.org>
To:     "Paul E. McKenney" <paulmck@...nel.org>
Cc:     tglx@...utronix.de, frederic@...nel.org,
        linux-kernel@...r.kernel.org, x86@...nel.org, cai@....pw,
        mgorman@...hsingularity.net, joel@...lfernandes.org,
        valentin.schneider@....com
Subject: Re: [RFC][PATCH 4/7] smp: Optimize send_call_function_single_ipi()

On Thu, Jan 21, 2021 at 04:20:12PM -0800, Paul E. McKenney wrote:

> > ---
> > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> > index 368749008ae8..2c8d4c3e341e 100644
> > --- a/kernel/rcu/tree.c
> > +++ b/kernel/rcu/tree.c
> > @@ -445,7 +445,7 @@ static int rcu_is_cpu_rrupt_from_idle(void)
> >  	/*
> >  	 * Usually called from the tick; but also used from smp_function_call()
> >  	 * for expedited grace periods. This latter can result in running from
> > -	 * the idle task, instead of an actual IPI.
> > +	 * a (usually the idle) task, instead of an actual IPI.
> 
> The story is growing enough hair that we should tell it only once.
> So here just where it is called from:
> 
> 	/*
> 	 * Usually called from the tick; but also used from smp_function_call()
> 	 * for expedited grace periods.
> 	 */
> 
> >  	lockdep_assert_irqs_disabled();
> >  
> > @@ -461,9 +461,14 @@ static int rcu_is_cpu_rrupt_from_idle(void)
> >  		return false;
> >  
> >  	/*
> > -	 * If we're not in an interrupt, we must be in the idle task!
> > +	 * If we're not in an interrupt, we must be in task context.
> > +	 *
> > +	 * This will typically be the idle task through:
> > +	 *   flush_smp_call_function_from_idle(),
> > +	 *
> > +	 * but can also be in CPU HotPlug through smpcfd_dying().
> >  	 */
> 
> Good, but how about like this?
> 
> 	/*
> 	 * If we are not in an interrupt handler, we must be in
> 	 * smp_call_function() handler.
> 	 *
> 	 * Normally, smp_call_function() handlers are invoked from
> 	 * the idle task via flush_smp_call_function_from_idle().
> 	 * However, they can also be invoked from CPU hotplug
> 	 * operations via smpcfd_dying().
> 	 */
> 
> > -	WARN_ON_ONCE(!nesting && !is_idle_task(current));
> > +	WARN_ON_ONCE(!nesting && !in_task(current));
> 
> This is used in time-critical contexts, so why not RCU_LOCKDEP_WARN()?
> That should also allow checking more closely.  Would something like the
> following work?
> 
> 	RCU_LOCKDEP_WARN(!nesting && !is_idle_task(current) && (!in_task(current) || !lockdep_cpus_write_held()));
> 
> Where lockdep_cpus_write_held is defined in kernel/cpu.c:

Works for me, except s/in_task(current)/in_task()/ compiles a lot
better.