Re: [PATCH net-next v4 04/13] net: mana: swap queue sets in mana_set_ringparam
From: sashiko-bot@kernel.org
Date: 2026-09-09 03:29:45
Also in:
linux-rdma
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] The patch introduces an ad-hoc locking pattern using a boolean flag (`apc->channel_changing`) to guard the reconfiguration critical section in `mana_set_ringparam`. -- commit 40668ebf41a83bf44835ffb33803930c039ce405 Author: Long Li [off-list ref] net: mana: swap queue sets in mana_set_ringparam This commit replaces detach/attach operations in mana_set_ringparam() with queue-set allocation and publication. Failed allocations now leave the running queues and ring sizes unchanged instead of risking a detached port.
quoted hunk ↗ jump to hunk
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
[ ... ]
quoted hunk ↗ jump to hunk
@@ -774,32 +772,56 @@ static int mana_set_ringparam(struct net_device *ndev, netdev_info(ndev, "Using nearest power of 2 values for Txq:%d Rxq:%d\n", new_tx, new_rx); - /* pre-allocating new buffers to prevent failures in mana_attach() later */ - apc->rx_queue_size = new_rx; - err = mana_pre_alloc_rxbufs(apc, ndev->mtu, apc->num_queues); - apc->rx_queue_size = old_rx; - if (err) { - netdev_err(ndev, "Insufficient memory for new allocations\n"); - return err; + if (new_rx == apc->rx_queue_size && new_tx == apc->tx_queue_size) + return 0; + + if (!apc->port_is_up) { + apc->rx_queue_size = new_rx; + apc->tx_queue_size = new_tx; + return 0; } - err = mana_detach(ndev, false); - if (err) { - netdev_err(ndev, "mana_detach failed: %d\n", err); - goto out; + /* 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 locking pattern using the apc->channel_changing boolean flag? The networking drivers subsystem guidelines indicate that using a boolean flag set on entry and cleared on exit to guard a reconfiguration critical section bypasses lockdep and provides weaker synchronization guarantees than using a proper lock. Could a standard kernel synchronization primitive be used for this critical section in mana_set_ringparam() instead of relying on the apc->channel_changing flag? [ ... ]
+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/20260908032843.397667-1-longli@microsoft.com?part=4