Re: [PATCH V8 0/7] blk-mq support for ZBC disks

[Jens Axboe]

On 11/24/2017 04:48 PM, Damien Le Moal wrote:
> [Full quote deleted]
> 
> Hi Jens,
> 
> Any comment regarding this series ?
> I understand that this would be for the 4.16 merge window, so no hurry,
> but I would like to know if I need to go back to the drawing board for
> ZBC blk-mq/scsi-mq support or if this is an acceptable solution.

I'll give it a thorough look-over on Monday.

-- 
Jens Axboe

Re: [PATCH V8 0/7] blk-mq support for ZBC disks

[Damien Le Moal]

[Full quote deleted]

Hi Jens,

Any comment regarding this series ?
I understand that this would be for the 4.16 merge window, so no hurry,
but I would like to know if I need to go back to the drawing board for
ZBC blk-mq/scsi-mq support or if this is an acceptable solution.

Best regards.

-- 
Damien Le Moal
Western Digital Research

[PATCH] scsi: sd: add missing KERN_CONT for disk spin-up

[Michał Mirosław]

KERN_CONT is now required for continued printks(). Add it.

Signed-off-by: Michał Mirosław 
---
 drivers/scsi/sd.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index d175c5c5ccf8..9743b5f0800d 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -2142,7 +2142,7 @@ sd_spinup_disk(struct scsi_disk *sdkp)
}
/* Wait 1 second for next try */
msleep(1000);
-   printk(".");
+   printk(KERN_CONT ".");
 
/*
 * Wait for USB flash devices with slow firmware.
@@ -2172,9 +2172,9 @@ sd_spinup_disk(struct scsi_disk *sdkp)
 
if (spintime) {
if (scsi_status_is_good(the_result))
-   printk("ready\n");
+   printk(KERN_CONT "ready\n");
else
-   printk("not responding...\n");
+   printk(KERN_CONT "not responding...\n");
}
 }
 
-- 
2.11.0

Re: [PATCH 0/3] scsi: arcmsr: add driver module parameter - msi_enable, msix_enable

[Dan Carpenter]

On Fri, Nov 24, 2017 at 03:12:39AM +0800, Ching Huang wrote:
> On Thu, 2017-11-23 at 04:57 -0800, Christoph Hellwig wrote:
> > On Thu, Nov 23, 2017 at 09:22:03AM +0800, Ching Huang wrote:
> > > From: Ching Huang 
> > > 
> > > Hi all,
> > > 
> > > The following patches apply to Martin's 4.16/scsi-queue.
> > > 
> > > Patch 1: Add module parameter msi_enable to has a chance to disable msi 
> > > interrupt if it does not work properly.
> > > 
> > > Patch 2: Add module parameter msix_enable to has a chance to disable msix 
> > > interrupt if it does not work properly.
> > 
> > Why would it not work properly?
> This patch is apply to
> https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git/tree/?h=4.16/scsi-queue
>

No, the question is we're adding an option called "msi_enable" which is
used to disable MSI interrupts "if it does not work properly".  Why is
the current code not working properly?

Is there a crash or a performance issue?  What does the bug in the
current code look like from a user perspective?  Can you send us a dmesg
from a failing system?

regards,
dan carpenter

[PATCH] scsi: sym53c8xx_2: use setup_timer instead of init_timer

[Colin King]

From: Colin Ian King 

Use setup_timer function instead of initializing timer with the
function and data fields.

Signed-off-by: Colin Ian King 
---
 drivers/scsi/sym53c8xx_2/sym_glue.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/scsi/sym53c8xx_2/sym_glue.c 
b/drivers/scsi/sym53c8xx_2/sym_glue.c
index d32e3ba8863e..285397d42558 100644
--- a/drivers/scsi/sym53c8xx_2/sym_glue.c
+++ b/drivers/scsi/sym53c8xx_2/sym_glue.c
@@ -1351,9 +1351,7 @@ static struct Scsi_Host *sym_attach(struct 
scsi_host_template *tpnt, int unit,
/*
 *  Start the timer daemon
 */
-   init_timer(>s.timer);
-   np->s.timer.data = (unsigned long) np;
-   np->s.timer.function = sym53c8xx_timer;
+   setup_timer(>s.timer, sym53c8xx_timer, (unsigned long)np);
np->s.lasttime=0;
sym_timer (np);
 
-- 
2.14.1

[PATCH] scsi: bfa: use setup_timer instead of init_timer

[Colin King]

From: Colin Ian King 

Use setup_timer function instead of initializing timer with the
function and data fields.

Signed-off-by: Colin Ian King 
---
 drivers/scsi/bfa/bfad.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/scsi/bfa/bfad.c b/drivers/scsi/bfa/bfad.c
index 5caf5f3ff642..c910b238bc9e 100644
--- a/drivers/scsi/bfa/bfad.c
+++ b/drivers/scsi/bfa/bfad.c
@@ -719,10 +719,7 @@ bfad_bfa_tmo(unsigned long data)
 void
 bfad_init_timer(struct bfad_s *bfad)
 {
-   init_timer(>hal_tmo);
-   bfad->hal_tmo.function = bfad_bfa_tmo;
-   bfad->hal_tmo.data = (unsigned long)bfad;
-
+   setup_timer(>hal_tmo, bfad_bfa_tmo, (unsigned long)bfad);
mod_timer(>hal_tmo,
  jiffies + msecs_to_jiffies(BFA_TIMER_FREQ));
 }
-- 
2.14.1

Re: [PATCH 1/3] scsi: arcmsr: Add driver module parameter msi_enable

[Ching Huang]

On Fri, 2017-11-24 at 04:45 +0800, Ching Huang wrote:
> Hello Dan,
> 
> On Thu, 2017-11-23 at 13:44 +0300, Dan Carpenter wrote:
> > On Thu, Nov 23, 2017 at 09:27:19AM +0800, Ching Huang wrote:
> > > From: Ching Huang 
> > > 
> > > Add module parameter msi_enable to has a chance to disable msi interrupt 
> > > if it does not work properly.
> > > 
> > > Signed-off-by: Ching Huang 
> > > ---
> > > 
> > > diff -uprN a/drivers/scsi/arcmsr/arcmsr_hba.c 
> > > b/drivers/scsi/arcmsr/arcmsr_hba.c
> > > --- a/drivers/scsi/arcmsr/arcmsr_hba.c2017-11-23 14:29:26.0 
> > > +0800
> > > +++ b/drivers/scsi/arcmsr/arcmsr_hba.c2017-11-23 16:02:28.0 
> > > +0800
> > > @@ -75,6 +75,10 @@ MODULE_DESCRIPTION("Areca ARC11xx/12xx/1
> > >  MODULE_LICENSE("Dual BSD/GPL");
> > >  MODULE_VERSION(ARCMSR_DRIVER_VERSION);
> > >  
> > > +static int msi_enable = 1;
> > > +module_param(msi_enable, int, S_IRUGO);
> >  ^^^
> > checkpatch.pl will complain that this should be 0444
> S_IRUGO value is 00444, defined in  -> .
>  A. It will be not a issue.
> > 
> > > +MODULE_PARM_DESC(msi_enable, " Enable MSI interrupt(0 ~ 1), 
> > > msi_enable=1(enable), =0(disable)");
> >  ^
> > Remove the extra space
> OK
> > 
> > > +
> > >  static int host_can_queue = ARCMSR_DEFAULT_OUTSTANDING_CMD;
> > >  module_param(host_can_queue, int, S_IRUGO);
> > >  MODULE_PARM_DESC(host_can_queue, " adapter queue depth(32 ~ 1024), 
> > > default is 128");
> > > @@ -831,11 +835,15 @@ arcmsr_request_irq(struct pci_dev *pdev,
> > >   pr_info("arcmsr%d: msi-x enabled\n", acb->host->host_no);
> > >   flags = 0;
> > >   } else {
> > > - nvec = pci_alloc_irq_vectors(pdev, 1, 1,
> > > - PCI_IRQ_MSI | PCI_IRQ_LEGACY);
> > > + if (msi_enable == 1)
> > > + nvec = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_MSI);
> > > + else
> > > + nvec = pci_alloc_irq_vectors(pdev, 1, 1, 
> > > PCI_IRQ_LEGACY);
> > >   if (nvec < 1)
> > >   return FAILED;
> > 
> > I feel like we should try PCI_IRQ_MSI then if it fails we could fall
> > back to PCI_IRQ_LEGACY.  Originally, it worked like this and now it just
> > fails unless you toggle the module param.  It's a regression.
> update as below
> ---
>   unsigned int irq_flag;
>   irq_flag = PCI_IRQ_LEGACY;
>   if (msi_enable == 1)
>   irq_flag |= PCI_IRQ_MSI;
>   nvec = pci_alloc_irq_vectors(pdev, 1, 1, irq_flag);
> > >  
> > > + if (msi_enable == 1)
> > > + pr_info("arcmsr%d: msi enabled\n", acb->host->host_no);
> > 
> > This printk could be improved.  Use dev_info(>dev, for a start.
> > I know that the other prints don't use this, but we could use it one
> > time then slowly add more users until more are using dev_info() than
> > pr_info() and then someone will decide to clean up the old users.
> update as below
> ---
>   if (msi_enable == 1)
>   dev_info(>dev, "msi enabled\n");
> 
> > 
> > regards,
> > dan carpenter
> > 
> 
Dan,.

This new patch apply to mkp/scsi.git 4.16/scsi-queue
---

diff -uprN a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
--- a/drivers/scsi/arcmsr/arcmsr_hba.c  2017-11-23 14:29:26.0 +0800
+++ b/drivers/scsi/arcmsr/arcmsr_hba.c  2017-11-24 15:16:20.0 +0800
@@ -75,6 +75,10 @@ MODULE_DESCRIPTION("Areca ARC11xx/12xx/1
 MODULE_LICENSE("Dual BSD/GPL");
 MODULE_VERSION(ARCMSR_DRIVER_VERSION);
 
+static int msi_enable = 1;
+module_param(msi_enable, int, S_IRUGO);
+MODULE_PARM_DESC(msi_enable, "Enable MSI interrupt(0 ~ 1), 
msi_enable=1(enable), =0(disable)");
+
 static int host_can_queue = ARCMSR_DEFAULT_OUTSTANDING_CMD;
 module_param(host_can_queue, int, S_IRUGO);
 MODULE_PARM_DESC(host_can_queue, " adapter queue depth(32 ~ 1024), default is 
128");
@@ -831,11 +835,17 @@ arcmsr_request_irq(struct pci_dev *pdev,
pr_info("arcmsr%d: msi-x enabled\n", acb->host->host_no);
flags = 0;
} else {
-   nvec = pci_alloc_irq_vectors(pdev, 1, 1,
-   PCI_IRQ_MSI | PCI_IRQ_LEGACY);
+   if (msi_enable == 1) {
+   nvec = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_MSI);
+   if (nvec == 1) {
+   dev_info(>dev, "msi enabled\n");
+   goto msi_int1;
+   }
+   }
+   nvec = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_LEGACY);
if (nvec < 1)
return FAILED;
-
+msi_int1:
flags = IRQF_SHARED;
}