Re: [PATCH net-next v13 02/13] net: pse-pd: Add support for reporting events

[Jakub Kicinski <kuba@kernel.org>]

On Tue, 10 Jun 2025 10:11:36 +0200 Kory Maincent wrote:
> +static struct net_device *
> +pse_control_find_net_by_id(struct pse_controller_dev *pcdev, int id,
> +			   netdevice_tracker *tracker)
> +{
> +	struct pse_control *psec, *next;
> +
> +	mutex_lock(&pse_list_mutex);
> +	list_for_each_entry_safe(psec, next, &pcdev->pse_control_head, list) {

nit: _safe is not necessary here, the body of the if always exits after
dropping the lock

Do you plan to add more callers for this function?
Maybe it's better if it returns the psec pointer with the refcount
elevated. Because it would be pretty neat if we could move the 
ethnl_pse_send_ntf(netdev, notifs, &extack); that  pse_isr() does
right after calling this function under the rtnl_lock.
I don't think calling ethnl_pse_send_ntf() may crash the kernel as is,
but it feels like a little bit of a trap to have ethtool code called
outside of any networking lock.

> +		if (psec->id == id) {
> +			struct net_device *netdev = NULL;
> +			struct phy_device *phydev;
> +
> +			kref_get(&psec->refcnt);
> +			/* Release the mutex before taking the rtnl lock
> +			 * to avoid deadlock in case of a pse_control_put
> +			 * call with the rtnl lock held.
> +			 */
> +			mutex_unlock(&pse_list_mutex);
> +			/* Acquire rtnl to protect the net device
> +			 * reference get.
> +			 */
> +			rtnl_lock();
> +			phydev = psec->attached_phydev;
> +			if (phydev->attached_dev) {
> +				netdev = phydev->attached_dev;
> +				netdev_hold(netdev, tracker, GFP_KERNEL);
> +			}
> +			rtnl_unlock();
> +			pse_control_put(psec);
> +			return netdev;
> +		}
> +	}
> +	mutex_unlock(&pse_list_mutex);
> +	return NULL;
> +}