Re: [PATCH net-next 01/14] net: ethtool: cmis_cdb: hold instance lock for ops locked devices

[Jakub Sitnicki <jakub@cloudflare.com>]

On Thu, May 28, 2026 at 04:16 PM -07, Jakub Kicinski wrote:
> FW module flashing was written so that the flashing happens
> without holding rtnl_lock. This allows flashing multiple modules
> at once. Current drivers can handle that well, but we should
> let drivers depend on the netdev instance lock. Instance lock
> is per netdev, and so is the module so we won't break parallel
> updates.
>
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
>  include/net/netdev_lock.h    | 6 ++++++
>  net/ethtool/cmis_cdb.c       | 3 +++
>  net/ethtool/cmis_fw_update.c | 8 ++------
>  net/ethtool/module.c         | 2 ++
>  4 files changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/include/net/netdev_lock.h b/include/net/netdev_lock.h
> index 3d3aef80beac..f4c77899fb86 100644
> --- a/include/net/netdev_lock.h
> +++ b/include/net/netdev_lock.h
> @@ -80,6 +80,12 @@ netdev_ops_assert_locked_or_invisible(const struct net_device *dev)
>  		netdev_ops_assert_locked(dev);
>  }
>  
> +static inline void netdev_assert_locked_if_ops(const struct net_device *dev)
> +{
> +	if (netdev_need_ops_lock(dev))
> +		netdev_assert_locked(dev);
> +}
> +

Nit (because in the kernel naming is a competitive sport):

How about we name that netdev_assert_locked_ops() so it's clear that
this checks if netdev_{lock,unlock}_ops() has been called?

I'm guessing the intention behind the name was to make it
distinguishable from the legacy netdev_ops_assert_locked().

My suggestion would be to rename that one to
netdev_assert_locked_ops_compat() following the
netdev_{lock,unlock}_ops_compat() naming scheme. That should avoid
confusion and provide a clear signal which one we're phasing out.

Symmetry is beautiful :-)

Reviewed-by: Jakub Sitnicki <jakub@cloudflare.com>