Re: [PATCH net-next V3 2/7] netdevsim: Register devlink after device init

[Jakub Kicinski <kuba@kernel.org>]

On Thu, 11 Jun 2026 09:02:03 +0300 Mark Bloch wrote:
> On 11/06/2026 2:50, Jakub Kicinski wrote:
> > On Fri, 5 Jun 2026 21:10:25 +0300 Mark Bloch wrote:  
> >> devl_register() makes the devlink instance visible to userspace. A later
> >> patch also makes registration the point where devlink core may call
> >> eswitch_mode_set() to apply a boot-time default eswitch mode.
> >>
> >> Move netdevsim registration after all objects (resources, params, regions,
> >> traps, debugfs etc) are initialized, and after the initial eswitch mode is
> >> set to legacy.
> >>
> >> Move devl_unregister() to the beginning of nsim_drv_remove(), before those
> >> devlink objects are torn down. This keeps devlink register/unregister as
> >> the notification barrier and makes the later object teardown paths run
> >> after devlink is no longer registered, so they do not emit their own
> >> netlink DEL notifications.  
> > 
> > This is going backwards. At some point someone from nVidia thought that
> > we can order our way out of locking, so mlx5 is likely ordered this way,
> > but this must not be required, or in any way normalized.
> > We (syzbot) quickly discovered that it doesn't cover all corner cases.
> > devl_lock() is exposed specifically to allow the driver to finish
> > whatever init it needs without letting user space invoke callbacks, yet.
> > Almost (?) all driver callbacks hold devl_lock(), so maybe the devlink
> > instance is "visible" to user space but that should not matter.  
> 
> Let me clarify.
> 
> No locking is changed here, and I don't want to make register/unregister
> ordering a substitute for devl_lock().
> 
> The only requirement I have for this series is that devl_register() is called
> only once the driver is ready for devlink core to call eswitch_mode_set().
> That follows from the earlier direction to have the core apply the default
> mode from devl_register() instead of adding an explicit driver call.

This is exactly what I'm objecting to. AFAIU we are trading off
explicit call to get the default value for an implicit behavior
depending on order of calls. We want to optimize for how easy it
is to get the API wrong, not for LoC.

If we don't have a clean way to implement this without driver
changes let's add the explicit API to get the default value.
If driver doesn't call it schedule a work to go via the callback
once devl_lock() is dropped. That way drivers which care can optimize
themselves by reading the default value upfront. Drivers which don't 
care will work correctly, and there's no API call order trap.

Not ideal, but isn't that best we can do here?
I still have flashbacks of the fallout from the call ordering games, 
we have too many drivers to keep this straight...

> So if the objection is to the commit message wording, I can fix that and drop
> the "notification barrier" language.
> 
> For unregister, I can probably leave the old ordering as-is. I moved it only
> to mirror the register path, which felt cleaner, but it is not required for
> the default-mode change and as the lock is held I see no issue with doing
> that.