Re: [PATCH net 1/2] netpoll: Use rcu_access_pointer() in __netpoll_setup
From: Herbert Xu
Date: Tue Nov 19 2024 - 22:02:02 EST
On Tue, Nov 19, 2024 at 02:22:06AM -0800, Breno Leitao wrote:
>
> I looked about rcu_dereference_protected() as well, and I though it is
> used when you are de-referencing the pointer, which is a more expensive
> approach. In the code above, the code basically need to check if the
> pointer is assigned or not. Looking at the code, it seems that having
> rcu_access_pointer() inside the update lock seems a common pattern, than
> that is what I chose.
No, rcu_dereference_protected is actually cheaper than rcu_access_pointer:
#define __rcu_access_pointer(p, local, space) \
({ \
typeof(*p) *local = (typeof(*p) *__force)READ_ONCE(p); \
rcu_check_sparse(p, space); \
((typeof(*p) __force __kernel *)(local)); \
})
#define __rcu_dereference_protected(p, local, c, space) \
({ \
RCU_LOCKDEP_WARN(!(c), "suspicious rcu_dereference_protected() usage"); \
rcu_check_sparse(p, space); \
((typeof(*p) __force __kernel *)(p)); \
})
> On the other side, I understand we want to call an RCU primitive with
> the _protected() context, so, I looked for a possible
> `rcu_access_pointer_protected()`, but this best does not exist. Anyway,
> I am happy to change it, if it is the correct API.
There is no need for rcu_access_pointer_protected because the
rcu_dereference_protected helper is already the cheapest.
> When 8fdd95ec162a was created, npinfo was an RCU pointer, although
> without the RCU annotation that came later (5fbee843c). That is
> reason I chose to fix 8fdd95ec162a.
The code was correct as is without RCU markings. The only reason
we need the RCU markings is because an __rcu tag was added to the
variable later, without also making the necessary changes in the
existing code using that variable.
Cheers,
--
Email: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt