Thread (20 messages) 20 messages, 3 authors, 2012-05-07

Re: linux-next ppc64: RCU mods cause __might_sleep BUGs

From: Hugh Dickins <hughd@google.com>
Date: 2012-05-03 00:30:32
Also in: lkml

On Wed, 2 May 2012, Paul E. McKenney wrote:
On Wed, May 02, 2012 at 03:54:24PM -0700, Hugh Dickins wrote:
quoted
On Wed, May 2, 2012 at 2:54 PM, Paul E. McKenney
[off-list ref] wrote:
quoted
On Thu, May 03, 2012 at 07:20:15AM +1000, Benjamin Herrenschmidt wrote:
quoted
On Wed, 2012-05-02 at 13:25 -0700, Hugh Dickins wrote:
quoted
Got it at last.  Embarrassingly obvious.  __rcu_read_lock() and
__rcu_read_unlock() are not safe to be using __this_cpu operations,
the cpu may change in between the rmw's read and write: they should
be using this_cpu operations (or, I put preempt_disable/enable in the
__rcu_read_unlock below).  __this_cpus there work out fine on x86,
which was given good instructions to use; but not so well on PowerPC.

I've been running successfully for an hour now with the patch below;
but I expect you'll want to consider the tradeoffs, and may choose a
different solution.
Didn't Linus recently rant about these __this_cpu vs this_cpu nonsense ?

I thought that was going out..
Linus did rant about __raw_get_cpu_var() because it cannot use the x86
%fs segement overrides a bit more than a month ago.  The __this_cpu
stuff is useful if you have preemption disabled -- avoids the extra
layer of preempt_disable().

Or was this a different rant?
https://lkml.org/lkml/2011/11/29/321

I think it ended up with Christoph removing the more egregious
variants, but this_cpu_that and __this_cpu_the_other remaining.
Ah, thank you for the pointer.

It would be nice to have the CPU transparency of x86 on other
architectures, but from what I can see, that would require dedicating
a register to this purpose -- and even then requires that the arch
have indexed addressing modes.  There are some other approaches, for
example, having __this_cpu_that() be located at a special address that
the scheduler treated as implicitly preempt_disable().  Or I suppose
that the arch-specific trap-handling code could fake it.  A little
bit messy, but the ability to access a given CPU's per-CPU variable
while running on that CPU does appear to have at least a couple of
uses -- inlining RCU and also making preempt_disable() use per-CPU
variables.

In any case, I must confess that I feel quite silly about my series
of patches.  I have reverted them aside from a couple that did useful
optimizations, and they should show up in -next shortly.
A wee bit sad, but thank you - it was an experiment worth trying,
and perhaps there will be reason to come back to it future.

Hugh
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help