lkml.org 
[lkml]   [2012]   [Apr]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
Subject[PATCH RFC 0/7] rcu: v2 Inlinable preemptible rcu_read_lock() and rcu_read_unlock()
Hello!

This series is version two of the inlinable versions of preemptible
RCU's __rcu_read_lock() and __rcu_read_unlock(). The first version may
be found at https://lkml.org/lkml/2012/3/25/94. The individual commits
in this new series are as follows:

1. Move preemptible RCU's hook in the scheduler from the common
RCU scheduler-entry hook to just before the scheduler's call
to switch_to. This reduces overhead in the case where the
scheduler is called but does not switch and also sets the
stage for saving and restoring the per-CPU variables needed
for inlining.

2. Create the per-CPU variables and rename rcu_read_unlock_special()
to avoid name conflict.

3. Make exit_rcu() use a more precise method of checking the need
for exit-time RCU-related cleanup, and consolidate the two
identical versions of exit_rcu() into one place.

4. Make __rcu_read_lock() and __rcu_read_unlock() use the per-CPU
variables, but leave them out of line for the moment. This
requires adding a second preemptible-RCU hook in the scheduler
to restore the values of the per-CPU variables.

5. Silence bogus copy_to_user() build errors that seem to be triggered
by differences in gcc's inlining decisions when __rcu_read_lock()
becomes inlinable. Apparently, copy_to_user() needs to be inlined
in order to function correctly? Hmmm, sort of like kfree_rcu().

6. Inline __rcu_read_lock().

7. Inline __rcu_read_unlock().

With these changes, the 32-bit x86 gcc compiler compiles this:

void rcu_read_lock_code(void)
{
rcu_read_lock();
}

to this:

000000d0 <rcu_read_lock_code>:
d0: 64 ff 05 00 00 00 00 incl %fs:0x0
d7: c3 ret
d8: 90 nop
d9: 8d b4 26 00 00 00 00 lea 0x0(%esi,%eiz,1),%esi

It also compiles this:

void rcu_read_unlock_code(void)
{
rcu_read_unlock();
}

to this:

000000e0 <rcu_read_unlock_code>:
e0: 64 a1 00 00 00 00 mov %fs:0x0,%eax
e6: 83 f8 01 cmp $0x1,%eax
e9: 74 0d je f8 <rcu_read_unlock_code+0x18>
eb: 64 ff 0d 00 00 00 00 decl %fs:0x0
f2: c3 ret
f3: 90 nop
f4: 8d 74 26 00 lea 0x0(%esi,%eiz,1),%esi
f8: 64 c7 05 00 00 00 00 movl $0x80000000,%fs:0x0
ff: 00 00 00 80
103: 64 a1 00 00 00 00 mov %fs:0x0,%eax
109: 85 c0 test %eax,%eax
10b: 75 0c jne 119 <rcu_read_unlock_code+0x39>
10d: 64 c7 05 00 00 00 00 movl $0x0,%fs:0x0
114: 00 00 00 00
118: c3 ret
119: 8d b4 26 00 00 00 00 lea 0x0(%esi,%eiz,1),%esi
120: e8 fc ff ff ff call 121 <rcu_read_unlock_code+0x41>
125: eb e6 jmp 10d <rcu_read_unlock_code+0x2d>

It is therefore not at all clear to me that the final patch in this
series is worthwhile. Unless someone comes up with a good reason to
keep it, I will drop it. The only possible justification I can see is
that gcc could (in theory, anyway) drop dead code in the case of nested
RCU read-side critical sections (everything from address f3 onwards),
but this just doesn't cut it for me at the moment. I could also imagine
having the inlined portion contain only the nesting check and decrement,
along with a call to an out-of-line function that does the rest, but
this looks to me to bloat the code for no good reason.

Thoughts?

Thanx, Paul

arch/um/drivers/mconsole_kern.c | 3
b/arch/um/drivers/mconsole_kern.c | 1
b/fs/binfmt_misc.c | 4 -
b/include/linux/init_task.h | 4 -
b/include/linux/rcupdate.h | 1
b/include/linux/rcutiny.h | 6 -
b/include/linux/rcutree.h | 12 ---
b/include/linux/sched.h | 10 +++
b/kernel/rcu.h | 4 +
b/kernel/rcupdate.c | 5 +
b/kernel/rcutiny_plugin.h | 10 +--
b/kernel/rcutree.c | 1
b/kernel/rcutree.h | 1
b/kernel/rcutree_plugin.h | 14 ----
b/kernel/sched/core.c | 1
include/linux/rcupdate.h | 72 ++++++++++++++++++++-
include/linux/rcutiny.h | 5 -
include/linux/sched.h | 92 +++++++++++++++++++++++++--
kernel/rcu.h | 4 -
kernel/rcupdate.c | 126 ++++++++++++++++++++++----------------
kernel/rcutiny_plugin.h | 123 +++++++------------------------------
kernel/rcutree_plugin.h | 114 ++++++++--------------------------
kernel/sched/core.c | 3
23 files changed, 321 insertions(+), 295 deletions(-)



\
 
 \ /
  Last update: 2012-04-14 18:23    [W:0.068 / U:0.104 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site