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: <34cbf3f5-9485-a82d-2548-272e87033ab2@redhat.com>
Date:   Fri, 27 Aug 2021 16:27:00 -0400
From:   Waiman Long <llong@...hat.com>
To:     paulmck@...nel.org
Cc:     Josh Triplett <josh@...htriplett.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
        Lai Jiangshan <jiangshanlai@...il.com>,
        Joel Fernandes <joel@...lfernandes.org>, rcu@...r.kernel.org,
        linux-kernel@...r.kernel.org, alexei.starovoitov@...il.com,
        andrii@...nel.org
Subject: Re: [PATCH] rcu: Avoid unneeded function call in rcu_read_unlock()

On 8/27/21 2:34 PM, Paul E. McKenney wrote:
> On Thu, Aug 26, 2021 at 10:21:22PM -0400, Waiman Long wrote:
>> Since commit aa40c138cc8f3 ("rcu: Report QS for outermost
>> PREEMPT=n rcu_read_unlock() for strict GPs"). A real function call
>> rcu_read_unlock_strict() is added to the inlined rcu_read_unlock().
>> The rcu_read_unlock_strict() call is only needed if the performance
>> sagging CONFIG_RCU_STRICT_GRACE_PERIOD option is set. This config
>> option isn't set for most production kernels while the function call
>> overhead remains.
>>
>> To provide a slight performance improvement, the
>> CONFIG_RCU_STRICT_GRACE_PERIOD config check is moved from
>> rcu_read_unlock_strict() to __rcu_read_unlock() so that the function
>> call can be compiled out in most cases.
>>
>> Besides, the GPL exported rcu_read_unlock_strict() also impact the
>> the compilation of non-GPL kernel modules as rcu_read_unlock() is a
>> frequently used kernel API.
>>
>> Signed-off-by: Waiman Long <longman@...hat.com>
> Nice, and good eyes!!!
>
> I have queued this for v5.16, that is, not the upcoming merge window
> but the one after that.
>
> I did my usual wordsmithing, so please check the following in case I
> messed something up.  I intentionally omitted the EXPORT_SYMBOL_GPL()
> discussion because:
>
> 1.	Kernels built with CONFIG_PREEMPT=y have the same issue
> 	with the __rcu_read_lock() and __rcu_read_unlock() functions.
>
> 2.	Many other RCU functions are EXPORT_SYMBOL_GPL() and have
> 	been for almost two decades.
>
> But if someone does use RCU readers within CONFIG_PREEMPT=n kernels from
> a binary module, I will happily refer them to you for any RCU issues
> that they encounter.  ;-)
>
> I am also CCing the BPF guys in case my interpretation of the code in
> the BPF verifier is incorrect.
>
> 							Thanx, Paul
>
It looks good to me. Thanks for the rewording. I did regret mentioning 
about about the GPL export symbol in the commit log and it is good that 
you had taken it out.

Cheers,
Longman

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ