Messages in this thread | | | Date | Wed, 25 Mar 2015 16:01:24 +0100 | From | Patrick Marlier <> | Subject | Re: [PATCH 1/3] rculist: Fix list_entry_rcu to read ptr with rcu_dereference_raw |
| |
On 03/25/2015 03:30 PM, Paul E. McKenney wrote: > On Tue, Mar 24, 2015 at 11:31:38AM +0100, Patrick Marlier wrote: >> Change to read effectively ptr with rcu_dereference_raw and not the >> __ptr variable on the stack. >> >> Signed-off-by: Patrick Marlier <patrick.marlier@gmail.com> > Avoiding an extra load could be worthwhile in a number of situations, > agreed. Not only a load. It adds a store and a load on the stack and I think this creates a dependency in the processor pipeline.
> However, won't this change cause sparse to complain if invoked on a > non-RCU-protected pointer? The ability to use list-RCU API > members on both RCU and non-RCU pointers was one of the points > of the previous commit, right? Probably we can put back the cast but I am not familiar enough with the RCU API.
Also, the problem here is that you probably want ACCESS_ONCE to happen on the content of 'ptr' and not on the stack variable '__ptr'.
(you have to follow this chain: rcu_dereference_raw -> rcu_dereference_check -> __rcu_dereference_check -> lockless_dereference -> ACCESS_ONCE)
#define lockless_dereference(p) \ ({ \ typeof(p) _________p1 = ACCESS_ONCE(p); \ smp_read_barrier_depends(); /* Dependency order vs. p above. */ \ (_________p1); \ })
#define __ACCESS_ONCE(x) ({ \ __maybe_unused typeof(x) __var = (__force typeof(x)) 0; \ (volatile typeof(x) *)&(x); }) #define ACCESS_ONCE(x) (*__ACCESS_ONCE(x))
Note that ACCESS_ONCE is doing "&" on x.
IMHO, I would prefer saving some useless instructions for better performance rather than giving too much flexibility on the API (also pretty sure the cast can be still done). -- Patrick Marlier
| |