ibmveth_open() allocates the filter list and every RX queue inline.
That is already a long sequence and would get uglier once we loop over
num_rx_queues, especially on error unwind.

Pull the RX bits into helpers and wire them into open()/close() in the
same patch:

  ibmveth_alloc_filter_list() / ibmveth_free_filter_list()
    - shared multicast filter list (one per adapter, not per queue)

  ibmveth_alloc_rx_queues() / ibmveth_cleanup_rx_resources()
    - per-queue buffer lists and RX rings, looping [0, num_rx_queues)

alloc_rx_queues() rolls back on failure so open() does not need nested
goto chains for every queue index. open-failure and close release the
same resources through the same helpers.

Runtime behavior stays single-queue (num_rx_queues is still 1). Buffer
pools, IRQ, TX LTB, and PHYP registration remain inline for later
helper patches.

Also set rc = -ENOMEM before the TX LTB allocation loop so a failed
ibmveth_allocate_tx_ltb() still returns a useful errno after the RX
allocation blocks move into helpers.

Signed-off-by: Mingming Cao <[email protected]>
Reviewed-by: Dave Marquardt <[email protected]>
Tested-by: Shaik Abdulla <[email protected]>
---

Changes in v4:
- Introduce RX/filter allocation helpers in the same patch that wires
  their first open/close callers; v3 left unused statics ahead of the
  old open/close pipeline patch.
- Preserve correct -ENOMEM return on TX LTB allocation failure after
  the RX helper extract.
- Drop reliance on v3's separate "open/close pipeline" patch for this
  wiring.

 drivers/net/ethernet/ibm/ibmveth.c | 268 ++++++++++++++++++++---------
 1 file changed, 190 insertions(+), 78 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmveth.c 
b/drivers/net/ethernet/ibm/ibmveth.c
index 8e758362cb26..1007dd95cde0 100644
--- a/drivers/net/ethernet/ibm/ibmveth.c
+++ b/drivers/net/ethernet/ibm/ibmveth.c
@@ -151,6 +151,184 @@ static unsigned int ibmveth_real_max_tx_queues(void)
        return min(n_cpu, IBMVETH_MAX_QUEUES);
 }
 
+/**
+ * ibmveth_alloc_filter_list - Allocate and map filter list
+ * @adapter: ibmveth adapter structure
+ *
+ * Return: 0 on success, negative error code on failure
+ */
+static int
+ibmveth_alloc_filter_list(struct ibmveth_adapter *adapter)
+{
+       struct device *dev = &adapter->vdev->dev;
+       struct net_device *netdev = adapter->netdev;
+
+       adapter->filter_list_addr = (void *)get_zeroed_page(GFP_KERNEL);
+       if (!adapter->filter_list_addr) {
+               netdev_err(netdev, "unable to allocate filter pages\n");
+               return -ENOMEM;
+       }
+
+       adapter->filter_list_dma = dma_map_single(dev,
+                                                 adapter->filter_list_addr,
+                                                 4096, DMA_BIDIRECTIONAL);
+       if (dma_mapping_error(dev, adapter->filter_list_dma)) {
+               netdev_err(netdev, "unable to map filter list pages\n");
+               free_page((unsigned long)adapter->filter_list_addr);
+               adapter->filter_list_addr = NULL;
+               return -ENOMEM;
+       }
+
+       netdev_dbg(netdev, "filter list @ 0x%p (DMA: 0x%llx)\n",
+                  adapter->filter_list_addr,
+                  (unsigned long long)adapter->filter_list_dma);
+
+       return 0;
+}
+
+/**
+ * ibmveth_free_filter_list - Free filter list resources
+ * @adapter: ibmveth adapter structure
+ */
+static void
+ibmveth_free_filter_list(struct ibmveth_adapter *adapter)
+{
+       struct device *dev = &adapter->vdev->dev;
+
+       if (adapter->filter_list_dma) {
+               dma_unmap_single(dev, adapter->filter_list_dma, 4096,
+                                DMA_BIDIRECTIONAL);
+               adapter->filter_list_dma = 0;
+       }
+
+       if (adapter->filter_list_addr) {
+               free_page((unsigned long)adapter->filter_list_addr);
+               adapter->filter_list_addr = NULL;
+       }
+}
+
+/**
+ * ibmveth_alloc_rx_queues - Allocate per-queue RX resources
+ * @adapter: ibmveth adapter structure
+ * @rxq_entries: Number of entries per RX queue
+ *
+ * Return: 0 on success, negative error code on failure
+ */
+static int
+ibmveth_alloc_rx_queues(struct ibmveth_adapter *adapter, int rxq_entries)
+{
+       struct device *dev = &adapter->vdev->dev;
+       struct net_device *netdev = adapter->netdev;
+       int i;
+
+       for (i = 0; i < adapter->num_rx_queues; i++) {
+               adapter->buffer_list_addr[i] =
+                       (void *)get_zeroed_page(GFP_KERNEL);
+               if (!adapter->buffer_list_addr[i]) {
+                       netdev_err(netdev,
+                                  "unable to allocate buffer list for queue 
%d\n",
+                                  i);
+                       goto err_cleanup;
+               }
+
+               adapter->rx_queue[i].queue_len =
+                       sizeof(struct ibmveth_rx_q_entry) * rxq_entries;
+               adapter->rx_queue[i].queue_addr =
+                       dma_alloc_coherent(dev, adapter->rx_queue[i].queue_len,
+                                          &adapter->rx_queue[i].queue_dma,
+                                          GFP_KERNEL);
+               if (!adapter->rx_queue[i].queue_addr) {
+                       netdev_err(netdev,
+                                  "unable to allocate RX queue for queue %d\n",
+                                  i);
+                       goto err_cleanup;
+               }
+
+               adapter->buffer_list_dma[i] =
+                       dma_map_single(dev, adapter->buffer_list_addr[i],
+                                      4096, DMA_BIDIRECTIONAL);
+               if (dma_mapping_error(dev, adapter->buffer_list_dma[i])) {
+                       netdev_err(netdev,
+                                  "unable to map buffer list for queue %d\n",
+                                  i);
+                       adapter->buffer_list_dma[i] = 0;
+                       goto err_cleanup;
+               }
+
+               adapter->rx_queue[i].index = 0;
+               adapter->rx_queue[i].num_slots = rxq_entries;
+               adapter->rx_queue[i].toggle = 1;
+
+               netdev_dbg(netdev, "queue %d: buffer_list @ 0x%p (DMA: 0x%llx), 
rx_queue @ 0x%p (DMA: 0x%llx), %llu entries\n",
+                          i, adapter->buffer_list_addr[i],
+                          (unsigned long long)adapter->buffer_list_dma[i],
+                          adapter->rx_queue[i].queue_addr,
+                          (unsigned long long)adapter->rx_queue[i].queue_dma,
+                          (unsigned long long)rxq_entries);
+       }
+
+       netdev_dbg(netdev, "allocated %d RX queue(s) with %d entries each\n",
+                  adapter->num_rx_queues, rxq_entries);
+
+       return 0;
+
+err_cleanup:
+       /* Clean up previously allocated queues */
+       for (; i >= 0; i--) {
+               if (adapter->buffer_list_dma[i]) {
+                       dma_unmap_single(dev, adapter->buffer_list_dma[i],
+                                        4096, DMA_BIDIRECTIONAL);
+                       adapter->buffer_list_dma[i] = 0;
+               }
+               if (adapter->rx_queue[i].queue_addr) {
+                       dma_free_coherent(dev, adapter->rx_queue[i].queue_len,
+                                         adapter->rx_queue[i].queue_addr,
+                                         adapter->rx_queue[i].queue_dma);
+                       adapter->rx_queue[i].queue_addr = NULL;
+               }
+               if (adapter->buffer_list_addr[i]) {
+                       free_page((unsigned long)adapter->buffer_list_addr[i]);
+                       adapter->buffer_list_addr[i] = NULL;
+               }
+       }
+
+       return -ENOMEM;
+}
+
+/**
+ * ibmveth_cleanup_rx_resources - Free all RX queue resources
+ * @adapter: ibmveth adapter structure
+ */
+static void
+ibmveth_cleanup_rx_resources(struct ibmveth_adapter *adapter)
+{
+       struct device *dev = &adapter->vdev->dev;
+       int i;
+
+       netdev_dbg(adapter->netdev, "cleaning up %d RX queue(s)\n",
+                  adapter->num_rx_queues);
+
+       for (i = 0; i < adapter->num_rx_queues; i++) {
+               if (adapter->buffer_list_dma[i]) {
+                       dma_unmap_single(dev, adapter->buffer_list_dma[i],
+                                        4096, DMA_BIDIRECTIONAL);
+                       adapter->buffer_list_dma[i] = 0;
+               }
+
+               if (adapter->rx_queue[i].queue_addr) {
+                       dma_free_coherent(dev, adapter->rx_queue[i].queue_len,
+                                         adapter->rx_queue[i].queue_addr,
+                                         adapter->rx_queue[i].queue_dma);
+                       adapter->rx_queue[i].queue_addr = NULL;
+               }
+
+               if (adapter->buffer_list_addr[i]) {
+                       free_page((unsigned long)adapter->buffer_list_addr[i]);
+                       adapter->buffer_list_addr[i] = NULL;
+               }
+       }
+}
+
 /* setup the initial settings for a buffer pool */
 static void ibmveth_init_buffer_pool(struct ibmveth_buff_pool *pool,
                                     u32 pool_index, u32 pool_size,
@@ -627,74 +805,34 @@ static int ibmveth_open(struct net_device *netdev)
        int rc;
        union ibmveth_buf_desc rxq_desc;
        int i;
-       struct device *dev;
 
        netdev_dbg(netdev, "open starting\n");
 
        napi_enable(&adapter->napi[0]);
 
-       for(i = 0; i < IBMVETH_NUM_BUFF_POOLS; i++)
+       for (i = 0; i < IBMVETH_NUM_BUFF_POOLS; i++)
                rxq_entries += adapter->rx_buff_pool[0][i].size;
 
-       rc = -ENOMEM;
-       adapter->buffer_list_addr[0] = (void *)get_zeroed_page(GFP_KERNEL);
-       if (!adapter->buffer_list_addr[0]) {
-               netdev_err(netdev, "unable to allocate list pages\n");
+       rc = ibmveth_alloc_filter_list(adapter);
+       if (rc)
                goto out;
-       }
 
-       adapter->filter_list_addr = (void*) get_zeroed_page(GFP_KERNEL);
-       if (!adapter->filter_list_addr) {
-               netdev_err(netdev, "unable to allocate filter pages\n");
-               goto out_free_buffer_list;
-       }
-
-       dev = &adapter->vdev->dev;
-
-       adapter->rx_queue[0].queue_len = sizeof(struct ibmveth_rx_q_entry) *
-                                               rxq_entries;
-       adapter->rx_queue[0].queue_addr =
-               dma_alloc_coherent(dev, adapter->rx_queue[0].queue_len,
-                                  &adapter->rx_queue[0].queue_dma, GFP_KERNEL);
-       if (!adapter->rx_queue[0].queue_addr)
+       rc = ibmveth_alloc_rx_queues(adapter, rxq_entries);
+       if (rc)
                goto out_free_filter_list;
 
-       adapter->buffer_list_dma[0] =
-               dma_map_single(dev, adapter->buffer_list_addr[0],
-                              4096, DMA_BIDIRECTIONAL);
-       if (dma_mapping_error(dev, adapter->buffer_list_dma[0])) {
-               netdev_err(netdev, "unable to map buffer list pages\n");
-               goto out_free_queue_mem;
-       }
-
-       adapter->filter_list_dma = dma_map_single(dev,
-                       adapter->filter_list_addr, 4096, DMA_BIDIRECTIONAL);
-       if (dma_mapping_error(dev, adapter->filter_list_dma)) {
-               netdev_err(netdev, "unable to map filter list pages\n");
-               goto out_unmap_buffer_list;
-       }
-
+       rc = -ENOMEM;
        for (i = 0; i < netdev->real_num_tx_queues; i++) {
                if (ibmveth_allocate_tx_ltb(adapter, i))
                        goto out_free_tx_ltb;
        }
 
-       adapter->rx_queue[0].index = 0;
-       adapter->rx_queue[0].num_slots = rxq_entries;
-       adapter->rx_queue[0].toggle = 1;
-
        mac_address = ether_addr_to_u64(netdev->dev_addr);
 
        rxq_desc.fields.flags_len = IBMVETH_BUF_VALID |
                                        adapter->rx_queue[0].queue_len;
        rxq_desc.fields.address = adapter->rx_queue[0].queue_dma;
 
-       netdev_dbg(netdev, "buffer list @ 0x%p\n",
-                  adapter->buffer_list_addr[0]);
-       netdev_dbg(netdev, "filter list @ 0x%p\n", adapter->filter_list_addr);
-       netdev_dbg(netdev, "receive q   @ 0x%p\n",
-                  adapter->rx_queue[0].queue_addr);
-
        h_vio_signal(adapter->vdev->unit_address, VIO_IRQ_DISABLE);
 
        lpar_rc = ibmveth_register_logical_lan(adapter, rxq_desc, mac_address);
@@ -709,7 +847,7 @@ static int ibmveth_open(struct net_device *netdev)
                                     rxq_desc.desc,
                                     mac_address);
                rc = -ENONET;
-               goto out_unmap_filter_list;
+               goto out_free_tx_ltb;
        }
 
        for (i = 0; i < IBMVETH_NUM_BUFF_POOLS; i++) {
@@ -736,8 +874,6 @@ static int ibmveth_open(struct net_device *netdev)
                goto out_free_buffer_pools;
        }
 
-       rc = -ENOMEM;
-
        netdev_dbg(netdev, "initial replenish cycle\n");
        ibmveth_interrupt(netdev->irq, netdev);
 
@@ -753,26 +889,12 @@ static int ibmveth_open(struct net_device *netdev)
                        ibmveth_free_buffer_pool(adapter,
                                                 &adapter->rx_buff_pool[0][i]);
        }
-out_unmap_filter_list:
-       dma_unmap_single(dev, adapter->filter_list_dma, 4096,
-                        DMA_BIDIRECTIONAL);
-
 out_free_tx_ltb:
-       while (--i >= 0) {
+       while (--i >= 0)
                ibmveth_free_tx_ltb(adapter, i);
-       }
-
-out_unmap_buffer_list:
-       dma_unmap_single(dev, adapter->buffer_list_dma[0], 4096,
-                        DMA_BIDIRECTIONAL);
-out_free_queue_mem:
-       dma_free_coherent(dev, adapter->rx_queue[0].queue_len,
-                         adapter->rx_queue[0].queue_addr,
-                         adapter->rx_queue[0].queue_dma);
+       ibmveth_cleanup_rx_resources(adapter);
 out_free_filter_list:
-       free_page((unsigned long)adapter->filter_list_addr);
-out_free_buffer_list:
-       free_page((unsigned long)adapter->buffer_list_addr[0]);
+       ibmveth_free_filter_list(adapter);
 out:
        napi_disable(&adapter->napi[0]);
        return rc;
@@ -781,7 +903,6 @@ static int ibmveth_open(struct net_device *netdev)
 static int ibmveth_close(struct net_device *netdev)
 {
        struct ibmveth_adapter *adapter = netdev_priv(netdev);
-       struct device *dev = &adapter->vdev->dev;
        long lpar_rc;
        int i;
 
@@ -806,17 +927,8 @@ static int ibmveth_close(struct net_device *netdev)
 
        ibmveth_update_rx_no_buffer(adapter);
 
-       dma_unmap_single(dev, adapter->buffer_list_dma[0], 4096,
-                        DMA_BIDIRECTIONAL);
-       free_page((unsigned long)adapter->buffer_list_addr[0]);
-
-       dma_unmap_single(dev, adapter->filter_list_dma, 4096,
-                        DMA_BIDIRECTIONAL);
-       free_page((unsigned long)adapter->filter_list_addr);
-
-       dma_free_coherent(dev, adapter->rx_queue[0].queue_len,
-                         adapter->rx_queue[0].queue_addr,
-                         adapter->rx_queue[0].queue_dma);
+       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)
-- 
2.50.1 (Apple Git-155)


Reply via email to