Patchwork [RFC,v2,29/29] vmxnet3: Make use of pcim_enable_msix_range() interface

login
register
mail settings
Submitter Alexander Gordeev
Date Oct. 18, 2013, 5:12 p.m.
Message ID <75788c5da384b16633fdab43d37a277aa92f38fd.1382103786.git.agordeev@redhat.com>
Download mbox | patch
Permalink /patch/284833/
State Superseded
Headers show

Comments

Alexander Gordeev - Oct. 18, 2013, 5:12 p.m.
Signed-off-by: Alexander Gordeev <agordeev@redhat.com>
---
 drivers/net/vmxnet3/vmxnet3_drv.c |   40 +++++++++---------------------------
 1 files changed, 10 insertions(+), 30 deletions(-)
David Laight - Oct. 21, 2013, 8:51 a.m.
> Subject: [PATCH RFC v2 29/29] vmxnet3: Make use of pcim_enable_msix_range() interface
...
> diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
> index d33802c..e552d2b 100644
> --- a/drivers/net/vmxnet3/vmxnet3_drv.c
> +++ b/drivers/net/vmxnet3/vmxnet3_drv.c
> @@ -2735,39 +2735,19 @@ vmxnet3_read_mac_addr(struct vmxnet3_adapter *adapter, u8 *mac)
>   */
> 
>  static int
> -vmxnet3_acquire_msix_vectors(struct vmxnet3_adapter *adapter,
> -			     int vectors)
> +vmxnet3_acquire_msix_vectors(struct vmxnet3_adapter *adapter, int vectors)
>  {
> -	int err = -EINVAL, vector_threshold;
> -	vector_threshold = VMXNET3_LINUX_MIN_MSIX_VECT;
> -
> -	while (vectors >= vector_threshold) {
> -		err = pci_enable_msix(adapter->pdev, adapter->intr.msix_entries,
> -				      vectors);
> -		if (!err) {
> -			adapter->intr.num_intrs = vectors;
> -			return 0;
> -		} else if (err < 0) {
> -			dev_err(&adapter->netdev->dev,
> -				   "Failed to enable MSI-X, error: %d\n", err);
> -			return err;
> -		} else if (err < vector_threshold) {
> -			dev_info(&adapter->pdev->dev,
> -				 "Number of MSI-Xs which can be allocated "
> -				 "is lower than min threshold required.\n");
> -			return -ENOSPC;
> -		} else {
> -			/* If fails to enable required number of MSI-x vectors
> -			 * try enabling minimum number of vectors required.
> -			 */
> -			dev_err(&adapter->netdev->dev,
> -				"Failed to enable %d MSI-X, trying %d instead\n",
> -				    vectors, vector_threshold);
> -			vectors = vector_threshold;
> -		}
> +	vectors = pcim_enable_msix_range(adapter->pdev,
> +					 adapter->intr.msix_entries, vectors,
> +					 VMXNET3_LINUX_MIN_MSIX_VECT);
> +	if (vectors < 0) {
> +		dev_err(&adapter->netdev->dev,
> +			"Failed to enable MSI-X, error: %d\n", vectors);
> +		return vectors;
>  	}
> 
> -	return err;
> +	adapter->intr.num_intrs = vectors;
> +	return 0;
>  }

AFAICT the old code either used the requested number or the minimum number.
The new code seems to claim an intermediate number of interrupts - but probably
only uses the minimum number.
This wastes the last few MSI-X interrupts.
The code (especially the calling code) would be easier to read if the 'vectors'
value wasn't explicitly passed.

	David



--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexander Gordeev - Oct. 21, 2013, 10:19 a.m.
On Mon, Oct 21, 2013 at 09:51:32AM +0100, David Laight wrote:
> > Subject: [PATCH RFC v2 29/29] vmxnet3: Make use of pcim_enable_msix_range() interface
> AFAICT the old code either used the requested number or the minimum number.
> The new code seems to claim an intermediate number of interrupts - but probably
> only uses the minimum number.

Yes, you are right. I missed the vectors = vector_threshold assignment.
This driver is inapplicable as an example.

Patch

diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
index d33802c..e552d2b 100644
--- a/drivers/net/vmxnet3/vmxnet3_drv.c
+++ b/drivers/net/vmxnet3/vmxnet3_drv.c
@@ -2735,39 +2735,19 @@  vmxnet3_read_mac_addr(struct vmxnet3_adapter *adapter, u8 *mac)
  */
 
 static int
-vmxnet3_acquire_msix_vectors(struct vmxnet3_adapter *adapter,
-			     int vectors)
+vmxnet3_acquire_msix_vectors(struct vmxnet3_adapter *adapter, int vectors)
 {
-	int err = -EINVAL, vector_threshold;
-	vector_threshold = VMXNET3_LINUX_MIN_MSIX_VECT;
-
-	while (vectors >= vector_threshold) {
-		err = pci_enable_msix(adapter->pdev, adapter->intr.msix_entries,
-				      vectors);
-		if (!err) {
-			adapter->intr.num_intrs = vectors;
-			return 0;
-		} else if (err < 0) {
-			dev_err(&adapter->netdev->dev,
-				   "Failed to enable MSI-X, error: %d\n", err);
-			return err;
-		} else if (err < vector_threshold) {
-			dev_info(&adapter->pdev->dev,
-				 "Number of MSI-Xs which can be allocated "
-				 "is lower than min threshold required.\n");
-			return -ENOSPC;
-		} else {
-			/* If fails to enable required number of MSI-x vectors
-			 * try enabling minimum number of vectors required.
-			 */
-			dev_err(&adapter->netdev->dev,
-				"Failed to enable %d MSI-X, trying %d instead\n",
-				    vectors, vector_threshold);
-			vectors = vector_threshold;
-		}
+	vectors = pcim_enable_msix_range(adapter->pdev,
+					 adapter->intr.msix_entries, vectors,
+					 VMXNET3_LINUX_MIN_MSIX_VECT);
+	if (vectors < 0) {
+		dev_err(&adapter->netdev->dev,
+			"Failed to enable MSI-X, error: %d\n", vectors);
+		return vectors;
 	}
 
-	return err;
+	adapter->intr.num_intrs = vectors;
+	return 0;
 }