Re: [PATCH] mpt3sas: switch to pci_alloc_irq_vectors

[kbuild test robot]

Hi Hannes,

[auto build test ERROR on scsi/for-next]
[also build test ERROR on next-20161202]
[cannot apply to v4.9-rc7]
[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/Hannes-Reinecke/mpt3sas-switch-to-pci_alloc_irq_vectors/20161203-074559
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
config: i386-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/scsi/mpt3sas/mpt3sas_base.c: In function '_base_get_msix_index':
>> drivers/scsi/mpt3sas/mpt3sas_base.c:2198:9: error: implicit declaration of 
>> function 'pci_irq_get_affinity_vector' 
>> [-Werror=implicit-function-declaration]
 return pci_irq_get_affinity_vector(ioc->pdev, raw_smp_processor_id());
^~~
   cc1: some warnings being treated as errors
--
   drivers/scsi/mpt3sas/mpt3sas_scsih.c: In function 'scsih_map_queues':
>> drivers/scsi/mpt3sas/mpt3sas_scsih.c:8564:9: error: too many arguments to 
>> function 'blk_mq_pci_map_queues'
 return blk_mq_pci_map_queues(>tag_set, ioc->pdev, 0);
^
   In file included from drivers/scsi/mpt3sas/mpt3sas_scsih.c:57:0:
   include/linux/blk-mq-pci.h:7:5: note: declared here
int blk_mq_pci_map_queues(struct blk_mq_tag_set *set, struct pci_dev *pdev);
^

vim +/pci_irq_get_affinity_vector +2198 drivers/scsi/mpt3sas/mpt3sas_base.c

  2192  return ioc->reply + (phys_addr - (u32)ioc->reply_dma);
  2193  }
  2194  
  2195  static inline u16
  2196  _base_get_msix_index(struct MPT3SAS_ADAPTER *ioc)
  2197  {
> 2198  return pci_irq_get_affinity_vector(ioc->pdev, 
> raw_smp_processor_id());
  2199  }
  2200  
  2201  /**

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


.config.gz
Description: application/gzip

Re: [PATCH] mpt3sas: switch to pci_alloc_irq_vectors

[Hannes Reinecke]

On 12/02/2016 03:30 PM, Christoph Hellwig wrote:

+static inline u16
 _base_get_msix_index(struct MPT3SAS_ADAPTER *ioc)
 {
+   return pci_irq_get_affinity_vector(ioc->pdev, raw_smp_processor_id());
 }


This looks extremely sketchy. Are we guaranteed to always have enough
MSI-X vectors that each CPU has it's own?  I'd much prefer to keep the
indirection array of the existing code.


Hmm. Okay. will be doing so.


+static int scsih_map_queues(struct Scsi_Host *shost)
+{
+   struct MPT3SAS_ADAPTER *ioc = shost_priv(shost);
+
+   return blk_mq_pci_map_queues(>tag_set, ioc->pdev, 0);
+}


Given that mpt3sas only supports a single hardware queue (and doesn't
set the nr_hw_queues field) this is pointless and won't have any effect.


Well, it does, actually.
But this is part of another patchset enabling 'real' mq support.
I'll be redoing the patch.

Cheers,

Hannes
--
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mpt3sas: switch to pci_alloc_irq_vectors

[Christoph Hellwig]

> +static inline u16
>  _base_get_msix_index(struct MPT3SAS_ADAPTER *ioc)
>  {
> + return pci_irq_get_affinity_vector(ioc->pdev, raw_smp_processor_id());
>  }

This looks extremely sketchy. Are we guaranteed to always have enough
MSI-X vectors that each CPU has it's own?  I'd much prefer to keep the
indirection array of the existing code.

> +static int scsih_map_queues(struct Scsi_Host *shost)
> +{
> + struct MPT3SAS_ADAPTER *ioc = shost_priv(shost);
> +
> + return blk_mq_pci_map_queues(>tag_set, ioc->pdev, 0);
> +}

Given that mpt3sas only supports a single hardware queue (and doesn't
set the nr_hw_queues field) this is pointless and won't have any effect.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] mpt3sas: switch to pci_alloc_irq_vectors

[Hannes Reinecke]

Cleanup the MSI-X handling allowing us to use
the PCI-layer provided vector allocation.

Signed-off-by: Hannes Reinecke 
---
 drivers/scsi/mpt3sas/mpt3sas_base.c  | 167 +++
 drivers/scsi/mpt3sas/mpt3sas_base.h  |   6 --
 drivers/scsi/mpt3sas/mpt3sas_scsih.c |   9 ++
 3 files changed, 42 insertions(+), 140 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c 
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index f00ef88..0d93cb0 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -1129,7 +1129,7 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
/* TMs are on msix_index == 0 */
if (reply_q->msix_index == 0)
continue;
-   synchronize_irq(reply_q->vector);
+   synchronize_irq(pci_irq_vector(ioc->pdev, reply_q->msix_index));
}
 }
 
@@ -1818,11 +1818,7 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
 
list_for_each_entry_safe(reply_q, next, >reply_queue_list, list) {
list_del(_q->list);
-   if (smp_affinity_enable) {
-   irq_set_affinity_hint(reply_q->vector, NULL);
-   free_cpumask_var(reply_q->affinity_hint);
-   }
-   free_irq(reply_q->vector, reply_q);
+   free_irq(pci_irq_vector(ioc->pdev, reply_q->msix_index), 
reply_q);
kfree(reply_q);
}
 }
@@ -1831,13 +1827,13 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
  * _base_request_irq - request irq
  * @ioc: per adapter object
  * @index: msix index into vector table
- * @vector: irq vector
  *
  * Inserting respective reply_queue into the list.
  */
 static int
-_base_request_irq(struct MPT3SAS_ADAPTER *ioc, u8 index, u32 vector)
+_base_request_irq(struct MPT3SAS_ADAPTER *ioc, u8 index)
 {
+   struct pci_dev *pdev = ioc->pdev;
struct adapter_reply_queue *reply_q;
int r;
 
@@ -1849,14 +1845,6 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
}
reply_q->ioc = ioc;
reply_q->msix_index = index;
-   reply_q->vector = vector;
-
-   if (smp_affinity_enable) {
-   if (!zalloc_cpumask_var(_q->affinity_hint, GFP_KERNEL)) {
-   kfree(reply_q);
-   return -ENOMEM;
-   }
-   }
 
atomic_set(_q->busy, 0);
if (ioc->msix_enable)
@@ -1865,12 +1853,11 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
else
snprintf(reply_q->name, MPT_NAME_LENGTH, "%s%d",
ioc->driver_name, ioc->id);
-   r = request_irq(vector, _base_interrupt, IRQF_SHARED, reply_q->name,
-   reply_q);
+   r = request_irq(pci_irq_vector(pdev, index), _base_interrupt,
+   IRQF_SHARED, reply_q->name, reply_q);
if (r) {
pr_err(MPT3SAS_FMT "unable to allocate interrupt %d!\n",
-   reply_q->name, vector);
-   free_cpumask_var(reply_q->affinity_hint);
+  reply_q->name, pci_irq_vector(pdev, index));
kfree(reply_q);
return -EBUSY;
}
@@ -1881,61 +1868,6 @@ static int mpt3sas_remove_dead_ioc_func(void *arg)
 }
 
 /**
- * _base_assign_reply_queues - assigning msix index for each cpu
- * @ioc: per adapter object
- *
- * The enduser would need to set the affinity via /proc/irq/#/smp_affinity
- *
- * It would nice if we could call irq_set_affinity, however it is not
- * an exported symbol
- */
-static void
-_base_assign_reply_queues(struct MPT3SAS_ADAPTER *ioc)
-{
-   unsigned int cpu, nr_cpus, nr_msix, index = 0;
-   struct adapter_reply_queue *reply_q;
-
-   if (!_base_is_controller_msix_enabled(ioc))
-   return;
-
-   memset(ioc->cpu_msix_table, 0, ioc->cpu_msix_table_sz);
-
-   nr_cpus = num_online_cpus();
-   nr_msix = ioc->reply_queue_count = min(ioc->reply_queue_count,
-  ioc->facts.MaxMSIxVectors);
-   if (!nr_msix)
-   return;
-
-   cpu = cpumask_first(cpu_online_mask);
-
-   list_for_each_entry(reply_q, >reply_queue_list, list) {
-
-   unsigned int i, group = nr_cpus / nr_msix;
-
-   if (cpu >= nr_cpus)
-   break;
-
-   if (index < nr_cpus % nr_msix)
-   group++;
-
-   for (i = 0 ; i < group ; i++) {
-   ioc->cpu_msix_table[cpu] = index;
-   if (smp_affinity_enable)
-   cpumask_or(reply_q->affinity_hint,
-  reply_q->affinity_hint, get_cpu_mask(cpu));
-   cpu = cpumask_next(cpu, cpu_online_mask);
-   }
-   if (smp_affinity_enable)
-   if (irq_set_affinity_hint(reply_q->vector,
-