Re: [PATCH RFC v2 29/29] vmxnet3: Make use of pcim_enable_msix_range() interface

2013-10-21 Thread Alexander Gordeev
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.


-- 
Regards,
Alexander Gordeev
agord...@redhat.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [PATCH RFC v2 29/29] vmxnet3: Make use of pcim_enable_msix_range() interface

2013-10-21 Thread David Laight
> 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-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/