Re: [PATCH net 1/2] netpoll: Use rcu_access_pointer() in __netpoll_setup

[Herbert Xu <herbert@gondor.apana.org.au>]

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@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt