Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] The boolean flag `apc->channel_changing` is used as an ad-hoc boolean lock to guard the queue-set swap region against concurrent RDMA operations, violating networking driver guidelines. --
commit 47803780905d457c79c09a52f280d496801d6fc3 Author: Long Li <[email protected]> net: mana: swap queue sets in mana_set_ringparam This commit updates mana_set_ringparam to use queue-set allocation and publication instead of a detach and attach sequence. It ensures that failed allocations leave running queues unchanged and keeps RDMA excluded through failure cleanup. > 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 > @@ -774,32 +772,56 @@ static int mana_set_ringparam(struct net_device *ndev, [ ... ] > + /* Exclude RDMA through failure cleanup, which may release the vport. */ > + 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: Medium] Does this code introduce an ad-hoc boolean lock? The networking driver guidelines advise against using boolean flags like apc->channel_changing to guard code sections instead of using proper kernel synchronization primitives. By returning -EBUSY when the flag is set, this forces userspace clients calling ethtool to implement retry loops rather than gracefully waiting on a standard mutex. Could a standard mutex be used here to serialize these operations instead? [ ... ] > +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
