| Message ID | 20250331150603.1906635-8-sdf@fomichev.me (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: hold instance lock during NETDEV_UP/REGISTER | expand |
On Mon, 31 Mar 2025 08:05:59 -0700 Stanislav Fomichev wrote: > +The following notifiers are running without the lock (so the ops-locked > +devices need to manually grab the lock if needed): Not sure about the text in the parenthesis, "the devices" don't "grab the lock". I mean - drivers don't generally register for notifications about their own devices. It's whoever registered the notifier that needs to make sure they take appropriate locks. I think we're fine without that sentence. > +* ``NETDEV_UNREGISTER`` > + > +There are no clear expectations for the remaining notifiers. Notifiers not on > +the list may run with or without the instance lock, potentially even invoking > +the same notifier type with and without the lock from different code paths. > +The goal is to eventually ensure that all (or most, with a few documented > +exceptions) notifiers run under the instance lock. Should we add a sentence here along the lines of "Please extend this documentation whenever you make explicit assumption about lock being held from a notifier." or is that obvious?
On 03/31, Jakub Kicinski wrote: > On Mon, 31 Mar 2025 08:05:59 -0700 Stanislav Fomichev wrote: > > +The following notifiers are running without the lock (so the ops-locked > > +devices need to manually grab the lock if needed): > > Not sure about the text in the parenthesis, "the devices" don't "grab > the lock". I mean - drivers don't generally register for notifications > about their own devices. It's whoever registered the notifier that needs > to make sure they take appropriate locks. I think we're fine without > that sentence. Good point, I was mostly referring to dev_ vs netif_ calls for managing lower devices, will drop the sentence. > > +* ``NETDEV_UNREGISTER`` > > + > > +There are no clear expectations for the remaining notifiers. Notifiers not on > > +the list may run with or without the instance lock, potentially even invoking > > +the same notifier type with and without the lock from different code paths. > > +The goal is to eventually ensure that all (or most, with a few documented > > +exceptions) notifiers run under the instance lock. > > Should we add a sentence here along the lines of "Please extend this > documentation whenever you make explicit assumption about lock being > held from a notifier." or is that obvious? Yes, that was the assumption, but let's explicitly state that, shouldn't hurt.
diff --git a/Documentation/networking/netdevices.rst b/Documentation/networking/netdevices.rst index ebb868f50ac2..381243c002c1 100644 --- a/Documentation/networking/netdevices.rst +++ b/Documentation/networking/netdevices.rst @@ -343,6 +343,28 @@ there are two sets of interfaces: ``dev_xxx`` and ``netif_xxx`` (e.g., acquiring the instance lock themselves, while the ``netif_xxx`` functions assume that the driver has already acquired the instance lock. +Notifiers and netdev instance lock +================================== + +For device drivers that implement shaping or queue management APIs, +some of the notifiers (``enum netdev_cmd``) are running under the netdev +instance lock. + +For devices with locked ops, currently only the following notifiers are +running under the lock: +* ``NETDEV_REGISTER`` +* ``NETDEV_UP`` + +The following notifiers are running without the lock (so the ops-locked +devices need to manually grab the lock if needed): +* ``NETDEV_UNREGISTER`` + +There are no clear expectations for the remaining notifiers. Notifiers not on +the list may run with or without the instance lock, potentially even invoking +the same notifier type with and without the lock from different code paths. +The goal is to eventually ensure that all (or most, with a few documented +exceptions) notifiers run under the instance lock. + NETDEV_INTERNAL symbol namespace ================================
We don't have a consistent state yet, but document where we think we are and where we wanna be. Signed-off-by: Stanislav Fomichev <sdf@fomichev.me> --- Documentation/networking/netdevices.rst | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)