Re: [PATCH net-next v2 04/13] net: mana: swap queue sets in mana_set_priv_flags

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The boolean flag `apc->channel_changing` is used as an open-coded, ad-hoc lock to protect vport reconfiguration against concurrent RDMA access, violating core kernel locking rules.
--

--- Patch [4]: [PATCH net-next v2 04/13] net: mana: swap queue sets in mana_set_priv_flags ---
commit 922132e276d163d3c58e43d88b85f6b85a403949
Author: Long Li <[email protected]>

net: mana: swap queue sets in mana_set_priv_flags

This commit modifies mana_set_priv_flags to pre-allocate and swap queue sets
instead of rebuilding them in place. This ensures that if the rebuild fails,
both the queues and the flag word remain untouched.

> diff --git a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
> --- a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
[ ... ]
> @@ -884,54 +888,53 @@ static int mana_set_priv_flags(struct net_device *ndev, u32 priv_flags)
[ ... ]
> -		/* Block RDMA from grabbing the vport during detach/attach */
> -		mutex_lock(&apc->vport_mutex);
> -		apc->channel_changing = true;
> +	/* Block RDMA from acquiring the vport for the duration. */
> +	mutex_lock(&apc->vport_mutex);
> +	if (apc->channel_changing) {
>  		mutex_unlock(&apc->vport_mutex);
> +		return -EBUSY;
> +	}
> +	apc->channel_changing = true;
> +	mutex_unlock(&apc->vport_mutex);

[Severity: High]
This is a pre-existing issue, but in mana_set_priv_flags, is using a boolean
flag like apc->channel_changing as an open-coded trylock safe?

It appears to be set on entry here and cleared on exit to protect vport
reconfiguration against concurrent RDMA access, rather than using a standard
synchronization primitive. Does this ad-hoc locking pattern evade lockdep
and risk races since it bypasses proper kernel locking rules?

[ ... ]
> clear_flag:
>  	mutex_lock(&apc->vport_mutex);
>  	apc->channel_changing = false;
>  	mutex_unlock(&apc->vport_mutex);
> -
>  	return err;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4