rcu: updates for RCU lockdep
Subsystems (2)
PATCH 1/1 rcu: updates for RCU lockdep
Hello!
These four patches provides a pair of new APIs and documentation to handle a couple of cases that David Howells came across when running an NFS workload under CONFIG_PROVE_RCU, along with some comment improvements suggested by David and also some by Yong Zhang.
Thanx, Paul
b/Documentation/RCU/NMI-RCU.txt | 39 +++++++++++++++++++++----------------- b/Documentation/RCU/checklist.txt | 7 +++--- b/Documentation/RCU/lockdep.txt | 28 +++++++++++++++++++++++++-- b/Documentation/RCU/whatisRCU.txt | 6 +++++ b/include/linux/rcupdate.h | 33 ++++++++++++++++++++++++++++++++ include/linux/rcupdate.h | 30 +++++++++++++++++++++++------ 6 files changed, 115 insertions(+), 28 deletions(-)
PATCH 1/4 rcu: add rcu_access_pointer and rcu_dereference_protected
This patch adds variants of rcu_dereference() that handle situations where the RCU-protected data structure cannot change, perhaps due to our holding the update-side lock, or where the RCU-protected pointer is only to be fetched, not dereferenced. These are needed due to some performance concerns with using rcu_dereference() where it is not required, aside from the need for lockdep/sparse checking.
The new rcu_access_pointer() primitive is for the case where the pointer is be fetch and not dereferenced. This primitive may be used without protection, RCU or otherwise, due to the fact that it uses ACCESS_ONCE().
The new rcu_dereference_protected() primitive is for the case where updates are prevented, for example, due to holding the update-side lock. This primitive does neither ACCESS_ONCE() nor smp_read_barrier_depends(), so can only be used when updates are somehow prevented.
--- include/linux/rcupdate.h | 33 +++++++++++++++++++++++++++++++++ 1 files changed, 33 insertions(+), 0 deletions(-)
include/linux/rcupdate.h +33 -0
@@ -209,9 +209,42 @@ static inline int rcu_read_lock_sched_held(void)rcu_dereference_raw (p); \ }) +/** +* rcu_access_pointer - fetch RCU pointer with no dereferencing +* +* Return the value of the specified RCU-protected pointer, but omit the +* smp_read_barrier_depends() and keep the ACCESS_ONCE(). This is useful +* when the value of this pointer is accessed, but the pointer is not +* dereferenced, for example, when testing an RCU-protected pointer against +* NULL. This may also be used in cases where update-side locks prevent +* the value of the pointer from changing, but rcu_dereference_protected() +* is a lighter-weight primitive for this use case. +*/ +#define rcu_access_pointer (p) ((void *)ACCESS_ONCE(p)) + +/** +* rcu_dereference_protected - fetch RCU pointer when updates prevented +* +* Return the value of the specified RCU-protected pointer, but omit +* both the smp_read_barrier_depends() and the ACCESS_ONCE(). This +* is useful in cases where update-side locks prevent the value of the +* pointer from changing. Please note that this primitive does -not- +* prevent the compiler from repeating this reference or combining it +* with other references, so it should not be used without protection +* of appropriate locks. +*/ +#define rcu_dereference_protected (p, c) \ + ({ \ + if (debug_lockdep_rcu_enabled() && !(c)) \ + lockdep_rcu_dereference(__FILE__, __LINE__); \ + (p); \ + }) +#else /* #ifdef CONFIG_PROVE_RCU */ #define rcu_dereference_check (p, c) rcu_dereference_raw(p) +#define rcu_access_pointer (p) ((void *)ACCESS_ONCE(p)) +#define rcu_dereference_protected (p, c) (p)#endif /* #else #ifdef CONFIG_PROVE_RCU */
Signature · 1 line
1.7.0
PATCH 4/4 rcu: fix syntax error in rcu_dereference_check() example in comment
In reply to Paul E. McKenneyNeed a logical "or" rather than a comma.
--- include/linux/rcupdate.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
include/linux/rcupdate.h +1 -1
@@ -217,7 +217,7 @@ static inline int rcu_read_lock_sched_held(void)* target struct: * * bar = rcu_dereference_check(foo->bar, rcu_read_lock_held() || -* lockdep_is_held(&foo->lock), +* lockdep_is_held(&foo->lock) || * atomic_read(&foo->usage) == 0); */ #define rcu_dereference_check (p, c) \
Signature · 1 line
1.7.0
PATCH 3/4 rcu: Update docs for rcu_access_pointer and rcu_dereference_protected
In reply to Paul E. McKenneyUpdate examples and lists of APIs to include these new primitives.
--- Documentation/RCU/NMI-RCU.txt | 39 ++++++++++++++++++++++----------------- Documentation/RCU/checklist.txt | 7 ++++--- Documentation/RCU/lockdep.txt | 28 ++++++++++++++++++++++++++-- Documentation/RCU/whatisRCU.txt | 6 ++++++ 4 files changed, 58 insertions(+), 22 deletions(-)
Documentation/RCU/NMI-RCU.txt +22 -17
@@ -34,7 +34,7 @@ NMI handler. cpu = smp_processor_id(); ++nmi_count(cpu); - if (!rcu_dereference(nmi_callback)(regs, cpu)) + if (!rcu_dereference_sched(nmi_callback)(regs, cpu)) default_do_nmi(regs); nmi_exit();
@@ -47,12 +47,13 @@ function pointer. If this handler returns zero, do_nmi() invokes the default_do_nmi() function to handle a machine-specific NMI. Finally, preemption is restored. -Strictly speaking, rcu_dereference() is not needed, since this code runs -only on i386, which does not need rcu_dereference() anyway. However, -it is a good documentation aid, particularly for anyone attempting to -do something similar on Alpha. +In theory, rcu_dereference_sched() is not needed, since this code runs +only on i386, which in theory does not need rcu_dereference_sched() +anyway. However, in practice it is a good documentation aid, particularly +for anyone attempting to do something similar on Alpha or on systems +with aggressive optimizing compilers. -Quick Quiz: Why might the rcu_dereference() be necessary on Alpha, +Quick Quiz: Why might the rcu_dereference_sched() be necessary on Alpha, given that the code referenced by the pointer is read-only?
@@ -99,17 +100,21 @@ invoke irq_enter() and irq_exit() on NMI entry and exit, respectively. Answer to Quick Quiz - Why might the rcu_dereference() be necessary on Alpha, given + Why might the rcu_dereference_sched() be necessary on Alpha, given that the code referenced by the pointer is read-only? Answer: The caller to set_nmi_callback() might well have - initialized some data that is to be used by the - new NMI handler. In this case, the rcu_dereference() - would be needed, because otherwise a CPU that received - an NMI just after the new handler was set might see - the pointer to the new NMI handler, but the old - pre-initialized version of the handler's data. - - More important, the rcu_dereference() makes it clear - to someone reading the code that the pointer is being - protected by RCU. + initialized some data that is to be used by the new NMI + handler. In this case, the rcu_dereference_sched() would + be needed, because otherwise a CPU that received an NMI + just after the new handler was set might see the pointer + to the new NMI handler, but the old pre-initialized + version of the handler's data. + + This same sad story can happen on other CPUs when using + a compiler with aggressive pointer-value speculation + optimizations. + + More important, the rcu_dereference_sched() makes it + clear to someone reading the code that the pointer is + being protected by RCU-sched.
Documentation/RCU/checklist.txt +4 -3
@@ -260,7 +260,8 @@ over a rather long period of time, but improvements are always welcome! The reason that it is permissible to use RCU list-traversal primitives when the update-side lock is held is that doing so can be quite helpful in reducing code bloat when common code is - shared between readers and updaters. + shared between readers and updaters. Additional primitives + are provided for this case, as discussed in lockdep.txt. 10. Conversely, if you are in an RCU read-side critical section, and you don't hold the appropriate update-side lock, you -must-
@@ -344,8 +345,8 @@ over a rather long period of time, but improvements are always welcome! requiring SRCU's read-side deadlock immunity or low read-side realtime latency. - Note that, rcu_assign_pointer() and rcu_dereference() relate to - SRCU just as they do to other forms of RCU. + Note that, rcu_assign_pointer() relates to SRCU just as they do + to other forms of RCU. 15. The whole point of call_rcu(), synchronize_rcu(), and friends is to wait until all pre-existing readers have finished before
Documentation/RCU/lockdep.txt +26 -2
@@ -32,9 +32,20 @@ checking of rcu_dereference() primitives: srcu_dereference(p, sp): Check for SRCU read-side critical section. rcu_dereference_check(p, c): - Use explicit check expression "c". + Use explicit check expression "c". This is useful in + code that is invoked by both readers and updaters. rcu_dereference_raw(p) Don't check. (Use sparingly, if at all.) + rcu_dereference_protected(p, c): + Use explicit check expression "c", and omit all barriers + and compiler constraints. This is useful when the data + structure cannot change, for example, in code that is + invoked only by updaters. + rcu_access_pointer(p): + Return the value of the pointer and omit all barriers, + but retain the compiler constraints that prevent duplicating + or coalescsing. This is useful when when testing the + value of the pointer itself, for example, against NULL. The rcu_dereference_check() check expression can be any boolean expression, but would normally include one of the rcu_read_lock_held()
@@ -59,7 +70,20 @@ In case (1), the pointer is picked up in an RCU-safe manner for vanilla RCU read-side critical sections, in case (2) the ->file_lock prevents any change from taking place, and finally, in case (3) the current task is the only task accessing the file_struct, again preventing any change -from taking place. +from taking place. If the above statement was invoked only from updater +code, it could instead be written as follows: + + file = rcu_dereference_protected(fdt->fd[fd], + lockdep_is_held(&files->file_lock) || + atomic_read(&files->count) == 1); + +This would verify cases #2 and #3 above, and furthermore lockdep would +complain if this was used in an RCU read-side critical section unless one +of these two cases held. Because rcu_dereference_protected() omits all +barriers and compiler constraints, it generates better code than do the +other flavors of rcu_dereference(). On the other hand, it is illegal +to use rcu_dereference_protected() if either the RCU-protected pointer +or the RCU-protected data that it points to can change concurrently. There are currently only "universal" versions of the rcu_assign_pointer() and RCU list-/tree-traversal primitives, which do not (yet) check for
Documentation/RCU/whatisRCU.txt +6 -0
@@ -840,6 +840,12 @@ SRCU: Initialization/cleanup init_srcu_struct cleanup_srcu_struct +All: lockdep-checked RCU-protected pointer access + + rcu_dereference_check + rcu_dereference_protected + rcu_access_pointer + See the comment headers in the source code (or the docbook generated from them) for more information.
Signature · 1 line
1.7.0
PATCH 2/4 RCU: Better explain the condition parameter of rcu_dereference_check()
In reply to Paul E. McKenneyFrom: David Howells <[email protected]>
Better explain the condition parameter of rcu_dereference_check() that describes the conditions under which the dereference is permitted to take place. This condition is only checked under lockdep proving.
--- include/linux/rcupdate.h | 28 +++++++++++++++++++++++----- 1 files changed, 23 insertions(+), 5 deletions(-)
include/linux/rcupdate.h +23 -5
@@ -195,12 +195,30 @@ static inline int rcu_read_lock_sched_held(void)/** * rcu_dereference_check - rcu_dereference with debug checking +* @p: The pointer to read, prior to dereferencing +* @c: The conditions under which the dereference will take place * -* Do an rcu_dereference(), but check that the context is correct. -* For example, rcu_dereference_check(gp, rcu_read_lock_held()) to -* ensure that the rcu_dereference_check() executes within an RCU -* read-side critical section. It is also possible to check for -* locks being held, for example, by using lockdep_is_held(). +* Do an rcu_dereference(), but check that the conditions under which the +* dereference will take place are correct. Typically the conditions indicate +* the various locking conditions that should be held at that point. The check +* should return true if the conditions are satisfied. +* +* For example: +* +* bar = rcu_dereference_check(foo->bar, rcu_read_lock_held() || +* lockdep_is_held(&foo->lock)); +* +* could be used to indicate to lockdep that foo->bar may only be dereferenced +* if either the RCU read lock is held, or that the lock required to replace +* the bar struct at foo->bar is held. +* +* Note that the list of conditions may also include indications of when a lock +* need not be held, for example during initialisation or destruction of the +* target struct: +* +* bar = rcu_dereference_check(foo->bar, rcu_read_lock_held() || +* lockdep_is_held(&foo->lock), +* atomic_read(&foo->usage) == 0); */ #define rcu_dereference_check (p, c) \ ({ \
Signature · 1 line
1.7.0
For whole patchset:
Quoted 18 lines · Paul E. McKenney
Hello!
These four patches provides a pair of new APIs and documentation to handle a couple of cases that David Howells came across when running an NFS workload under
CONFIG_PROVE_RCU, along with some comment improvements suggested by David and also some by Yong Zhang.Thanx, Paul
b/Documentation/RCU/NMI-RCU.txt | 39 +++++++++++++++++++++----------------- b/Documentation/RCU/checklist.txt | 7 +++--- b/Documentation/RCU/lockdep.txt | 28 +++++++++++++++++++++++++-- b/Documentation/RCU/whatisRCU.txt | 6 +++++ b/include/linux/rcupdate.h | 33 ++++++++++++++++++++++++++++++++ include/linux/rcupdate.h | 30 +++++++++++++++++++++++------ 6 files changed, 115 insertions(+), 28 deletions(-)
Re: [PATCH tip/core/urgent 1/4] rcu: add rcu_access_pointer and rcu_dereference_protected
In reply to Paul E. McKenneyinclude/linux/rcupdate.h static inline int rcu_read_lock_sched_held(void) · line 246
Quoted 3 lines · Paul E. McKenney
include/linux/rcupdate.h +2
-0+#define rcu_access_pointer (p) ((void *)ACCESS_ONCE(p)) +#define rcu_access_pointer (p) ((void *)ACCESS_ONCE(p))
There's no difference between your two versions of rcu_access_pointer(), so you could move that whole construct outside of the #ifdef'ed section.
Other than that:
Re: [PATCH tip/core/urgent 2/4] RCU: Better explain the condition parameter of rcu_dereference_check()
In reply to Paul E. McKenneyQuoted 3 lines · Paul E. McKenney
+ * bar = rcu_dereference_check(foo->bar,
rcu_read_lock_held()||
+ *lockdep_is_held(&foo->lock),
+ *atomic_read(&foo->usage)== 0);
As pointed out by Yong Zhang, the comma after lockdep_is_held() should be a ||.
David
Re: [PATCH tip/core/urgent 3/4] rcu: Update docs for rcu_access_pointer and rcu_dereference_protected
In reply to Paul E. McKenneyQuoted 3 lines · Paul E. McKenney
Update examples and lists of APIs to include these new primitives.
Signed-off-by Paul E. McKenney <[email protected]>
Re: [PATCH tip/core/urgent 4/4] rcu: fix syntax error in rcu_dereference_check() example in comment
In reply to Paul E. McKenneyQuoted 4 lines · Paul E. McKenney
Need a logical "or" rather than a comma.
Suggested-by Yong Zhang <[email protected]>Signed-off-by Paul E. McKenney <[email protected]>
I'm surprised you didn't just roll this in to patch 2.
Eric Dumazet
Acked-byRe: [PATCH tip/core/urgent 1/4] rcu: add rcu_access_pointer and rcu_dereference_protected
In reply to David HowellsQuoted 9 lines · David Howells
include/linux/rcupdate.h static inline int rcu_read_lock_sched_held(void) · line 246
Quoted 3 lines · Paul E. McKenney
include/linux/rcupdate.h +2
-0+#define rcu_access_pointer (p) ((void *)ACCESS_ONCE(p)) +#define rcu_access_pointer (p) ((void *)ACCESS_ONCE(p))There's no difference between your two versions of
rcu_access_pointer(), so you could move that whole construct outside of the #ifdef'ed section.
Quoted 3 lines
Other than that:
Acked-by David Howells <[email protected]>
I also prefer to keep type checking if possible
#define rcu_access_pointer(p) (ACCESS_ONCE(p))
Other than that:
Re: [PATCH tip/core/urgent 1/4] rcu: add rcu_access_pointer and rcu_dereference_protected
In reply to David HowellsQuoted 9 lines · David Howells
include/linux/rcupdate.h static inline int rcu_read_lock_sched_held(void) · line 246
Quoted 3 lines · Paul E. McKenney
include/linux/rcupdate.h +2
-0+#define rcu_access_pointer (p) ((void *)ACCESS_ONCE(p)) +#define rcu_access_pointer (p) ((void *)ACCESS_ONCE(p))There's no difference between your two versions of
rcu_access_pointer(), so you could move that whole construct outside of the #ifdef'ed section.
Quoted 3 lines
Other than that:
Acked-by David Howells <[email protected]>
Good point, will rework and resubmit.
Thanx, Paul
Re: [PATCH tip/core/urgent 1/4] rcu: add rcu_access_pointer and rcu_dereference_protected
In reply to Eric DumazetQuoted 10 lines · Eric Dumazet
Quoted 9 lines · David Howells
include/linux/rcupdate.h static inline int rcu_read_lock_sched_held(void) · line 246
Quoted 3 lines · Paul E. McKenney
include/linux/rcupdate.h +2
-0+#define rcu_access_pointer (p) ((void *)ACCESS_ONCE(p)) +#define rcu_access_pointer (p) ((void *)ACCESS_ONCE(p))There's no difference between your two versions of
rcu_access_pointer(), so you could move that whole construct outside of the #ifdef'ed section.
Quoted 7 lines
Quoted 3 lines
Other than that:
Acked-by David Howells <[email protected]>I also prefer to keep type checking if possible
#define
rcu_access_pointer(p)(ACCESS_ONCE(p))
Yikes! I was obsessing on preventing people from dereferencing the pointer, but you are quite right, they might be comparing it to a constant pointer to check for array bounds or for use of a statically allocated emergency-pool item.
I will fix and resubmit.
Quoted 3 lines
Other than that:
Acked-by Eric Dumazet <[email protected]>
Thanx, Paul
Re: [PATCH tip/core/urgent 4/4] rcu: fix syntax error in rcu_dereference_check() example in comment
In reply to David HowellsQuoted 8 lines · David Howells
Quoted 4 lines · Paul E. McKenney
Need a logical "or" rather than a comma.
Suggested-by Yong Zhang <[email protected]>Signed-off-by Paul E. McKenney <[email protected]>I'm surprised you didn't just roll this in to patch 2.
I will do so when I resubmit. I am finally growing more comfortable with "git rebase -i", but still some ways to go. It has been a long journey from the old punched-card cabinets of my youth. ;-)
Thanx, Paul
Quoted 6 lines
Acked-by David Howells <[email protected]>--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
PATCH 1/1 v2 rcu: updates for RCU lockdep
In reply to Paul E. McKenneyHello!
These four patches provides a pair of new APIs and documentation to handle a couple of cases that David Howells came across when running an NFS workload under CONFIG_PROVE_RCU, along with some comment improvements suggested by David and also some by Yong Zhang.
Updated from v1 (http://lkml.org/lkml/2010/4/9/28) to reflect David's and Eric Dumazet's comments:
o Remove (void*) cast from rcu_access_pointer().
o Merge identical definitions of rcu_access_pointer() outside
of #ifdef.
o Merge comment syntax-error fix into comment patch.
Thanx, Paul
b/Documentation/RCU/NMI-RCU.txt | 39 +++++++++++++++++++++----------------- b/Documentation/RCU/checklist.txt | 7 +++--- b/Documentation/RCU/lockdep.txt | 28 +++++++++++++++++++++++++-- b/Documentation/RCU/whatisRCU.txt | 6 +++++ b/include/linux/rcupdate.h | 32 +++++++++++++++++++++++++++++++ include/linux/rcupdate.h | 28 ++++++++++++++++++++++----- 6 files changed, 113 insertions(+), 27 deletions(-)
PATCH 1/3 rcu: add rcu_access_pointer and rcu_dereference_protected
This patch adds variants of rcu_dereference() that handle situations where the RCU-protected data structure cannot change, perhaps due to our holding the update-side lock, or where the RCU-protected pointer is only to be fetched, not dereferenced. These are needed due to some performance concerns with using rcu_dereference() where it is not required, aside from the need for lockdep/sparse checking.
The new rcu_access_pointer() primitive is for the case where the pointer is be fetch and not dereferenced. This primitive may be used without protection, RCU or otherwise, due to the fact that it uses ACCESS_ONCE().
The new rcu_dereference_protected() primitive is for the case where updates are prevented, for example, due to holding the update-side lock. This primitive does neither ACCESS_ONCE() nor smp_read_barrier_depends(), so can only be used when updates are somehow prevented.
--- include/linux/rcupdate.h | 32 ++++++++++++++++++++++++++++++++ 1 files changed, 32 insertions(+), 0 deletions(-)
include/linux/rcupdate.h +32 -0
@@ -209,13 +209,45 @@ static inline int rcu_read_lock_sched_held(void)rcu_dereference_raw (p); \ }) +/** +* rcu_dereference_protected - fetch RCU pointer when updates prevented +* +* Return the value of the specified RCU-protected pointer, but omit +* both the smp_read_barrier_depends() and the ACCESS_ONCE(). This +* is useful in cases where update-side locks prevent the value of the +* pointer from changing. Please note that this primitive does -not- +* prevent the compiler from repeating this reference or combining it +* with other references, so it should not be used without protection +* of appropriate locks. +*/ +#define rcu_dereference_protected (p, c) \ + ({ \ + if (debug_lockdep_rcu_enabled() && !(c)) \ + lockdep_rcu_dereference(__FILE__, __LINE__); \ + (p); \ + }) +#else /* #ifdef CONFIG_PROVE_RCU */ #define rcu_dereference_check (p, c) rcu_dereference_raw(p) +#define rcu_dereference_protected (p, c) (p)#endif /* #else #ifdef CONFIG_PROVE_RCU */ /** +* rcu_access_pointer - fetch RCU pointer with no dereferencing +* +* Return the value of the specified RCU-protected pointer, but omit the +* smp_read_barrier_depends() and keep the ACCESS_ONCE(). This is useful +* when the value of this pointer is accessed, but the pointer is not +* dereferenced, for example, when testing an RCU-protected pointer against +* NULL. This may also be used in cases where update-side locks prevent +* the value of the pointer from changing, but rcu_dereference_protected() +* is a lighter-weight primitive for this use case. +*/ +#define rcu_access_pointer (p) ACCESS_ONCE(p) + +/** * rcu_read_lock - mark the beginning of an RCU read-side critical section. * * When synchronize_rcu() is invoked on one CPU while other CPUs
Signature · 1 line
1.7.0
PATCH 2/3 RCU: Better explain the condition parameter of rcu_dereference_check()
In reply to Paul E. McKenneyFrom: David Howells <[email protected]>
Better explain the condition parameter of rcu_dereference_check() that describes the conditions under which the dereference is permitted to take place (and incorporate Yong Zhang's suggestion). This condition is only checked under lockdep proving.
--- include/linux/rcupdate.h | 28 +++++++++++++++++++++++----- 1 files changed, 23 insertions(+), 5 deletions(-)
include/linux/rcupdate.h +23 -5
@@ -195,12 +195,30 @@ static inline int rcu_read_lock_sched_held(void)/** * rcu_dereference_check - rcu_dereference with debug checking +* @p: The pointer to read, prior to dereferencing +* @c: The conditions under which the dereference will take place * -* Do an rcu_dereference(), but check that the context is correct. -* For example, rcu_dereference_check(gp, rcu_read_lock_held()) to -* ensure that the rcu_dereference_check() executes within an RCU -* read-side critical section. It is also possible to check for -* locks being held, for example, by using lockdep_is_held(). +* Do an rcu_dereference(), but check that the conditions under which the +* dereference will take place are correct. Typically the conditions indicate +* the various locking conditions that should be held at that point. The check +* should return true if the conditions are satisfied. +* +* For example: +* +* bar = rcu_dereference_check(foo->bar, rcu_read_lock_held() || +* lockdep_is_held(&foo->lock)); +* +* could be used to indicate to lockdep that foo->bar may only be dereferenced +* if either the RCU read lock is held, or that the lock required to replace +* the bar struct at foo->bar is held. +* +* Note that the list of conditions may also include indications of when a lock +* need not be held, for example during initialisation or destruction of the +* target struct: +* +* bar = rcu_dereference_check(foo->bar, rcu_read_lock_held() || +* lockdep_is_held(&foo->lock) || +* atomic_read(&foo->usage) == 0); */ #define rcu_dereference_check (p, c) \ ({ \
Signature · 1 line
1.7.0
PATCH 3/3 rcu: Update docs for rcu_access_pointer and rcu_dereference_protected
In reply to Paul E. McKenneyUpdate examples and lists of APIs to include these new primitives.
--- Documentation/RCU/NMI-RCU.txt | 39 ++++++++++++++++++++++----------------- Documentation/RCU/checklist.txt | 7 ++++--- Documentation/RCU/lockdep.txt | 28 ++++++++++++++++++++++++++-- Documentation/RCU/whatisRCU.txt | 6 ++++++ 4 files changed, 58 insertions(+), 22 deletions(-)
Documentation/RCU/NMI-RCU.txt +22 -17
@@ -34,7 +34,7 @@ NMI handler. cpu = smp_processor_id(); ++nmi_count(cpu); - if (!rcu_dereference(nmi_callback)(regs, cpu)) + if (!rcu_dereference_sched(nmi_callback)(regs, cpu)) default_do_nmi(regs); nmi_exit();
@@ -47,12 +47,13 @@ function pointer. If this handler returns zero, do_nmi() invokes the default_do_nmi() function to handle a machine-specific NMI. Finally, preemption is restored. -Strictly speaking, rcu_dereference() is not needed, since this code runs -only on i386, which does not need rcu_dereference() anyway. However, -it is a good documentation aid, particularly for anyone attempting to -do something similar on Alpha. +In theory, rcu_dereference_sched() is not needed, since this code runs +only on i386, which in theory does not need rcu_dereference_sched() +anyway. However, in practice it is a good documentation aid, particularly +for anyone attempting to do something similar on Alpha or on systems +with aggressive optimizing compilers. -Quick Quiz: Why might the rcu_dereference() be necessary on Alpha, +Quick Quiz: Why might the rcu_dereference_sched() be necessary on Alpha, given that the code referenced by the pointer is read-only?
@@ -99,17 +100,21 @@ invoke irq_enter() and irq_exit() on NMI entry and exit, respectively. Answer to Quick Quiz - Why might the rcu_dereference() be necessary on Alpha, given + Why might the rcu_dereference_sched() be necessary on Alpha, given that the code referenced by the pointer is read-only? Answer: The caller to set_nmi_callback() might well have - initialized some data that is to be used by the - new NMI handler. In this case, the rcu_dereference() - would be needed, because otherwise a CPU that received - an NMI just after the new handler was set might see - the pointer to the new NMI handler, but the old - pre-initialized version of the handler's data. - - More important, the rcu_dereference() makes it clear - to someone reading the code that the pointer is being - protected by RCU. + initialized some data that is to be used by the new NMI + handler. In this case, the rcu_dereference_sched() would + be needed, because otherwise a CPU that received an NMI + just after the new handler was set might see the pointer + to the new NMI handler, but the old pre-initialized + version of the handler's data. + + This same sad story can happen on other CPUs when using + a compiler with aggressive pointer-value speculation + optimizations. + + More important, the rcu_dereference_sched() makes it + clear to someone reading the code that the pointer is + being protected by RCU-sched.
Documentation/RCU/checklist.txt +4 -3
@@ -260,7 +260,8 @@ over a rather long period of time, but improvements are always welcome! The reason that it is permissible to use RCU list-traversal primitives when the update-side lock is held is that doing so can be quite helpful in reducing code bloat when common code is - shared between readers and updaters. + shared between readers and updaters. Additional primitives + are provided for this case, as discussed in lockdep.txt. 10. Conversely, if you are in an RCU read-side critical section, and you don't hold the appropriate update-side lock, you -must-
@@ -344,8 +345,8 @@ over a rather long period of time, but improvements are always welcome! requiring SRCU's read-side deadlock immunity or low read-side realtime latency. - Note that, rcu_assign_pointer() and rcu_dereference() relate to - SRCU just as they do to other forms of RCU. + Note that, rcu_assign_pointer() relates to SRCU just as they do + to other forms of RCU. 15. The whole point of call_rcu(), synchronize_rcu(), and friends is to wait until all pre-existing readers have finished before
Documentation/RCU/lockdep.txt +26 -2
@@ -32,9 +32,20 @@ checking of rcu_dereference() primitives: srcu_dereference(p, sp): Check for SRCU read-side critical section. rcu_dereference_check(p, c): - Use explicit check expression "c". + Use explicit check expression "c". This is useful in + code that is invoked by both readers and updaters. rcu_dereference_raw(p) Don't check. (Use sparingly, if at all.) + rcu_dereference_protected(p, c): + Use explicit check expression "c", and omit all barriers + and compiler constraints. This is useful when the data + structure cannot change, for example, in code that is + invoked only by updaters. + rcu_access_pointer(p): + Return the value of the pointer and omit all barriers, + but retain the compiler constraints that prevent duplicating + or coalescsing. This is useful when when testing the + value of the pointer itself, for example, against NULL. The rcu_dereference_check() check expression can be any boolean expression, but would normally include one of the rcu_read_lock_held()
@@ -59,7 +70,20 @@ In case (1), the pointer is picked up in an RCU-safe manner for vanilla RCU read-side critical sections, in case (2) the ->file_lock prevents any change from taking place, and finally, in case (3) the current task is the only task accessing the file_struct, again preventing any change -from taking place. +from taking place. If the above statement was invoked only from updater +code, it could instead be written as follows: + + file = rcu_dereference_protected(fdt->fd[fd], + lockdep_is_held(&files->file_lock) || + atomic_read(&files->count) == 1); + +This would verify cases #2 and #3 above, and furthermore lockdep would +complain if this was used in an RCU read-side critical section unless one +of these two cases held. Because rcu_dereference_protected() omits all +barriers and compiler constraints, it generates better code than do the +other flavors of rcu_dereference(). On the other hand, it is illegal +to use rcu_dereference_protected() if either the RCU-protected pointer +or the RCU-protected data that it points to can change concurrently. There are currently only "universal" versions of the rcu_assign_pointer() and RCU list-/tree-traversal primitives, which do not (yet) check for
Documentation/RCU/whatisRCU.txt +6 -0
@@ -840,6 +840,12 @@ SRCU: Initialization/cleanup init_srcu_struct cleanup_srcu_struct +All: lockdep-checked RCU-protected pointer access + + rcu_dereference_check + rcu_dereference_protected + rcu_access_pointer + See the comment headers in the source code (or the docbook generated from them) for more information.
Signature · 1 line
1.7.0
Re: [PATCH tip/core/urgent 1/3] rcu: add rcu_access_pointer and rcu_dereference_protected
In reply to Paul E. McKenneyQuoted 11 lines · Paul E. McKenney
This patch adds variants of
rcu_dereference()that handle situations where the RCU-protected data structure cannot change, perhaps due to our holding the update-side lock, or where the RCU-protected pointer is only to be fetched, not dereferenced. These are needed due to some performance concerns with usingrcu_dereference()where it is not required, aside from the need for lockdep/sparse checking.The new
rcu_access_pointer()primitive is for the case where the pointer is be fetch and not dereferenced. This primitive may be used without protection, RCU or otherwise, due to the fact that it usesACCESS_ONCE().
Quoted 7 lines
The new
rcu_dereference_protected()primitive is for the case where updates are prevented, for example, due to holding the update-side lock. This primitive does neitherACCESS_ONCE()norsmp_read_barrier_depends(), so can only be used when updates are somehow prevented.Suggested-by David Howells <[email protected]>Signed-off-by Paul E. McKenney <[email protected]>
Re: [PATCH tip/core/urgent 1/3] rcu: add rcu_access_pointer and rcu_dereference_protected
In reply to Paul E. McKenneyQuoted 11 lines · Paul E. McKenney
This patch adds variants of
rcu_dereference()that handle situations where the RCU-protected data structure cannot change, perhaps due to our holding the update-side lock, or where the RCU-protected pointer is only to be fetched, not dereferenced. These are needed due to some performance concerns with usingrcu_dereference()where it is not required, aside from the need for lockdep/sparse checking.The new
rcu_access_pointer()primitive is for the case where the pointer is be fetch and not dereferenced. This primitive may be used without protection, RCU or otherwise, due to the fact that it usesACCESS_ONCE().
Quoted 7 lines
The new
rcu_dereference_protected()primitive is for the case where updates are prevented, for example, due to holding the update-side lock. This primitive does neitherACCESS_ONCE()norsmp_read_barrier_depends(), so can only be used when updates are somehow prevented.Suggested-by David Howells <[email protected]>Signed-off-by Paul E. McKenney <[email protected]>
Josh Triplett
maintainerRe: [PATCH tip/core/urgent 1/4] rcu: add rcu_access_pointer and rcu_dereference_protected
In reply to Paul E. McKenneyQuoted 15 lines · Paul E. McKenney
Quoted 14 lines · Eric Dumazet
Quoted 12 lines · David Howells
include/linux/rcupdate.h static inline int rcu_read_lock_sched_held(void) · line 246
Quoted 3 lines · Paul E. McKenney
include/linux/rcupdate.h +2
-0+#define rcu_access_pointer (p) ((void *)ACCESS_ONCE(p)) +#define rcu_access_pointer (p) ((void *)ACCESS_ONCE(p))There's no difference between your two versions of
rcu_access_pointer(), so you could move that whole construct outside of the #ifdef'ed section.Other than that:
Acked-by David Howells <[email protected]>
Quoted 8 lines
Quoted 3 lines
I also prefer to keep type checking if possible
#define
rcu_access_pointer(p)(ACCESS_ONCE(p))Yikes! I was obsessing on preventing people from dereferencing the pointer, but you are quite right, they might be comparing it to a constant pointer to check for array bounds or for use of a statically allocated emergency-pool item.
If you want to prevent people from deferencing the pointer directly, or from assigning it to some other pointer, you could use Sparse's __attribute__((noderef)) and __attribute__((address_space(...)).
See the definitions of __user and __iomem in include/linux/compiler.h for instance. (And, looking at that file, now __percpu. Nice!)
You'd then have to explicitly use __force at the point where you legitimately dereference it.
- Josh Triplett
Re: [PATCH tip/core/urgent 1/4] rcu: add rcu_access_pointer and rcu_dereference_protected
Quoted 10 lines · On Monday 12 April 2010 21:24:42 Josh Triplett wrote:
If you want to prevent people from deferencing the pointer directly, or from assigning it to some other pointer, you could use Sparse's __attribute__((noderef)) and __attribute__((address_space(...)).
See the definitions of
__userand__iomemininclude/linux/compiler.hfor instance. (And, looking at that file, now__percpu. Nice!)You'd then have to explicitly use
__forceat the point where you legitimately dereference it.
I have started a patch set for doing that a few weeks ago, still need to pick up that work again. Are there any other patches besides this series that I should base on top of now?
Arnd
Re: [PATCH tip/core/urgent 1/4] rcu: add rcu_access_pointer and rcu_dereference_protected
Quoted 9 lines · Arnd Bergmann
Quoted 8 lines · On Monday 12 April 2010 21:24:42 Josh Triplett wrote:
If you want to prevent people from deferencing the pointer directly, or from assigning it to some other pointer, you could use Sparse's __attribute__((noderef)) and __attribute__((address_space(...)).
See the definitions of
__userand__iomemininclude/linux/compiler.hfor instance. (And, looking at that file, now__percpu. Nice!)
Quoted 6 lines
Quoted 2 lines
You'd then have to explicitly use
__forceat the point where you legitimately dereference it.I have started a patch set for doing that a few weeks ago, still need to pick up that work again. Are there any other patches besides this series that I should base on top of now?
Very good, Arnd!!! Looking forward to seeing the updated patchset!!!
My suggestion would be to take the current tip/core/urgent, apply this patch set, and start from there.
Thanx, Paul
[tip:core/urgent] rcu: Add rcu_access_pointer and rcu_dereference_protected
In reply to Paul E. McKenneyCommit-ID: b62730baea32f86fe91a7930e4b7ee8d82778b79 Gitweb: http://git.kernel.org/tip/b62730baea32f86fe91a7930e4b7ee8d82778b79 Author: Paul E. McKenney <[email protected]> AuthorDate: Fri, 9 Apr 2010 15:39:10 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Wed, 14 Apr 2010 12:19:51 +0200
rcu: Add rcu_access_pointer and rcu_dereference_protected
This patch adds variants of rcu_dereference() that handle situations where the RCU-protected data structure cannot change, perhaps due to our holding the update-side lock, or where the RCU-protected pointer is only to be fetched, not dereferenced. These are needed due to some performance concerns with using rcu_dereference() where it is not required, aside from the need for lockdep/sparse checking.
The new rcu_access_pointer() primitive is for the case where the pointer is be fetch and not dereferenced. This primitive may be used without protection, RCU or otherwise, due to the fact that it uses ACCESS_ONCE().
The new rcu_dereference_protected() primitive is for the case where updates are prevented, for example, due to holding the update-side lock. This primitive does neither ACCESS_ONCE() nor smp_read_barrier_depends(), so can only be used when updates are somehow prevented.
LKML-Reference: <[email protected]>
--- include/linux/rcupdate.h | 32 ++++++++++++++++++++++++++++++++ 1 files changed, 32 insertions(+), 0 deletions(-)
include/linux/rcupdate.h +32 -0
@@ -209,13 +209,45 @@ static inline int rcu_read_lock_sched_held(void)rcu_dereference_raw (p); \ }) +/** +* rcu_dereference_protected - fetch RCU pointer when updates prevented +* +* Return the value of the specified RCU-protected pointer, but omit +* both the smp_read_barrier_depends() and the ACCESS_ONCE(). This +* is useful in cases where update-side locks prevent the value of the +* pointer from changing. Please note that this primitive does -not- +* prevent the compiler from repeating this reference or combining it +* with other references, so it should not be used without protection +* of appropriate locks. +*/ +#define rcu_dereference_protected (p, c) \ + ({ \ + if (debug_lockdep_rcu_enabled() && !(c)) \ + lockdep_rcu_dereference(__FILE__, __LINE__); \ + (p); \ + }) +#else /* #ifdef CONFIG_PROVE_RCU */ #define rcu_dereference_check (p, c) rcu_dereference_raw(p) +#define rcu_dereference_protected (p, c) (p)#endif /* #else #ifdef CONFIG_PROVE_RCU */ /** +* rcu_access_pointer - fetch RCU pointer with no dereferencing +* +* Return the value of the specified RCU-protected pointer, but omit the +* smp_read_barrier_depends() and keep the ACCESS_ONCE(). This is useful +* when the value of this pointer is accessed, but the pointer is not +* dereferenced, for example, when testing an RCU-protected pointer against +* NULL. This may also be used in cases where update-side locks prevent +* the value of the pointer from changing, but rcu_dereference_protected() +* is a lighter-weight primitive for this use case. +*/ +#define rcu_access_pointer (p) ACCESS_ONCE(p) + +/** * rcu_read_lock - mark the beginning of an RCU read-side critical section. * * When synchronize_rcu() is invoked on one CPU while other CPUs
[tip:core/urgent] rcu: Better explain the condition parameter of rcu_dereference_check()
In reply to Paul E. McKenneyCommit-ID: c08c68dd76bd6b776bc0eb45a5e8f354ed772cdf Gitweb: http://git.kernel.org/tip/c08c68dd76bd6b776bc0eb45a5e8f354ed772cdf Author: David Howells <[email protected]> AuthorDate: Fri, 9 Apr 2010 15:39:11 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Wed, 14 Apr 2010 12:20:04 +0200
rcu: Better explain the condition parameter of rcu_dereference_check()
Better explain the condition parameter of rcu_dereference_check() that describes the conditions under which the dereference is permitted to take place (and incorporate Yong Zhang's suggestion). This condition is only checked under lockdep proving.
LKML-Reference: <[email protected]>
--- include/linux/rcupdate.h | 28 +++++++++++++++++++++++----- 1 files changed, 23 insertions(+), 5 deletions(-)
include/linux/rcupdate.h +23 -5
@@ -195,12 +195,30 @@ static inline int rcu_read_lock_sched_held(void)/** * rcu_dereference_check - rcu_dereference with debug checking +* @p: The pointer to read, prior to dereferencing +* @c: The conditions under which the dereference will take place * -* Do an rcu_dereference(), but check that the context is correct. -* For example, rcu_dereference_check(gp, rcu_read_lock_held()) to -* ensure that the rcu_dereference_check() executes within an RCU -* read-side critical section. It is also possible to check for -* locks being held, for example, by using lockdep_is_held(). +* Do an rcu_dereference(), but check that the conditions under which the +* dereference will take place are correct. Typically the conditions indicate +* the various locking conditions that should be held at that point. The check +* should return true if the conditions are satisfied. +* +* For example: +* +* bar = rcu_dereference_check(foo->bar, rcu_read_lock_held() || +* lockdep_is_held(&foo->lock)); +* +* could be used to indicate to lockdep that foo->bar may only be dereferenced +* if either the RCU read lock is held, or that the lock required to replace +* the bar struct at foo->bar is held. +* +* Note that the list of conditions may also include indications of when a lock +* need not be held, for example during initialisation or destruction of the +* target struct: +* +* bar = rcu_dereference_check(foo->bar, rcu_read_lock_held() || +* lockdep_is_held(&foo->lock) || +* atomic_read(&foo->usage) == 0); */ #define rcu_dereference_check (p, c) \ ({ \
[tip:core/urgent] rcu: Update docs for rcu_access_pointer and rcu_dereference_protected
In reply to Paul E. McKenneyCommit-ID: 50aec0024eccb1d5f540ab64a1958eebcdb9340c Gitweb: http://git.kernel.org/tip/50aec0024eccb1d5f540ab64a1958eebcdb9340c Author: Paul E. McKenney <[email protected]> AuthorDate: Fri, 9 Apr 2010 15:39:12 -0700 Committer: Ingo Molnar <[email protected]> CommitDate: Wed, 14 Apr 2010 12:20:12 +0200
rcu: Update docs for rcu_access_pointer and rcu_dereference_protected
Update examples and lists of APIs to include these new primitives.
LKML-Reference: <[email protected]>
--- Documentation/RCU/NMI-RCU.txt | 39 ++++++++++++++++++++++----------------- Documentation/RCU/checklist.txt | 7 ++++--- Documentation/RCU/lockdep.txt | 28 ++++++++++++++++++++++++++-- Documentation/RCU/whatisRCU.txt | 6 ++++++ 4 files changed, 58 insertions(+), 22 deletions(-)
Documentation/RCU/NMI-RCU.txt +22 -17
@@ -34,7 +34,7 @@ NMI handler. cpu = smp_processor_id(); ++nmi_count(cpu); - if (!rcu_dereference(nmi_callback)(regs, cpu)) + if (!rcu_dereference_sched(nmi_callback)(regs, cpu)) default_do_nmi(regs); nmi_exit();
@@ -47,12 +47,13 @@ function pointer. If this handler returns zero, do_nmi() invokes the default_do_nmi() function to handle a machine-specific NMI. Finally, preemption is restored. -Strictly speaking, rcu_dereference() is not needed, since this code runs -only on i386, which does not need rcu_dereference() anyway. However, -it is a good documentation aid, particularly for anyone attempting to -do something similar on Alpha. +In theory, rcu_dereference_sched() is not needed, since this code runs +only on i386, which in theory does not need rcu_dereference_sched() +anyway. However, in practice it is a good documentation aid, particularly +for anyone attempting to do something similar on Alpha or on systems +with aggressive optimizing compilers. -Quick Quiz: Why might the rcu_dereference() be necessary on Alpha, +Quick Quiz: Why might the rcu_dereference_sched() be necessary on Alpha, given that the code referenced by the pointer is read-only?
@@ -99,17 +100,21 @@ invoke irq_enter() and irq_exit() on NMI entry and exit, respectively. Answer to Quick Quiz - Why might the rcu_dereference() be necessary on Alpha, given + Why might the rcu_dereference_sched() be necessary on Alpha, given that the code referenced by the pointer is read-only? Answer: The caller to set_nmi_callback() might well have - initialized some data that is to be used by the - new NMI handler. In this case, the rcu_dereference() - would be needed, because otherwise a CPU that received - an NMI just after the new handler was set might see - the pointer to the new NMI handler, but the old - pre-initialized version of the handler's data. - - More important, the rcu_dereference() makes it clear - to someone reading the code that the pointer is being - protected by RCU. + initialized some data that is to be used by the new NMI + handler. In this case, the rcu_dereference_sched() would + be needed, because otherwise a CPU that received an NMI + just after the new handler was set might see the pointer + to the new NMI handler, but the old pre-initialized + version of the handler's data. + + This same sad story can happen on other CPUs when using + a compiler with aggressive pointer-value speculation + optimizations. + + More important, the rcu_dereference_sched() makes it + clear to someone reading the code that the pointer is + being protected by RCU-sched.
Documentation/RCU/checklist.txt +4 -3
@@ -260,7 +260,8 @@ over a rather long period of time, but improvements are always welcome! The reason that it is permissible to use RCU list-traversal primitives when the update-side lock is held is that doing so can be quite helpful in reducing code bloat when common code is - shared between readers and updaters. + shared between readers and updaters. Additional primitives + are provided for this case, as discussed in lockdep.txt. 10. Conversely, if you are in an RCU read-side critical section, and you don't hold the appropriate update-side lock, you -must-
@@ -344,8 +345,8 @@ over a rather long period of time, but improvements are always welcome! requiring SRCU's read-side deadlock immunity or low read-side realtime latency. - Note that, rcu_assign_pointer() and rcu_dereference() relate to - SRCU just as they do to other forms of RCU. + Note that, rcu_assign_pointer() relates to SRCU just as they do + to other forms of RCU. 15. The whole point of call_rcu(), synchronize_rcu(), and friends is to wait until all pre-existing readers have finished before
Documentation/RCU/lockdep.txt +26 -2
@@ -32,9 +32,20 @@ checking of rcu_dereference() primitives: srcu_dereference(p, sp): Check for SRCU read-side critical section. rcu_dereference_check(p, c): - Use explicit check expression "c". + Use explicit check expression "c". This is useful in + code that is invoked by both readers and updaters. rcu_dereference_raw(p) Don't check. (Use sparingly, if at all.) + rcu_dereference_protected(p, c): + Use explicit check expression "c", and omit all barriers + and compiler constraints. This is useful when the data + structure cannot change, for example, in code that is + invoked only by updaters. + rcu_access_pointer(p): + Return the value of the pointer and omit all barriers, + but retain the compiler constraints that prevent duplicating + or coalescsing. This is useful when when testing the + value of the pointer itself, for example, against NULL. The rcu_dereference_check() check expression can be any boolean expression, but would normally include one of the rcu_read_lock_held()
@@ -59,7 +70,20 @@ In case (1), the pointer is picked up in an RCU-safe manner for vanilla RCU read-side critical sections, in case (2) the ->file_lock prevents any change from taking place, and finally, in case (3) the current task is the only task accessing the file_struct, again preventing any change -from taking place. +from taking place. If the above statement was invoked only from updater +code, it could instead be written as follows: + + file = rcu_dereference_protected(fdt->fd[fd], + lockdep_is_held(&files->file_lock) || + atomic_read(&files->count) == 1); + +This would verify cases #2 and #3 above, and furthermore lockdep would +complain if this was used in an RCU read-side critical section unless one +of these two cases held. Because rcu_dereference_protected() omits all +barriers and compiler constraints, it generates better code than do the +other flavors of rcu_dereference(). On the other hand, it is illegal +to use rcu_dereference_protected() if either the RCU-protected pointer +or the RCU-protected data that it points to can change concurrently. There are currently only "universal" versions of the rcu_assign_pointer() and RCU list-/tree-traversal primitives, which do not (yet) check for
Documentation/RCU/whatisRCU.txt +6 -0
@@ -840,6 +840,12 @@ SRCU: Initialization/cleanup init_srcu_struct cleanup_srcu_struct +All: lockdep-checked RCU-protected pointer access + + rcu_dereference_check + rcu_dereference_protected + rcu_access_pointer + See the comment headers in the source code (or the docbook generated from them) for more information.