Re: [PATCH v10 net-next 3/5] psp: add a new netdev event for dev unregister

[Daniel Zahka <daniel.zahka@gmail.com>]

On 4/5/26 1:58 AM, Wei Wang wrote:
> From: Wei Wang <weibunny@fb.com>
>
> Add a new netdev event for dev unregister and handle the removal of this
> dev from psp->assoc_dev_list, upon the first dev-assoc operation.
>
> Signed-off-by: Wei Wang <weibunny@fb.com>
> ---
...
> +static bool psp_notifier_registered;
> +
> +/**
> + * psp_attach_netdev_notifier() - register netdev notifier on first use
> + *
> + * Register the netdevice notifier when the first device association
> + * is created. In many installations no associations will be created and
> + * the notifier won't be needed.
> + *
> + * Must be called without psd->lock held, due to lock ordering:
> + * rtnl_lock -> psd->lock (the notifier callback runs under rtnl_lock
> + * and takes psd->lock).
> + */
> +int psp_attach_netdev_notifier(void)
> +{
> +	int err = 0;
> +
> +	if (READ_ONCE(psp_notifier_registered))
> +		return 0;
> +
> +	mutex_lock(&psp_devs_lock);
> +	if (!psp_notifier_registered) {
> +		err = register_netdevice_notifier(&psp_netdev_notifier);
> +		if (!err)
> +			WRITE_ONCE(psp_notifier_registered, true);
> +	}
> +	mutex_unlock(&psp_devs_lock);
> +
> +	return err;
> +}
> +


This looks like an abuse of the psp_devs_lock to provide mutual 
exclusion on psp_notifier_registered, which has the undesirable property 
of establishing a lock dependency from psp_devs_lock -> rtnl_lock. 
Perhaps a dedicated mutex would be cleaner.

>   
> +/**
> + * Non-admin version of psp_device_get_locked() + psp_attach_netdev_notifier()
> + * only used for dev-assoc.
> + */
> +int psp_device_get_locked_dev_assoc(const struct genl_split_ops *ops,
> +				    struct sk_buff *skb, struct genl_info *info)


kdoc open, but not a kdoc comment.

Minor comments, otherwise:

Reviewed-by: Daniel Zahka <daniel.zahka@gmail.com>