| Message ID | 20241118-netpoll_rcu-v1-2-a1888dcb4a02@debian.org (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | netpoll: Use RCU primitives for npinfo pointer access | expand |
On Mon, Nov 18, 2024 at 03:15:18AM -0800, Breno Leitao wrote: > The ndev->npinfo pointer in netpoll_poll_lock() is RCU-protected but is > being accessed directly for a NULL check. While no RCU read lock is held > in this context, we should still use proper RCU primitives for > consistency and correctness. > > Replace the direct NULL check with rcu_access_pointer(), which is the > appropriate primitive when only checking for NULL without dereferencing > the pointer. This function provides the necessary ordering guarantees > without requiring RCU read-side protection. > > Signed-off-by: Breno Leitao <leitao@debian.org> > Fixes: bea3348eef27 ("[NET]: Make NAPI polling independent of struct net_device objects.") nitpick: As for the first patch - please check the tags order. Thanks, Reviewed-by: Michal Kubiak <michal.kubiak@intel.com>
On Mon, Nov 18, 2024 at 01:20:30PM +0100, Michal Kubiak wrote: > On Mon, Nov 18, 2024 at 03:15:18AM -0800, Breno Leitao wrote: > > The ndev->npinfo pointer in netpoll_poll_lock() is RCU-protected but is > > being accessed directly for a NULL check. While no RCU read lock is held > > in this context, we should still use proper RCU primitives for > > consistency and correctness. > > > > Replace the direct NULL check with rcu_access_pointer(), which is the > > appropriate primitive when only checking for NULL without dereferencing > > the pointer. This function provides the necessary ordering guarantees > > without requiring RCU read-side protection. > > > > Signed-off-by: Breno Leitao <leitao@debian.org> > > Fixes: bea3348eef27 ("[NET]: Make NAPI polling independent of struct net_device objects.") > > nitpick: As for the first patch - please check the tags order. > > Thanks, > Reviewed-by: Michal Kubiak <michal.kubiak@intel.com> Thanks for the review. I am not planning to resend it now, since I think maintainer's tooling will reorder that. If that is not true, I am more than happy to resend fixing the order. Thanks Breno
diff --git a/include/linux/netpoll.h b/include/linux/netpoll.h index cd4e28db0cbd77572a579aff2067b5864d1a904a..959a4daacea1f2f76536e309d198bc14407942a4 100644 --- a/include/linux/netpoll.h +++ b/include/linux/netpoll.h @@ -72,7 +72,7 @@ static inline void *netpoll_poll_lock(struct napi_struct *napi) { struct net_device *dev = napi->dev; - if (dev && dev->npinfo) { + if (dev && rcu_access_pointer(dev->npinfo)) { int owner = smp_processor_id(); while (cmpxchg(&napi->poll_owner, -1, owner) != -1)
The ndev->npinfo pointer in netpoll_poll_lock() is RCU-protected but is being accessed directly for a NULL check. While no RCU read lock is held in this context, we should still use proper RCU primitives for consistency and correctness. Replace the direct NULL check with rcu_access_pointer(), which is the appropriate primitive when only checking for NULL without dereferencing the pointer. This function provides the necessary ordering guarantees without requiring RCU read-side protection. Signed-off-by: Breno Leitao <leitao@debian.org> Fixes: bea3348eef27 ("[NET]: Make NAPI polling independent of struct net_device objects.") --- include/linux/netpoll.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)