On Sun, Apr 12, 2020 at 03:26:14PM +0200, Martin Pieuchot wrote: > The existing variations of the NET_LOCK() macros are confusing. We're > now all aware of this fact. So let's keep them simple to prevent future > mistakes :) > > The diff below reduces the current set of methods to the following: > > NET_LOCK()/NET_UNLOCK() > NET_ASSERT_LOCKED() > NET_ASSERT_UNLOCKED() > > On top of those, two extra methods are provided for a *very specific* > purpose: > > NET_RLOCK_IN_IOCTL() > NET_RUNLOCK_IN_IOCTL() > > "IN_IOCTL()" mean they should only be used in ioctl(2) context ;) The > purpose is to keep previous behavior where read-only ioctl(2) dot not > wait for packet processing to finish. > > To achieve this NET_LOCK() and NET_UNLOCK() now contain some magic to > ensure the only packet processing thread grabbing a read-lock is the > softnet thread. In other words, one doesn't need to be aware of all > this magic, just imagine that the Network Stack is big-locked and use > NET_LOCK()/NET_UNLOCK() everywhere.
I think it is a potential source of error if NET_LOCK() is sensitive to context. To understand what this code does, you have to know which thread calls it. I would prefer to have a separate NET_RLOCK variant for explicit use by softnet threads, say NET_RLOCK_IN_SOFTNET() or NET_RLOCK_SOFTNET(), which can assert that the calling context is correct and read-lock netlock.
