diff mbox series

[net-next,7/9] ibmvnic: Reuse LTB when possible

Message ID 20210901000812.120968-8-sukadev@linux.ibm.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series ibmvnic: Reuse ltb, rx, tx pools | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for net-next
netdev/subject_prefix success Link
netdev/cc_maintainers warning 7 maintainers not CCed: mpe@ellerman.id.au paulus@samba.org tlfalcon@linux.ibm.com davem@davemloft.net linuxppc-dev@lists.ozlabs.org kuba@kernel.org benh@kernel.crashing.org
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/kdoc fail Errors and warnings before: 4 this patch: 8
netdev/verify_fixes success Link
netdev/checkpatch warning WARNING: Possible unnecessary 'out of memory' message
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/header_inline success Link

Commit Message

Sukadev Bhattiprolu Sept. 1, 2021, 12:08 a.m. UTC 
Reuse the long term buffer during a reset as long as its size has
not changed. If the size has changed, free it and allocate a new
one of the appropriate size.

When we do this, alloc_long_term_buff() and reset_long_term_buff()
become identical. Drop reset_long_term_buff().

Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 122 ++++++++++++++---------------
 1 file changed, 59 insertions(+), 63 deletions(-)

Comments

Dany Madden Sept. 1, 2021, 1:34 a.m. UTC | #1 
On 2021-08-31 17:08, Sukadev Bhattiprolu wrote:
> Reuse the long term buffer during a reset as long as its size has
> not changed. If the size has changed, free it and allocate a new
> one of the appropriate size.
> 
> When we do this, alloc_long_term_buff() and reset_long_term_buff()
> become identical. Drop reset_long_term_buff().
> 
> Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.ibm.com>
Reviewed-by: Dany Madden <drt@linux.ibm.com>

> ---
>  drivers/net/ethernet/ibm/ibmvnic.c | 122 ++++++++++++++---------------
>  1 file changed, 59 insertions(+), 63 deletions(-)
> 
> diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
> b/drivers/net/ethernet/ibm/ibmvnic.c
> index 30153a8bb5ec..1bb5996c4313 100644
> --- a/drivers/net/ethernet/ibm/ibmvnic.c
> +++ b/drivers/net/ethernet/ibm/ibmvnic.c
> @@ -108,6 +108,8 @@ static int init_crq_queue(struct ibmvnic_adapter 
> *adapter);
>  static int send_query_phys_parms(struct ibmvnic_adapter *adapter);
>  static void ibmvnic_tx_scrq_clean_buffer(struct ibmvnic_adapter 
> *adapter,
>  					 struct ibmvnic_sub_crq_queue *tx_scrq);
> +static void free_long_term_buff(struct ibmvnic_adapter *adapter,
> +				struct ibmvnic_long_term_buff *ltb);
> 
>  struct ibmvnic_stat {
>  	char name[ETH_GSTRING_LEN];
> @@ -214,23 +216,62 @@ static int ibmvnic_wait_for_completion(struct
> ibmvnic_adapter *adapter,
>  	return -ETIMEDOUT;
>  }
> 
> +/**
> + * Reuse long term buffer unless size has changed.
> + */
> +static bool reuse_ltb(struct ibmvnic_long_term_buff *ltb, int size)
> +{
> +	return (ltb->buff && ltb->size == size);
> +}
> +
> +/**
> + * Allocate a long term buffer of the specified size and notify VIOS.
> + *
> + * If the given @ltb already has the correct size, reuse it. Otherwise 
> if
> + * its non-NULL, free it. Then allocate a new one of the correct size.
> + * Notify the VIOS either way since we may now be working with a new 
> VIOS.
> + *
> + * Allocating larger chunks of memory during resets, specially LPM or 
> under
> + * low memory situations can cause resets to fail/timeout and for LPAR 
> to
> + * lose connectivity. So hold onto the LTB even if we fail to 
> communicate
> + * with the VIOS and reuse it on next open. Free LTB when adapter is 
> closed.
> + */
>  static int alloc_long_term_buff(struct ibmvnic_adapter *adapter,
>  				struct ibmvnic_long_term_buff *ltb, int size)
>  {
>  	struct device *dev = &adapter->vdev->dev;
>  	int rc;
> 
> -	ltb->size = size;
> -	ltb->buff = dma_alloc_coherent(dev, ltb->size, &ltb->addr,
> -				       GFP_KERNEL);
> +	if (!reuse_ltb(ltb, size)) {
> +		dev_dbg(dev,
> +			"LTB size changed from 0x%llx to 0x%x, reallocating\n",
> +			 ltb->size, size);
> +		free_long_term_buff(adapter, ltb);
> +	}
> 
> -	if (!ltb->buff) {
> -		dev_err(dev, "Couldn't alloc long term buffer\n");
> -		return -ENOMEM;
> +	if (ltb->buff) {
> +		dev_dbg(dev, "Reusing LTB [map %d, size 0x%llx]\n",
> +			ltb->map_id, ltb->size);
> +	} else {
> +		ltb->buff = dma_alloc_coherent(dev, size, &ltb->addr,
> +					       GFP_KERNEL);
> +		if (!ltb->buff) {
> +			dev_err(dev, "Couldn't alloc long term buffer\n");
> +			return -ENOMEM;
> +		}
> +		ltb->size = size;
> +
> +		ltb->map_id = find_first_zero_bit(adapter->map_ids,
> +						  MAX_MAP_ID);
> +		bitmap_set(adapter->map_ids, ltb->map_id, 1);
> +
> +		dev_dbg(dev,
> +			"Allocated new LTB [map %d, size 0x%llx]\n",
> +			 ltb->map_id, ltb->size);
>  	}
> -	ltb->map_id = find_first_zero_bit(adapter->map_ids,
> -					  MAX_MAP_ID);
> -	bitmap_set(adapter->map_ids, ltb->map_id, 1);
> +
> +	/* Ensure ltb is zeroed - specially when reusing it. */
> +	memset(ltb->buff, 0, ltb->size);
> 
>  	mutex_lock(&adapter->fw_lock);
>  	adapter->fw_done_rc = 0;
> @@ -257,10 +298,7 @@ static int alloc_long_term_buff(struct
> ibmvnic_adapter *adapter,
>  	}
>  	rc = 0;
>  out:
> -	if (rc) {
> -		dma_free_coherent(dev, ltb->size, ltb->buff, ltb->addr);
> -		ltb->buff = NULL;
> -	}
> +	/* don't free LTB on communication error - see function header */
>  	mutex_unlock(&adapter->fw_lock);
>  	return rc;
>  }
> @@ -290,43 +328,6 @@ static void free_long_term_buff(struct
> ibmvnic_adapter *adapter,
>  	ltb->map_id = 0;
>  }
> 
> -static int reset_long_term_buff(struct ibmvnic_adapter *adapter,
> -				struct ibmvnic_long_term_buff *ltb)
> -{
> -	struct device *dev = &adapter->vdev->dev;
> -	int rc;
> -
> -	memset(ltb->buff, 0, ltb->size);
> -
> -	mutex_lock(&adapter->fw_lock);
> -	adapter->fw_done_rc = 0;
> -
> -	reinit_completion(&adapter->fw_done);
> -	rc = send_request_map(adapter, ltb->addr, ltb->size, ltb->map_id);
> -	if (rc) {
> -		mutex_unlock(&adapter->fw_lock);
> -		return rc;
> -	}
> -
> -	rc = ibmvnic_wait_for_completion(adapter, &adapter->fw_done, 10000);
> -	if (rc) {
> -		dev_info(dev,
> -			 "Reset failed, long term map request timed out or aborted\n");
> -		mutex_unlock(&adapter->fw_lock);
> -		return rc;
> -	}
> -
> -	if (adapter->fw_done_rc) {
> -		dev_info(dev,
> -			 "Reset failed, attempting to free and reallocate buffer\n");
> -		free_long_term_buff(adapter, ltb);
> -		mutex_unlock(&adapter->fw_lock);
> -		return alloc_long_term_buff(adapter, ltb, ltb->size);
> -	}
> -	mutex_unlock(&adapter->fw_lock);
> -	return 0;
> -}
> -
>  static void deactivate_rx_pools(struct ibmvnic_adapter *adapter)
>  {
>  	int i;
> @@ -548,18 +549,10 @@ static int reset_rx_pools(struct ibmvnic_adapter 
> *adapter)
> 
>  		netdev_dbg(adapter->netdev, "Re-setting rx_pool[%d]\n", i);
> 
> -		if (rx_pool->buff_size != buff_size) {
> -			free_long_term_buff(adapter, &rx_pool->long_term_buff);
> -			rx_pool->buff_size = ALIGN(buff_size, L1_CACHE_BYTES);
> -			rc = alloc_long_term_buff(adapter,
> -						  &rx_pool->long_term_buff,
> -						  rx_pool->size *
> -						  rx_pool->buff_size);
> -		} else {
> -			rc = reset_long_term_buff(adapter,
> -						  &rx_pool->long_term_buff);
> -		}
> -
> +		rx_pool->buff_size = ALIGN(buff_size, L1_CACHE_BYTES);
> +		rc = alloc_long_term_buff(adapter,
> +					  &rx_pool->long_term_buff,
> +					  rx_pool->size * rx_pool->buff_size);
>  		if (rc)
>  			return rc;
> 
> @@ -692,9 +685,12 @@ static int init_rx_pools(struct net_device 
> *netdev)
>  static int reset_one_tx_pool(struct ibmvnic_adapter *adapter,
>  			     struct ibmvnic_tx_pool *tx_pool)
>  {
> +	struct ibmvnic_long_term_buff *ltb;
>  	int rc, i;
> 
> -	rc = reset_long_term_buff(adapter, &tx_pool->long_term_buff);
> +	ltb = &tx_pool->long_term_buff;
> +
> +	rc = alloc_long_term_buff(adapter, ltb, ltb->size);
>  	if (rc)
>  		return rc;
diff mbox series

Patch

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 30153a8bb5ec..1bb5996c4313 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -108,6 +108,8 @@  static int init_crq_queue(struct ibmvnic_adapter *adapter);
 static int send_query_phys_parms(struct ibmvnic_adapter *adapter);
 static void ibmvnic_tx_scrq_clean_buffer(struct ibmvnic_adapter *adapter,
 					 struct ibmvnic_sub_crq_queue *tx_scrq);
+static void free_long_term_buff(struct ibmvnic_adapter *adapter,
+				struct ibmvnic_long_term_buff *ltb);
 
 struct ibmvnic_stat {
 	char name[ETH_GSTRING_LEN];
@@ -214,23 +216,62 @@  static int ibmvnic_wait_for_completion(struct ibmvnic_adapter *adapter,
 	return -ETIMEDOUT;
 }
 
+/**
+ * Reuse long term buffer unless size has changed.
+ */
+static bool reuse_ltb(struct ibmvnic_long_term_buff *ltb, int size)
+{
+	return (ltb->buff && ltb->size == size);
+}
+
+/**
+ * Allocate a long term buffer of the specified size and notify VIOS.
+ *
+ * If the given @ltb already has the correct size, reuse it. Otherwise if
+ * its non-NULL, free it. Then allocate a new one of the correct size.
+ * Notify the VIOS either way since we may now be working with a new VIOS.
+ *
+ * Allocating larger chunks of memory during resets, specially LPM or under
+ * low memory situations can cause resets to fail/timeout and for LPAR to
+ * lose connectivity. So hold onto the LTB even if we fail to communicate
+ * with the VIOS and reuse it on next open. Free LTB when adapter is closed.
+ */
 static int alloc_long_term_buff(struct ibmvnic_adapter *adapter,
 				struct ibmvnic_long_term_buff *ltb, int size)
 {
 	struct device *dev = &adapter->vdev->dev;
 	int rc;
 
-	ltb->size = size;
-	ltb->buff = dma_alloc_coherent(dev, ltb->size, &ltb->addr,
-				       GFP_KERNEL);
+	if (!reuse_ltb(ltb, size)) {
+		dev_dbg(dev,
+			"LTB size changed from 0x%llx to 0x%x, reallocating\n",
+			 ltb->size, size);
+		free_long_term_buff(adapter, ltb);
+	}
 
-	if (!ltb->buff) {
-		dev_err(dev, "Couldn't alloc long term buffer\n");
-		return -ENOMEM;
+	if (ltb->buff) {
+		dev_dbg(dev, "Reusing LTB [map %d, size 0x%llx]\n",
+			ltb->map_id, ltb->size);
+	} else {
+		ltb->buff = dma_alloc_coherent(dev, size, &ltb->addr,
+					       GFP_KERNEL);
+		if (!ltb->buff) {
+			dev_err(dev, "Couldn't alloc long term buffer\n");
+			return -ENOMEM;
+		}
+		ltb->size = size;
+
+		ltb->map_id = find_first_zero_bit(adapter->map_ids,
+						  MAX_MAP_ID);
+		bitmap_set(adapter->map_ids, ltb->map_id, 1);
+
+		dev_dbg(dev,
+			"Allocated new LTB [map %d, size 0x%llx]\n",
+			 ltb->map_id, ltb->size);
 	}
-	ltb->map_id = find_first_zero_bit(adapter->map_ids,
-					  MAX_MAP_ID);
-	bitmap_set(adapter->map_ids, ltb->map_id, 1);
+
+	/* Ensure ltb is zeroed - specially when reusing it. */
+	memset(ltb->buff, 0, ltb->size);
 
 	mutex_lock(&adapter->fw_lock);
 	adapter->fw_done_rc = 0;
@@ -257,10 +298,7 @@  static int alloc_long_term_buff(struct ibmvnic_adapter *adapter,
 	}
 	rc = 0;
 out:
-	if (rc) {
-		dma_free_coherent(dev, ltb->size, ltb->buff, ltb->addr);
-		ltb->buff = NULL;
-	}
+	/* don't free LTB on communication error - see function header */
 	mutex_unlock(&adapter->fw_lock);
 	return rc;
 }
@@ -290,43 +328,6 @@  static void free_long_term_buff(struct ibmvnic_adapter *adapter,
 	ltb->map_id = 0;
 }
 
-static int reset_long_term_buff(struct ibmvnic_adapter *adapter,
-				struct ibmvnic_long_term_buff *ltb)
-{
-	struct device *dev = &adapter->vdev->dev;
-	int rc;
-
-	memset(ltb->buff, 0, ltb->size);
-
-	mutex_lock(&adapter->fw_lock);
-	adapter->fw_done_rc = 0;
-
-	reinit_completion(&adapter->fw_done);
-	rc = send_request_map(adapter, ltb->addr, ltb->size, ltb->map_id);
-	if (rc) {
-		mutex_unlock(&adapter->fw_lock);
-		return rc;
-	}
-
-	rc = ibmvnic_wait_for_completion(adapter, &adapter->fw_done, 10000);
-	if (rc) {
-		dev_info(dev,
-			 "Reset failed, long term map request timed out or aborted\n");
-		mutex_unlock(&adapter->fw_lock);
-		return rc;
-	}
-
-	if (adapter->fw_done_rc) {
-		dev_info(dev,
-			 "Reset failed, attempting to free and reallocate buffer\n");
-		free_long_term_buff(adapter, ltb);
-		mutex_unlock(&adapter->fw_lock);
-		return alloc_long_term_buff(adapter, ltb, ltb->size);
-	}
-	mutex_unlock(&adapter->fw_lock);
-	return 0;
-}
-
 static void deactivate_rx_pools(struct ibmvnic_adapter *adapter)
 {
 	int i;
@@ -548,18 +549,10 @@  static int reset_rx_pools(struct ibmvnic_adapter *adapter)
 
 		netdev_dbg(adapter->netdev, "Re-setting rx_pool[%d]\n", i);
 
-		if (rx_pool->buff_size != buff_size) {
-			free_long_term_buff(adapter, &rx_pool->long_term_buff);
-			rx_pool->buff_size = ALIGN(buff_size, L1_CACHE_BYTES);
-			rc = alloc_long_term_buff(adapter,
-						  &rx_pool->long_term_buff,
-						  rx_pool->size *
-						  rx_pool->buff_size);
-		} else {
-			rc = reset_long_term_buff(adapter,
-						  &rx_pool->long_term_buff);
-		}
-
+		rx_pool->buff_size = ALIGN(buff_size, L1_CACHE_BYTES);
+		rc = alloc_long_term_buff(adapter,
+					  &rx_pool->long_term_buff,
+					  rx_pool->size * rx_pool->buff_size);
 		if (rc)
 			return rc;
 
@@ -692,9 +685,12 @@  static int init_rx_pools(struct net_device *netdev)
 static int reset_one_tx_pool(struct ibmvnic_adapter *adapter,
 			     struct ibmvnic_tx_pool *tx_pool)
 {
+	struct ibmvnic_long_term_buff *ltb;
 	int rc, i;
 
-	rc = reset_long_term_buff(adapter, &tx_pool->long_term_buff);
+	ltb = &tx_pool->long_term_buff;
+
+	rc = alloc_long_term_buff(adapter, ltb, ltb->size);
 	if (rc)
 		return rc;