Legacy ibmveth uses five adapter-level RX buffer pools (512 B through 64 KiB). pool_active[] enables the standard-MTU pools by default; larger pools activate when MTU requires them. With single-queue RX that set is shared on one completion path. MQ requires the same pool model per queue: buffers post with H_ADD_LOGICAL_LAN_BUFFERS_QUEUE against a queue handle and completions return on that queue. Sharing pools across queues would mix ownership and break queue-local replenish/drain/teardown. Refactor around queue-local pools: rx_buff_pool[queue][pool] ibmveth_alloc_queue_buffer_pools() ibmveth_free_queue_buffer_pools() ibmveth_alloc_buffer_pools() / ibmveth_free_buffer_pools() Queue 0 remains the template for pool geometry and activation policy (size, buff_size, threshold, index, active). Queues 1..N copy that metadata from queue 0, then allocate backing arrays/skbs per queue. The existing poolN/ sysfs knobs stay adapter-wide on queue 0: writing a pool size multiplies real memory by num_rx_queues (still 1 here). Per-queue runtime state is exposed later via debugfs, not separate per-queue pool sysfs nodes. Wire the helpers into open()/close() in the same patch. Runtime remains single-queue (num_rx_queues is still 1). A pool allocation failure still unwinds through out_free_buffer_pools, so the logical LAN registered just before is freed with h_free_logical_lan() as on the other post-register failures. Error handling is queue-safe: allocation failure unwinds only what that queue allocated (then prior queues in the caller). alloc_buffer_pool() already undoes its own partials, so this open-fail slot is empty. Free paths release by real allocations (free_map/dma_addr/skbuff) rather than pool->active, so a pool is freed whether or not its active flag still matches its memory. close() also reorders pool teardown ahead of cleanup_rx_resources() and filter-list free. napi_disable() and free_irq() have already run, so NAPI no longer references the pool buffers or RX completion queue, and PHYP does not once h_free_logical_lan() has succeeded. A failed free is logged and the memory is still released, as close() did before this patch; the order of the frees does not change that. Keep the legacy 64 KiB pool enabled by default at standard MTU (same as single-queue policy). Signed-off-by: Mingming Cao Reviewed-by: Dave Marquardt Tested-by: Shaik Abdulla --- Changes in v8: - a pool allocation failure in open() unwinds through h_free_logical_lan(), as the merged open-fail fix does Dropped from this series (merged fixes own them): - commit 84bec0bf0352: commit-message shared-TX-index unwind paragraph (base owns the unwind) Changes in v7: - no P04 code change - commit message: request_irq() failure also reached out_free_tx_ltb with i spent - noted: err_cleanup comment reworded in P03 - noted: no Fixes: peel of the shared-i TX LTB leak (named in the v6 body after v5); standalone [PATCH net] after this series - noted: no pool-before-register reorder here; P06 issues h_free on the pool-fail path, P07 allocates pools before register Changes in v6: - retarget the free-by-presence comment to the later resize paths; alloc_buffer_pool() already undoes its own partials, so this open-fail slot is empty - use %u for the two num_rx_queues dbg prints - noted: pool-fail after register without h_free_logical_lan predates this patch; the inline pool loop reached the same labels. P06 issues h_free before the pool DMA; P07 moves pools ahead of register_rx_queues Changes in v5: - vs mailed v4: keep pool_active[] = {1,1,0,0,1} (v4 set the 64 KiB pool inactive). No code delta vs prior patches here; SQ large-receive / CMO stay at the historical baseline. change_mtu does not re-enable pool4 at MTU 1500; MQ memory pressure belongs at scale-up, not SQ default - Unwind / free pools by real allocations (free_map/dma_addr/skbuff), not only pool->active, so open-fail cannot leak partially allocated pools (v4 fail path freed by active and skipped the failing pool) - Clarify kdoc: free paths use allocation presence (not "all active"); queue 1..N metadata copy from queue 0 (v4 said "queues 1-15" while MAX is still 1 here) - Document poolN sysfs size multiplies by num_rx_queues (queue 0 is the shared template; no per-queue pool sysfs in this series) - Call out close() pool-free before cleanup_rx_resources, after LAN/ NAPI/IRQ teardown (order already in v4; spell it for later drain) Changes in v4: - Introduce the pool helpers in the same patch that wires their first open/close callers, instead of leaving unused statics. - Copy pool->index when cloning queue-0 geometry to later queues (needed for correlators; also required by incremental resize). drivers/net/ethernet/ibm/ibmveth.c | 159 +++++++++++++++++++++++++---- 1 file changed, 139 insertions(+), 20 deletions(-) diff --git a/drivers/net/ethernet/ibm/ibmveth.c b/drivers/net/ethernet/ibm/ibmveth.c index 68791402cd53..31fad7713dc7 100644 --- a/drivers/net/ethernet/ibm/ibmveth.c +++ b/drivers/net/ethernet/ibm/ibmveth.c @@ -678,6 +678,140 @@ static void ibmveth_rxq_no_skb(struct ibmveth_adapter *adapter, schedule_work(&adapter->work); } +/** + * ibmveth_free_queue_buffer_pools - Free buffer pools for a single queue + * @adapter: ibmveth adapter structure + * @queue: queue index + * + * Frees buffer pools that still hold allocations for the specified + * queue (by free_map / dma_addr / skbuff presence), regardless of the + * active flag. + */ +static void ibmveth_free_queue_buffer_pools(struct ibmveth_adapter *adapter, + int queue) +{ + int i; + + for (i = 0; i < IBMVETH_NUM_BUFF_POOLS; i++) { + struct ibmveth_buff_pool *pool = + &adapter->rx_buff_pool[queue][i]; + + /* Free by allocation presence, not by the active flag, + * so a pool's memory is released whatever active says. + */ + if (pool->free_map || pool->dma_addr || pool->skbuff) + ibmveth_free_buffer_pool(adapter, pool); + } +} + +/** + * ibmveth_alloc_queue_buffer_pools - Allocate buffer pools for a single queue + * @adapter: ibmveth adapter structure + * @queue: queue index + * + * Allocates backing storage for each active pool on @queue. + * Inactive pools (!active) are skipped. Pool metadata must be + * initialized before calling this function. + * + * Return: 0 on success, negative error code on failure + */ +static int ibmveth_alloc_queue_buffer_pools(struct ibmveth_adapter *adapter, + int queue) +{ + struct net_device *netdev = adapter->netdev; + int i; + + for (i = 0; i < IBMVETH_NUM_BUFF_POOLS; i++) { + struct ibmveth_buff_pool *bpool = + &adapter->rx_buff_pool[queue][i]; + + if (!bpool->active) + continue; + + if (ibmveth_alloc_buffer_pool(bpool)) { + netdev_err(netdev, + "queue %d pool %d alloc failed (size=%u count=%u)\n", + queue, i, + bpool->buff_size, + bpool->size); + bpool->active = 0; + /* Undo the pools this queue already allocated; the + * failed one freed its own partial allocation. + */ + ibmveth_free_queue_buffer_pools(adapter, queue); + return -ENOMEM; + } + } + + return 0; +} + +/** + * ibmveth_alloc_buffer_pools - Allocate buffer pools for all queues + * @adapter: ibmveth adapter structure + * + * Initializes pool metadata for queues 1-N from queue 0 settings, + * then allocates buffer pools for all queues using the helper function. + * + * Return: 0 on success, negative error code on failure + */ +static int +ibmveth_alloc_buffer_pools(struct ibmveth_adapter *adapter) +{ + struct net_device *netdev = adapter->netdev; + int i, q, rc; + + /* Initialize pool metadata for queues 1..N from queue 0 settings */ + for (q = 1; q < adapter->num_rx_queues; q++) { + for (i = 0; i < IBMVETH_NUM_BUFF_POOLS; i++) { + struct ibmveth_buff_pool *src = + &adapter->rx_buff_pool[0][i]; + struct ibmveth_buff_pool *dst = + &adapter->rx_buff_pool[q][i]; + + dst->size = src->size; + dst->index = src->index; + dst->buff_size = src->buff_size; + dst->threshold = src->threshold; + dst->active = src->active; + } + } + + /* Allocate actual buffers for all queues */ + for (q = 0; q < adapter->num_rx_queues; q++) { + rc = ibmveth_alloc_queue_buffer_pools(adapter, q); + if (rc) { + /* Free pools for all previous queues */ + while (--q >= 0) + ibmveth_free_queue_buffer_pools(adapter, q); + return rc; + } + } + + netdev_dbg(netdev, "allocated buffer pools for %u queue(s)\n", + adapter->num_rx_queues); + return 0; +} + +/** + * ibmveth_free_buffer_pools - Free buffer pools for all queues + * @adapter: ibmveth adapter structure + * + * Frees buffer pools for all queues using the helper function. + */ +static void +ibmveth_free_buffer_pools(struct ibmveth_adapter *adapter) +{ + int q; + + /* Free buffer pools for all queues */ + for (q = 0; q < adapter->num_rx_queues; q++) + ibmveth_free_queue_buffer_pools(adapter, q); + + netdev_dbg(adapter->netdev, "freed buffer pools for %u queue(s)\n", + adapter->num_rx_queues); +} + /** * ibmveth_remove_buffer_from_pool - remove a buffer from a pool * @adapter: adapter instance @@ -907,16 +1041,9 @@ static int ibmveth_open(struct net_device *netdev) goto out_free_tx_ltb; } - for (i = 0; i < IBMVETH_NUM_BUFF_POOLS; i++) { - if (!adapter->rx_buff_pool[0][i].active) - continue; - if (ibmveth_alloc_buffer_pool(&adapter->rx_buff_pool[0][i])) { - netdev_err(netdev, "unable to alloc pool\n"); - adapter->rx_buff_pool[0][i].active = 0; - rc = -ENOMEM; - goto out_free_buffer_pools; - } - } + rc = ibmveth_alloc_buffer_pools(adapter); + if (rc) + goto out_free_buffer_pools; /* NAPI can run as soon as it is enabled, from netpoll during the * direct close()/open() pairs or from a direct ibmveth_interrupt() @@ -950,11 +1077,7 @@ static int ibmveth_open(struct net_device *netdev) do { lpar_rc = h_free_logical_lan(adapter->vdev->unit_address); } while (H_IS_LONG_BUSY(lpar_rc) || (lpar_rc == H_BUSY)); - while (--i >= 0) { - if (adapter->rx_buff_pool[0][i].active) - ibmveth_free_buffer_pool(adapter, - &adapter->rx_buff_pool[0][i]); - } + ibmveth_free_buffer_pools(adapter); out_free_tx_ltb: for (i = netdev->real_num_tx_queues - 1; i >= 0; i--) ibmveth_free_tx_ltb(adapter, i); @@ -1005,14 +1128,10 @@ static int ibmveth_close(struct net_device *netdev) ibmveth_update_rx_no_buffer(adapter); + ibmveth_free_buffer_pools(adapter); ibmveth_cleanup_rx_resources(adapter); ibmveth_free_filter_list(adapter); - for (i = 0; i < IBMVETH_NUM_BUFF_POOLS; i++) - if (adapter->rx_buff_pool[0][i].active) - ibmveth_free_buffer_pool(adapter, - &adapter->rx_buff_pool[0][i]); - for (i = 0; i < netdev->real_num_tx_queues; i++) ibmveth_free_tx_ltb(adapter, i); -- 2.50.1 (Apple Git-155)