| Message ID | 20240331091932.129617-1-mmpgouride@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | rcu: Remove unnecessary READ_ONCE in rcu_preempt_read_enter/exit | expand |
On Sun, Mar 31, 2024 at 09:19:32AM +0000, Alan Huang wrote: > The two READ_ONCEs here imply that there might be updater concurrently > modifies rcu_read_lock_nesting, although it's true there could be > interrupt handlers enter rcu RSCS, but rcu_read_lock_nesting should > remain unchanged after they finished. That is, READ_ONCE and then > WRITE_ONCE the same variable is not a valid pattern, this patch thus > removes the two READ_ONCEs. This is true, but I will take the READ_ONCE() calls over the KCSAN complaints. So I must avoid taking this one. Please note that RCU uses strict KCSAN configuration [1]. Thanx, Paul [1] https://docs.google.com/document/d/1FwZaXSg3A55ivVoWffA9iMuhJ3_Gmj_E494dLYjjyLQ/edit?usp=sharing > Signed-off-by: Alan Huang <mmpgouride@gmail.com> > --- > kernel/rcu/tree_plugin.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h > index 340bbefe5f65..c90edbb7fa40 100644 > --- a/kernel/rcu/tree_plugin.h > +++ b/kernel/rcu/tree_plugin.h > @@ -376,12 +376,12 @@ static int rcu_preempt_blocked_readers_cgp(struct rcu_node *rnp) > > static void rcu_preempt_read_enter(void) > { > - WRITE_ONCE(current->rcu_read_lock_nesting, READ_ONCE(current->rcu_read_lock_nesting) + 1); > + WRITE_ONCE(current->rcu_read_lock_nesting, current->rcu_read_lock_nesting + 1); > } > > static int rcu_preempt_read_exit(void) > { > - int ret = READ_ONCE(current->rcu_read_lock_nesting) - 1; > + int ret = current->rcu_read_lock_nesting - 1; > > WRITE_ONCE(current->rcu_read_lock_nesting, ret); > return ret; > -- > 2.34.1 > >
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h index 340bbefe5f65..c90edbb7fa40 100644 --- a/kernel/rcu/tree_plugin.h +++ b/kernel/rcu/tree_plugin.h @@ -376,12 +376,12 @@ static int rcu_preempt_blocked_readers_cgp(struct rcu_node *rnp) static void rcu_preempt_read_enter(void) { - WRITE_ONCE(current->rcu_read_lock_nesting, READ_ONCE(current->rcu_read_lock_nesting) + 1); + WRITE_ONCE(current->rcu_read_lock_nesting, current->rcu_read_lock_nesting + 1); } static int rcu_preempt_read_exit(void) { - int ret = READ_ONCE(current->rcu_read_lock_nesting) - 1; + int ret = current->rcu_read_lock_nesting - 1; WRITE_ONCE(current->rcu_read_lock_nesting, ret); return ret;
The two READ_ONCEs here imply that there might be updater concurrently modifies rcu_read_lock_nesting, although it's true there could be interrupt handlers enter rcu RSCS, but rcu_read_lock_nesting should remain unchanged after they finished. That is, READ_ONCE and then WRITE_ONCE the same variable is not a valid pattern, this patch thus removes the two READ_ONCEs. Signed-off-by: Alan Huang <mmpgouride@gmail.com> --- kernel/rcu/tree_plugin.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)