[PATCH net-next 0/5] ibmvnic: Make driver resources dynamic

[PATCH net-next 0/5] ibmvnic: Make driver resources dynamic

[Nathan Fontenot]

The ibmvnic driver needs to be able to handle the number of tx/rx
sub-crqs changing during a reset of the driver. To do this several
changes need to be made. First the num_active_[tx|rx]_pools
counters need to be re-named to num_active_[tc|rx]_scrqs, and
updated after resource initialization.

With this change we can now release and init the sub crqs and napi
(for rx sub crqs) when the number of sub crqs change.

Lastly, the stats buffer allocation is updated to always allocate
the maximum number of sub-crqs count of stats buffers.

-Nathan
---

Nathan Fontenot (5):
      ibmvnic: Rename active queue count variables
      ibmvnic: Move active sub-crq count settings
      ibmvnic: Free and re-allocate scrqs when tx/rx scrqs change
      ibmvnic: Make napi usage dynamic
      ibmvnic: Allocate max queues stats buffers


 drivers/net/ethernet/ibm/ibmvnic.c |  161 ++++++++++++++++++++++--------------
 drivers/net/ethernet/ibm/ibmvnic.h |    4 -
 2 files changed, 101 insertions(+), 64 deletions(-)

[PATCH net-next1/5] ibmvnic: Rename active queue count variables

[Nathan Fontenot]

Rename the tx/rx active pool variables to be tx/rx active scrq
counts. The tx/rx pools are per sub-crq so this is a more appropriate
name. This also is a preparatory step for using thiese variables
for handling updates to sub-crqs and napi based on the active
count.

Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c |   16 ++++++++--------
 drivers/net/ethernet/ibm/ibmvnic.h |    4 ++--
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 27447260215d..ca2e3fbfd848 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -461,7 +461,7 @@ static void release_rx_pools(struct ibmvnic_adapter *adapter)
 	if (!adapter->rx_pool)
 		return;
 
-	for (i = 0; i < adapter->num_active_rx_pools; i++) {
+	for (i = 0; i < adapter->num_active_rx_scrqs; i++) {
 		rx_pool = &adapter->rx_pool[i];
 
 		netdev_dbg(adapter->netdev, "Releasing rx_pool[%d]\n", i);
@@ -484,7 +484,7 @@ static void release_rx_pools(struct ibmvnic_adapter *adapter)
 
 	kfree(adapter->rx_pool);
 	adapter->rx_pool = NULL;
-	adapter->num_active_rx_pools = 0;
+	adapter->num_active_rx_scrqs = 0;
 }
 
 static int init_rx_pools(struct net_device *netdev)
@@ -509,7 +509,7 @@ static int init_rx_pools(struct net_device *netdev)
 		return -1;
 	}
 
-	adapter->num_active_rx_pools = 0;
+	adapter->num_active_rx_scrqs = 0;
 
 	for (i = 0; i < rxadd_subcrqs; i++) {
 		rx_pool = &adapter->rx_pool[i];
@@ -554,7 +554,7 @@ static int init_rx_pools(struct net_device *netdev)
 		rx_pool->next_free = 0;
 	}
 
-	adapter->num_active_rx_pools = rxadd_subcrqs;
+	adapter->num_active_rx_scrqs = rxadd_subcrqs;
 
 	return 0;
 }
@@ -613,7 +613,7 @@ static void release_tx_pools(struct ibmvnic_adapter *adapter)
 	if (!adapter->tx_pool)
 		return;
 
-	for (i = 0; i < adapter->num_active_tx_pools; i++) {
+	for (i = 0; i < adapter->num_active_tx_scrqs; i++) {
 		netdev_dbg(adapter->netdev, "Releasing tx_pool[%d]\n", i);
 		tx_pool = &adapter->tx_pool[i];
 		kfree(tx_pool->tx_buff);
@@ -624,7 +624,7 @@ static void release_tx_pools(struct ibmvnic_adapter *adapter)
 
 	kfree(adapter->tx_pool);
 	adapter->tx_pool = NULL;
-	adapter->num_active_tx_pools = 0;
+	adapter->num_active_tx_scrqs = 0;
 }
 
 static int init_tx_pools(struct net_device *netdev)
@@ -641,7 +641,7 @@ static int init_tx_pools(struct net_device *netdev)
 	if (!adapter->tx_pool)
 		return -1;
 
-	adapter->num_active_tx_pools = 0;
+	adapter->num_active_tx_scrqs = 0;
 
 	for (i = 0; i < tx_subcrqs; i++) {
 		tx_pool = &adapter->tx_pool[i];
@@ -690,7 +690,7 @@ static int init_tx_pools(struct net_device *netdev)
 		tx_pool->producer_index = 0;
 	}
 
-	adapter->num_active_tx_pools = tx_subcrqs;
+	adapter->num_active_tx_scrqs = tx_subcrqs;
 
 	return 0;
 }
diff --git a/drivers/net/ethernet/ibm/ibmvnic.h b/drivers/net/ethernet/ibm/ibmvnic.h
index fe21a6e2ddae..c6d0b4afe899 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.h
+++ b/drivers/net/ethernet/ibm/ibmvnic.h
@@ -1091,8 +1091,8 @@ struct ibmvnic_adapter {
 	u64 opt_rxba_entries_per_subcrq;
 	__be64 tx_rx_desc_req;
 	u8 map_id;
-	u64 num_active_rx_pools;
-	u64 num_active_tx_pools;
+	u64 num_active_rx_scrqs;
+	u64 num_active_tx_scrqs;
 
 	struct tasklet_struct tasklet;
 	enum vnic_state state;

[PATCH net-next2/5] ibmvnic: Move active sub-crq count settings

[Nathan Fontenot]

Inpreparation for using the active scrq count to track more active
resources, move the setting of the active count to after initialization
occurs in initial driver init and during driver reset.

Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c |   17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index ca2e3fbfd848..9cfbb20b5ac1 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -484,7 +484,6 @@ static void release_rx_pools(struct ibmvnic_adapter *adapter)
 
 	kfree(adapter->rx_pool);
 	adapter->rx_pool = NULL;
-	adapter->num_active_rx_scrqs = 0;
 }
 
 static int init_rx_pools(struct net_device *netdev)
@@ -509,8 +508,6 @@ static int init_rx_pools(struct net_device *netdev)
 		return -1;
 	}
 
-	adapter->num_active_rx_scrqs = 0;
-
 	for (i = 0; i < rxadd_subcrqs; i++) {
 		rx_pool = &adapter->rx_pool[i];
 
@@ -554,8 +551,6 @@ static int init_rx_pools(struct net_device *netdev)
 		rx_pool->next_free = 0;
 	}
 
-	adapter->num_active_rx_scrqs = rxadd_subcrqs;
-
 	return 0;
 }
 
@@ -624,7 +619,6 @@ static void release_tx_pools(struct ibmvnic_adapter *adapter)
 
 	kfree(adapter->tx_pool);
 	adapter->tx_pool = NULL;
-	adapter->num_active_tx_scrqs = 0;
 }
 
 static int init_tx_pools(struct net_device *netdev)
@@ -641,8 +635,6 @@ static int init_tx_pools(struct net_device *netdev)
 	if (!adapter->tx_pool)
 		return -1;
 
-	adapter->num_active_tx_scrqs = 0;
-
 	for (i = 0; i < tx_subcrqs; i++) {
 		tx_pool = &adapter->tx_pool[i];
 
@@ -690,8 +682,6 @@ static int init_tx_pools(struct net_device *netdev)
 		tx_pool->producer_index = 0;
 	}
 
-	adapter->num_active_tx_scrqs = tx_subcrqs;
-
 	return 0;
 }
 
@@ -975,6 +965,10 @@ static int init_resources(struct ibmvnic_adapter *adapter)
 		return rc;
 
 	rc = init_tx_pools(netdev);
+
+	adapter->num_active_tx_scrqs = adapter->req_tx_queues;
+	adapter->num_active_rx_scrqs = adapter->req_rx_queues;
+
 	return rc;
 }
 
@@ -1646,6 +1640,9 @@ static int do_reset(struct ibmvnic_adapter *adapter,
 			release_tx_pools(adapter);
 			init_rx_pools(netdev);
 			init_tx_pools(netdev);
+
+			adapter->num_active_tx_scrqs = adapter->req_tx_queues;
+			adapter->num_active_rx_scrqs = adapter->req_rx_queues;
 		} else {
 			rc = reset_tx_pools(adapter);
 			if (rc)

[PATCH net-next3/5] ibmvnic: Free and re-allocate scrqs when tx/rx scrqs change

[Nathan Fontenot]

When the driver resets it is possible that the number of tx/rx
sub-crqs can change. This patch handles this so that the driver does
not try to access non-existent sub-crqs.

Additionally, a parameter is added to release_sub_crqs() so that
we know if the h_call to free the sub-crq needs to be made. In
the reset path we have to do a reset of the main crq, which is
a free followed by a register of the main crq. The free of main
crq results in all of the sub crq's being free'ed. When updating
sub-crq count in the reset path we do not want to h_free the
sub-crqs, they are already free'ed.

Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c |   69 ++++++++++++++++++++++--------------
 1 file changed, 43 insertions(+), 26 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 9cfbb20b5ac1..a3079d5c072c 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -90,7 +90,7 @@ MODULE_VERSION(IBMVNIC_DRIVER_VERSION);
 
 static int ibmvnic_version = IBMVNIC_INITIAL_VERSION;
 static int ibmvnic_remove(struct vio_dev *);
-static void release_sub_crqs(struct ibmvnic_adapter *);
+static void release_sub_crqs(struct ibmvnic_adapter *, int);
 static int ibmvnic_reset_crq(struct ibmvnic_adapter *);
 static int ibmvnic_send_crq_init(struct ibmvnic_adapter *);
 static int ibmvnic_reenable_crq_queue(struct ibmvnic_adapter *);
@@ -740,7 +740,7 @@ static int ibmvnic_login(struct net_device *netdev)
 	do {
 		if (adapter->renegotiate) {
 			adapter->renegotiate = false;
-			release_sub_crqs(adapter);
+			release_sub_crqs(adapter, 1);
 
 			reinit_completion(&adapter->init_done);
 			send_cap_queries(adapter);
@@ -1602,7 +1602,7 @@ static int do_reset(struct ibmvnic_adapter *adapter,
 	if (adapter->reset_reason == VNIC_RESET_CHANGE_PARAM ||
 	    adapter->wait_for_reset) {
 		release_resources(adapter);
-		release_sub_crqs(adapter);
+		release_sub_crqs(adapter, 1);
 		release_crq_queue(adapter);
 	}
 
@@ -2241,24 +2241,27 @@ static int reset_sub_crq_queues(struct ibmvnic_adapter *adapter)
 }
 
 static void release_sub_crq_queue(struct ibmvnic_adapter *adapter,
-				  struct ibmvnic_sub_crq_queue *scrq)
+				  struct ibmvnic_sub_crq_queue *scrq,
+				  int do_h_free)
 {
 	struct device *dev = &adapter->vdev->dev;
 	long rc;
 
 	netdev_dbg(adapter->netdev, "Releasing sub-CRQ\n");
 
-	/* Close the sub-crqs */
-	do {
-		rc = plpar_hcall_norets(H_FREE_SUB_CRQ,
-					adapter->vdev->unit_address,
-					scrq->crq_num);
-	} while (rc == H_BUSY || H_IS_LONG_BUSY(rc));
+	if (do_h_free) {
+		/* Close the sub-crqs */
+		do {
+			rc = plpar_hcall_norets(H_FREE_SUB_CRQ,
+						adapter->vdev->unit_address,
+						scrq->crq_num);
+		} while (rc == H_BUSY || H_IS_LONG_BUSY(rc));
 
-	if (rc) {
-		netdev_err(adapter->netdev,
-			   "Failed to release sub-CRQ %16lx, rc = %ld\n",
-			   scrq->crq_num, rc);
+		if (rc) {
+			netdev_err(adapter->netdev,
+				   "Failed to release sub-CRQ %16lx, rc = %ld\n",
+				   scrq->crq_num, rc);
+		}
 	}
 
 	dma_unmap_single(dev, scrq->msg_token, 4 * PAGE_SIZE,
@@ -2326,12 +2329,12 @@ static struct ibmvnic_sub_crq_queue *init_sub_crq_queue(struct ibmvnic_adapter
 	return NULL;
 }
 
-static void release_sub_crqs(struct ibmvnic_adapter *adapter)
+static void release_sub_crqs(struct ibmvnic_adapter *adapter, int do_h_free)
 {
 	int i;
 
 	if (adapter->tx_scrq) {
-		for (i = 0; i < adapter->req_tx_queues; i++) {
+		for (i = 0; i < adapter->num_active_tx_scrqs; i++) {
 			if (!adapter->tx_scrq[i])
 				continue;
 
@@ -2344,7 +2347,8 @@ static void release_sub_crqs(struct ibmvnic_adapter *adapter)
 				adapter->tx_scrq[i]->irq = 0;
 			}
 
-			release_sub_crq_queue(adapter, adapter->tx_scrq[i]);
+			release_sub_crq_queue(adapter, adapter->tx_scrq[i],
+					      do_h_free);
 		}
 
 		kfree(adapter->tx_scrq);
@@ -2352,7 +2356,7 @@ static void release_sub_crqs(struct ibmvnic_adapter *adapter)
 	}
 
 	if (adapter->rx_scrq) {
-		for (i = 0; i < adapter->req_rx_queues; i++) {
+		for (i = 0; i < adapter->num_active_rx_scrqs; i++) {
 			if (!adapter->rx_scrq[i])
 				continue;
 
@@ -2365,7 +2369,8 @@ static void release_sub_crqs(struct ibmvnic_adapter *adapter)
 				adapter->rx_scrq[i]->irq = 0;
 			}
 
-			release_sub_crq_queue(adapter, adapter->rx_scrq[i]);
+			release_sub_crq_queue(adapter, adapter->rx_scrq[i],
+					      do_h_free);
 		}
 
 		kfree(adapter->rx_scrq);
@@ -2572,7 +2577,7 @@ static int init_sub_crq_irqs(struct ibmvnic_adapter *adapter)
 		free_irq(adapter->tx_scrq[j]->irq, adapter->tx_scrq[j]);
 		irq_dispose_mapping(adapter->rx_scrq[j]->irq);
 	}
-	release_sub_crqs(adapter);
+	release_sub_crqs(adapter, 1);
 	return rc;
 }
 
@@ -2654,7 +2659,7 @@ static int init_sub_crqs(struct ibmvnic_adapter *adapter)
 	adapter->tx_scrq = NULL;
 tx_failed:
 	for (i = 0; i < registered_queues; i++)
-		release_sub_crq_queue(adapter, allqueues[i]);
+		release_sub_crq_queue(adapter, allqueues[i], 1);
 	kfree(allqueues);
 	return -1;
 }
@@ -4279,6 +4284,7 @@ static int ibmvnic_init(struct ibmvnic_adapter *adapter)
 {
 	struct device *dev = &adapter->vdev->dev;
 	unsigned long timeout = msecs_to_jiffies(30000);
+	u64 old_num_rx_queues, old_num_tx_queues;
 	int rc;
 
 	if (adapter->resetting && !adapter->wait_for_reset) {
@@ -4296,6 +4302,9 @@ static int ibmvnic_init(struct ibmvnic_adapter *adapter)
 
 	adapter->from_passive_init = false;
 
+	old_num_rx_queues = adapter->req_rx_queues;
+	old_num_tx_queues = adapter->req_tx_queues;
+
 	init_completion(&adapter->init_done);
 	adapter->init_done_rc = 0;
 	ibmvnic_send_crq_init(adapter);
@@ -4315,10 +4324,18 @@ static int ibmvnic_init(struct ibmvnic_adapter *adapter)
 		return -1;
 	}
 
-	if (adapter->resetting && !adapter->wait_for_reset)
-		rc = reset_sub_crq_queues(adapter);
-	else
+	if (adapter->resetting && !adapter->wait_for_reset) {
+		if (adapter->req_rx_queues != old_num_rx_queues ||
+		    adapter->req_tx_queues != old_num_tx_queues) {
+			release_sub_crqs(adapter, 0);
+			rc = init_sub_crqs(adapter);
+		} else {
+			rc = reset_sub_crq_queues(adapter);
+		}
+	} else {
 		rc = init_sub_crqs(adapter);
+	}
+
 	if (rc) {
 		dev_err(dev, "Initialization of sub crqs failed\n");
 		release_crq_queue(adapter);
@@ -4418,7 +4435,7 @@ static int ibmvnic_probe(struct vio_dev *dev, const struct vio_device_id *id)
 	device_remove_file(&dev->dev, &dev_attr_failover);
 
 ibmvnic_init_fail:
-	release_sub_crqs(adapter);
+	release_sub_crqs(adapter, 1);
 	release_crq_queue(adapter);
 	free_netdev(netdev);
 
@@ -4435,7 +4452,7 @@ static int ibmvnic_remove(struct vio_dev *dev)
 	mutex_lock(&adapter->reset_lock);
 
 	release_resources(adapter);
-	release_sub_crqs(adapter);
+	release_sub_crqs(adapter, 1);
 	release_crq_queue(adapter);
 
 	adapter->state = VNIC_REMOVED;

[PATCH net-next4/5] ibmvnic: Make napi usage dynamic

[Nathan Fontenot]

In order to handle the number of rx sub crqs changing during a driver
reset, the ibmvnic driver also needs to update the number of napi.
To do this the code to init and free napi's is moved to their own
routines so they can be called during the reset process.

Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c |   67 ++++++++++++++++++++++++------------
 1 file changed, 45 insertions(+), 22 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index a3079d5c072c..f4261f03a522 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -730,6 +730,43 @@ static void ibmvnic_napi_disable(struct ibmvnic_adapter *adapter)
 	adapter->napi_enabled = false;
 }
 
+static int init_napi(struct ibmvnic_adapter *adapter)
+{
+	int i;
+
+	adapter->napi = kcalloc(adapter->req_rx_queues,
+				sizeof(struct napi_struct), GFP_KERNEL);
+	if (!adapter->napi)
+		return -ENOMEM;
+
+	for (i = 0; i < adapter->req_rx_queues; i++) {
+		netdev_dbg(adapter->netdev, "Adding napi[%d]\n", i);
+		netif_napi_add(adapter->netdev, &adapter->napi[i],
+			       ibmvnic_poll, NAPI_POLL_WEIGHT);
+	}
+
+	return 0;
+}
+
+static void release_napi(struct ibmvnic_adapter *adapter)
+{
+	int i;
+
+	if (!adapter->napi)
+		return;
+
+	for (i = 0; i < adapter->num_active_rx_scrqs; i++) {
+		if (&adapter->napi[i]) {
+			netdev_dbg(adapter->netdev,
+				   "Releasing napi[%d]\n", i);
+			netif_napi_del(&adapter->napi[i]);
+		}
+	}
+
+	kfree(adapter->napi);
+	adapter->napi = NULL;
+}
+
 static int ibmvnic_login(struct net_device *netdev)
 {
 	struct ibmvnic_adapter *adapter = netdev_priv(netdev);
@@ -783,8 +820,6 @@ static int ibmvnic_login(struct net_device *netdev)
 
 static void release_resources(struct ibmvnic_adapter *adapter)
 {
-	int i;
-
 	release_vpd_data(adapter);
 
 	release_tx_pools(adapter);
@@ -793,16 +828,7 @@ static void release_resources(struct ibmvnic_adapter *adapter)
 	release_stats_token(adapter);
 	release_stats_buffers(adapter);
 	release_error_buffers(adapter);
-
-	if (adapter->napi) {
-		for (i = 0; i < adapter->req_rx_queues; i++) {
-			if (&adapter->napi[i]) {
-				netdev_dbg(adapter->netdev,
-					   "Releasing napi[%d]\n", i);
-				netif_napi_del(&adapter->napi[i]);
-			}
-		}
-	}
+	release_napi(adapter);
 }
 
 static int set_link_state(struct ibmvnic_adapter *adapter, u8 link_state)
@@ -921,7 +947,7 @@ static int ibmvnic_get_vpd(struct ibmvnic_adapter *adapter)
 static int init_resources(struct ibmvnic_adapter *adapter)
 {
 	struct net_device *netdev = adapter->netdev;
-	int i, rc;
+	int rc;
 
 	rc = set_real_num_queues(netdev);
 	if (rc)
@@ -947,16 +973,10 @@ static int init_resources(struct ibmvnic_adapter *adapter)
 	}
 
 	adapter->map_id = 1;
-	adapter->napi = kcalloc(adapter->req_rx_queues,
-				sizeof(struct napi_struct), GFP_KERNEL);
-	if (!adapter->napi)
-		return -ENOMEM;
 
-	for (i = 0; i < adapter->req_rx_queues; i++) {
-		netdev_dbg(netdev, "Adding napi[%d]\n", i);
-		netif_napi_add(netdev, &adapter->napi[i], ibmvnic_poll,
-			       NAPI_POLL_WEIGHT);
-	}
+	rc = init_napi(adapter);
+	if (rc)
+		return rc;
 
 	send_map_query(adapter);
 
@@ -1641,6 +1661,9 @@ static int do_reset(struct ibmvnic_adapter *adapter,
 			init_rx_pools(netdev);
 			init_tx_pools(netdev);
 
+			release_napi(adapter);
+			init_napi(adapter);
+
 			adapter->num_active_tx_scrqs = adapter->req_tx_queues;
 			adapter->num_active_rx_scrqs = adapter->req_rx_queues;
 		} else {

[PATCH net-next5/5] ibmvnic: Allocate max queues stats buffers

[Nathan Fontenot]

To avoid losing any stats when the number of sub-crqs change, allocate
the max number of stats buffers so a stats buffer exists all possible
sub-crqs.

Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index f4261f03a522..79a0b9bc5edb 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -361,14 +361,14 @@ static void release_stats_buffers(struct ibmvnic_adapter *adapter)
 static int init_stats_buffers(struct ibmvnic_adapter *adapter)
 {
 	adapter->tx_stats_buffers =
-				kcalloc(adapter->req_tx_queues,
+				kcalloc(IBMVNIC_MAX_QUEUES,
 					sizeof(struct ibmvnic_tx_queue_stats),
 					GFP_KERNEL);
 	if (!adapter->tx_stats_buffers)
 		return -ENOMEM;
 
 	adapter->rx_stats_buffers =
-				kcalloc(adapter->req_rx_queues,
+				kcalloc(IBMVNIC_MAX_QUEUES,
 					sizeof(struct ibmvnic_rx_queue_stats),
 					GFP_KERNEL);
 	if (!adapter->rx_stats_buffers)

Re: [PATCH net-next3/5] ibmvnic: Free and re-allocate scrqs when tx/rx scrqs change

[Nathan Fontenot]

On 02/17/2018 05:32 PM, Nathan Fontenot wrote:
> When the driver resets it is possible that the number of tx/rx
> sub-crqs can change. This patch handles this so that the driver does
> not try to access non-existent sub-crqs.
> 
> Additionally, a parameter is added to release_sub_crqs() so that
> we know if the h_call to free the sub-crq needs to be made. In
> the reset path we have to do a reset of the main crq, which is
> a free followed by a register of the main crq. The free of main
> crq results in all of the sub crq's being free'ed. When updating
> sub-crq count in the reset path we do not want to h_free the
> sub-crqs, they are already free'ed.

This patch has a bug in that it does not use the correct queue count
when releasing the sub crqs when the driver is in the probed state.

A version 2 of the patch set will be sent.

-Nathan

> 
> Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
> ---
>  drivers/net/ethernet/ibm/ibmvnic.c |   69 ++++++++++++++++++++++--------------
>  1 file changed, 43 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
> index 9cfbb20b5ac1..a3079d5c072c 100644
> --- a/drivers/net/ethernet/ibm/ibmvnic.c
> +++ b/drivers/net/ethernet/ibm/ibmvnic.c
> @@ -90,7 +90,7 @@ MODULE_VERSION(IBMVNIC_DRIVER_VERSION);
> 
>  static int ibmvnic_version = IBMVNIC_INITIAL_VERSION;
>  static int ibmvnic_remove(struct vio_dev *);
> -static void release_sub_crqs(struct ibmvnic_adapter *);
> +static void release_sub_crqs(struct ibmvnic_adapter *, int);
>  static int ibmvnic_reset_crq(struct ibmvnic_adapter *);
>  static int ibmvnic_send_crq_init(struct ibmvnic_adapter *);
>  static int ibmvnic_reenable_crq_queue(struct ibmvnic_adapter *);
> @@ -740,7 +740,7 @@ static int ibmvnic_login(struct net_device *netdev)
>  	do {
>  		if (adapter->renegotiate) {
>  			adapter->renegotiate = false;
> -			release_sub_crqs(adapter);
> +			release_sub_crqs(adapter, 1);
> 
>  			reinit_completion(&adapter->init_done);
>  			send_cap_queries(adapter);
> @@ -1602,7 +1602,7 @@ static int do_reset(struct ibmvnic_adapter *adapter,
>  	if (adapter->reset_reason == VNIC_RESET_CHANGE_PARAM ||
>  	    adapter->wait_for_reset) {
>  		release_resources(adapter);
> -		release_sub_crqs(adapter);
> +		release_sub_crqs(adapter, 1);
>  		release_crq_queue(adapter);
>  	}
> 
> @@ -2241,24 +2241,27 @@ static int reset_sub_crq_queues(struct ibmvnic_adapter *adapter)
>  }
> 
>  static void release_sub_crq_queue(struct ibmvnic_adapter *adapter,
> -				  struct ibmvnic_sub_crq_queue *scrq)
> +				  struct ibmvnic_sub_crq_queue *scrq,
> +				  int do_h_free)
>  {
>  	struct device *dev = &adapter->vdev->dev;
>  	long rc;
> 
>  	netdev_dbg(adapter->netdev, "Releasing sub-CRQ\n");
> 
> -	/* Close the sub-crqs */
> -	do {
> -		rc = plpar_hcall_norets(H_FREE_SUB_CRQ,
> -					adapter->vdev->unit_address,
> -					scrq->crq_num);
> -	} while (rc == H_BUSY || H_IS_LONG_BUSY(rc));
> +	if (do_h_free) {
> +		/* Close the sub-crqs */
> +		do {
> +			rc = plpar_hcall_norets(H_FREE_SUB_CRQ,
> +						adapter->vdev->unit_address,
> +						scrq->crq_num);
> +		} while (rc == H_BUSY || H_IS_LONG_BUSY(rc));
> 
> -	if (rc) {
> -		netdev_err(adapter->netdev,
> -			   "Failed to release sub-CRQ %16lx, rc = %ld\n",
> -			   scrq->crq_num, rc);
> +		if (rc) {
> +			netdev_err(adapter->netdev,
> +				   "Failed to release sub-CRQ %16lx, rc = %ld\n",
> +				   scrq->crq_num, rc);
> +		}
>  	}
> 
>  	dma_unmap_single(dev, scrq->msg_token, 4 * PAGE_SIZE,
> @@ -2326,12 +2329,12 @@ static struct ibmvnic_sub_crq_queue *init_sub_crq_queue(struct ibmvnic_adapter
>  	return NULL;
>  }
> 
> -static void release_sub_crqs(struct ibmvnic_adapter *adapter)
> +static void release_sub_crqs(struct ibmvnic_adapter *adapter, int do_h_free)
>  {
>  	int i;
> 
>  	if (adapter->tx_scrq) {
> -		for (i = 0; i < adapter->req_tx_queues; i++) {
> +		for (i = 0; i < adapter->num_active_tx_scrqs; i++) {
>  			if (!adapter->tx_scrq[i])
>  				continue;
> 
> @@ -2344,7 +2347,8 @@ static void release_sub_crqs(struct ibmvnic_adapter *adapter)
>  				adapter->tx_scrq[i]->irq = 0;
>  			}
> 
> -			release_sub_crq_queue(adapter, adapter->tx_scrq[i]);
> +			release_sub_crq_queue(adapter, adapter->tx_scrq[i],
> +					      do_h_free);
>  		}
> 
>  		kfree(adapter->tx_scrq);
> @@ -2352,7 +2356,7 @@ static void release_sub_crqs(struct ibmvnic_adapter *adapter)
>  	}
> 
>  	if (adapter->rx_scrq) {
> -		for (i = 0; i < adapter->req_rx_queues; i++) {
> +		for (i = 0; i < adapter->num_active_rx_scrqs; i++) {
>  			if (!adapter->rx_scrq[i])
>  				continue;
> 
> @@ -2365,7 +2369,8 @@ static void release_sub_crqs(struct ibmvnic_adapter *adapter)
>  				adapter->rx_scrq[i]->irq = 0;
>  			}
> 
> -			release_sub_crq_queue(adapter, adapter->rx_scrq[i]);
> +			release_sub_crq_queue(adapter, adapter->rx_scrq[i],
> +					      do_h_free);
>  		}
> 
>  		kfree(adapter->rx_scrq);
> @@ -2572,7 +2577,7 @@ static int init_sub_crq_irqs(struct ibmvnic_adapter *adapter)
>  		free_irq(adapter->tx_scrq[j]->irq, adapter->tx_scrq[j]);
>  		irq_dispose_mapping(adapter->rx_scrq[j]->irq);
>  	}
> -	release_sub_crqs(adapter);
> +	release_sub_crqs(adapter, 1);
>  	return rc;
>  }
> 
> @@ -2654,7 +2659,7 @@ static int init_sub_crqs(struct ibmvnic_adapter *adapter)
>  	adapter->tx_scrq = NULL;
>  tx_failed:
>  	for (i = 0; i < registered_queues; i++)
> -		release_sub_crq_queue(adapter, allqueues[i]);
> +		release_sub_crq_queue(adapter, allqueues[i], 1);
>  	kfree(allqueues);
>  	return -1;
>  }
> @@ -4279,6 +4284,7 @@ static int ibmvnic_init(struct ibmvnic_adapter *adapter)
>  {
>  	struct device *dev = &adapter->vdev->dev;
>  	unsigned long timeout = msecs_to_jiffies(30000);
> +	u64 old_num_rx_queues, old_num_tx_queues;
>  	int rc;
> 
>  	if (adapter->resetting && !adapter->wait_for_reset) {
> @@ -4296,6 +4302,9 @@ static int ibmvnic_init(struct ibmvnic_adapter *adapter)
> 
>  	adapter->from_passive_init = false;
> 
> +	old_num_rx_queues = adapter->req_rx_queues;
> +	old_num_tx_queues = adapter->req_tx_queues;
> +
>  	init_completion(&adapter->init_done);
>  	adapter->init_done_rc = 0;
>  	ibmvnic_send_crq_init(adapter);
> @@ -4315,10 +4324,18 @@ static int ibmvnic_init(struct ibmvnic_adapter *adapter)
>  		return -1;
>  	}
> 
> -	if (adapter->resetting && !adapter->wait_for_reset)
> -		rc = reset_sub_crq_queues(adapter);
> -	else
> +	if (adapter->resetting && !adapter->wait_for_reset) {
> +		if (adapter->req_rx_queues != old_num_rx_queues ||
> +		    adapter->req_tx_queues != old_num_tx_queues) {
> +			release_sub_crqs(adapter, 0);
> +			rc = init_sub_crqs(adapter);
> +		} else {
> +			rc = reset_sub_crq_queues(adapter);
> +		}
> +	} else {
>  		rc = init_sub_crqs(adapter);
> +	}
> +
>  	if (rc) {
>  		dev_err(dev, "Initialization of sub crqs failed\n");
>  		release_crq_queue(adapter);
> @@ -4418,7 +4435,7 @@ static int ibmvnic_probe(struct vio_dev *dev, const struct vio_device_id *id)
>  	device_remove_file(&dev->dev, &dev_attr_failover);
> 
>  ibmvnic_init_fail:
> -	release_sub_crqs(adapter);
> +	release_sub_crqs(adapter, 1);
>  	release_crq_queue(adapter);
>  	free_netdev(netdev);
> 
> @@ -4435,7 +4452,7 @@ static int ibmvnic_remove(struct vio_dev *dev)
>  	mutex_lock(&adapter->reset_lock);
> 
>  	release_resources(adapter);
> -	release_sub_crqs(adapter);
> +	release_sub_crqs(adapter, 1);
>  	release_crq_queue(adapter);
> 
>  	adapter->state = VNIC_REMOVED;
> 

Re: [PATCH net-next3/5] ibmvnic: Free and re-allocate scrqs when tx/rx scrqs change

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3203 bytes --]

Hi Nathan,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net-next/master]
[also build test ERROR on v4.16-rc2 next-20180216]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Nathan-Fontenot/ibmvnic-Free-and-re-allocate-scrqs-when-tx-rx-scrqs-change/20180218-203503
config: powerpc-allmodconfig (attached as .config)
compiler: powerpc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=powerpc 

All errors (new ones prefixed by >>):

   drivers/net//ethernet/ibm/ibmvnic.c: In function 'release_sub_crqs':
>> drivers/net//ethernet/ibm/ibmvnic.c:2340:28: error: 'struct ibmvnic_adapter' has no member named 'num_active_tx_scrqs'; did you mean 'num_active_tx_pools'?
      for (i = 0; i < adapter->num_active_tx_scrqs; i++) {
                               ^~~~~~~~~~~~~~~~~~~
                               num_active_tx_pools
>> drivers/net//ethernet/ibm/ibmvnic.c:2362:28: error: 'struct ibmvnic_adapter' has no member named 'num_active_rx_scrqs'; did you mean 'num_active_rx_pools'?
      for (i = 0; i < adapter->num_active_rx_scrqs; i++) {
                               ^~~~~~~~~~~~~~~~~~~
                               num_active_rx_pools

vim +2340 drivers/net//ethernet/ibm/ibmvnic.c

  2334	
  2335	static void release_sub_crqs(struct ibmvnic_adapter *adapter, int do_h_free)
  2336	{
  2337		int i;
  2338	
  2339		if (adapter->tx_scrq) {
> 2340			for (i = 0; i < adapter->num_active_tx_scrqs; i++) {
  2341				if (!adapter->tx_scrq[i])
  2342					continue;
  2343	
  2344				netdev_dbg(adapter->netdev, "Releasing tx_scrq[%d]\n",
  2345					   i);
  2346				if (adapter->tx_scrq[i]->irq) {
  2347					free_irq(adapter->tx_scrq[i]->irq,
  2348						 adapter->tx_scrq[i]);
  2349					irq_dispose_mapping(adapter->tx_scrq[i]->irq);
  2350					adapter->tx_scrq[i]->irq = 0;
  2351				}
  2352	
  2353				release_sub_crq_queue(adapter, adapter->tx_scrq[i],
  2354						      do_h_free);
  2355			}
  2356	
  2357			kfree(adapter->tx_scrq);
  2358			adapter->tx_scrq = NULL;
  2359		}
  2360	
  2361		if (adapter->rx_scrq) {
> 2362			for (i = 0; i < adapter->num_active_rx_scrqs; i++) {
  2363				if (!adapter->rx_scrq[i])
  2364					continue;
  2365	
  2366				netdev_dbg(adapter->netdev, "Releasing rx_scrq[%d]\n",
  2367					   i);
  2368				if (adapter->rx_scrq[i]->irq) {
  2369					free_irq(adapter->rx_scrq[i]->irq,
  2370						 adapter->rx_scrq[i]);
  2371					irq_dispose_mapping(adapter->rx_scrq[i]->irq);
  2372					adapter->rx_scrq[i]->irq = 0;
  2373				}
  2374	
  2375				release_sub_crq_queue(adapter, adapter->rx_scrq[i],
  2376						      do_h_free);
  2377			}
  2378	
  2379			kfree(adapter->rx_scrq);
  2380			adapter->rx_scrq = NULL;
  2381		}
  2382	}
  2383	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 56628 bytes --]