Re: [PATCH for sg3_utils 0/4] Simplify the sg3_utils configure script

[Douglas Gilbert]

On 2018-01-03 04:37 AM, Bean Huo (beanhuo) wrote:

Hi, Bart and Dong
Do you think we need to submit all changed diff including some generated files 
by automake?
Such as config.h.in, Makefile.in..., in fact, we only manually did a few 
modificaiton on
Some files such as configura.ac,makefile.am.


Hi,
Yes, I only needed the configure.ac and Makefile.am changes from Bart.
Anyway, they are in my sg3_utils subversion rev 742 which should
appear soon on Hannes' site:
   https://github.com/hreinecke/sg3_utils

Currently it is at rev 741.

sgp_dd was also changed to only use pthread_kill() with the older code
"#if 0"-ed out just in case it needs to be revisited. The checking macros
for pthread_cancel() and pthread_kill() are still in configure.ac .

Doug Gilbert


The four patches in this series is what I came up with after having reviewed
the sg3_utils configure script. Please consider to apply these patches on the
sg3_utils trunk.

Thanks,

Bart.

Bart Van Assche (4):
  configure.ac: Suppress an autoconf warning
  configure.ac: Only link with the pthread library if required
  configure.ac: Avoid code duplication
  configure.ac, Makefile.am: Use CFLAGS and LIBS instead of os_cflags
and os_libs

Makefile.in |   3 +-
config.h.in |   6 +-
configure   | 672 +++-
configure.ac|  83 +++
doc/Makefile.in |   3 +-
include/Makefile.in |   3 +-
lib/Makefile.am |   2 +-
lib/Makefile.in |   5 +-
scripts/Makefile.in |   3 +-
src/Makefile.am | 140 ++-
src/Makefile.in | 145 ++--
11 files changed, 425 insertions(+), 640 deletions(-)

--
2.15.1

[PATCH] aacraid driver oops with dead battery

[Meelis Roos]

The battery in my HP NetRAID-4M died of old age, and the aacraid driver 
started oopsing with NULL pointer dereference on startup after that.

Fix it by reordering the init sequence to fill in function pointers 
before ioremapping memory, or dev->a_ops.adapter_ioremap pointer will be 
NULL.

Other subtypes of aacraid seem to have the order already correct.

This was the call trace:

 ? aac_probe_one+0x7a5/0xb30 [aacraid]
 pci_device_probe+0xc0/0x1a0
 driver_probe_device+0x1df/0x3b0
 __driver_attach+0xa9/0xe0
 ? driver_probe_device+0x3b0/0x3b0
 bus_for_each_dev+0x4c/0x90
 driver_attach+0x1d/0x40
 ? driver_probe_device+0x3b0/0x3b0
 bus_add_driver+0x1a7/0x2a0
 driver_register+0x6e/0x130
 __pci_register_driver+0x54/0x90
 ? 0xf81f4000
 aac_init+0x2b/0x1000 [aacraid]
 do_one_initcall+0x45/0x1e0
 ? kfree_skbmem+0x74/0xa0
 ? kfree+0x16d/0x240
 ? kvfree+0x45/0x50
 ? kvfree+0x45/0x50
 ? __vunmap+0x99/0x120
 ? do_init_module+0x1a/0x245
 do_init_module+0x83/0x245
 load_module+0x2764/0x34a0
 ? kernel_read_file+0x150/0x320
 SyS_finit_module+0x82/0xa0
 do_fast_syscall_32+0xba/0x340

Signed-off-by: Meelis Roos 

diff --git a/drivers/scsi/aacraid/sa.c b/drivers/scsi/aacraid/sa.c
index 553922fed524..882f40353b96 100644
--- a/drivers/scsi/aacraid/sa.c
+++ b/drivers/scsi/aacraid/sa.c
@@ -329,6 +329,22 @@ int aac_sa_init(struct aac_dev *dev)
instance = dev->id;
name = dev->name;
 
+   /*
+*  Fill in the function dispatch table.
+*/
+
+   dev->a_ops.adapter_interrupt = aac_sa_interrupt_adapter;
+   dev->a_ops.adapter_disable_int = aac_sa_disable_interrupt;
+   dev->a_ops.adapter_enable_int = aac_sa_enable_interrupt;
+   dev->a_ops.adapter_notify = aac_sa_notify_adapter;
+   dev->a_ops.adapter_sync_cmd = sa_sync_cmd;
+   dev->a_ops.adapter_check_health = aac_sa_check_health;
+   dev->a_ops.adapter_restart = aac_sa_restart_adapter;
+   dev->a_ops.adapter_start = aac_sa_start_adapter;
+   dev->a_ops.adapter_intr = aac_sa_intr;
+   dev->a_ops.adapter_deliver = aac_rx_deliver_producer;
+   dev->a_ops.adapter_ioremap = aac_sa_ioremap;
+
if (aac_sa_ioremap(dev, dev->base_size)) {
printk(KERN_WARNING "%s: unable to map adapter.\n", name);
goto error_iounmap;
@@ -362,22 +378,6 @@ int aac_sa_init(struct aac_dev *dev)
msleep(1);
}
 
-   /*
-*  Fill in the function dispatch table.
-*/
-
-   dev->a_ops.adapter_interrupt = aac_sa_interrupt_adapter;
-   dev->a_ops.adapter_disable_int = aac_sa_disable_interrupt;
-   dev->a_ops.adapter_enable_int = aac_sa_enable_interrupt;
-   dev->a_ops.adapter_notify = aac_sa_notify_adapter;
-   dev->a_ops.adapter_sync_cmd = sa_sync_cmd;
-   dev->a_ops.adapter_check_health = aac_sa_check_health;
-   dev->a_ops.adapter_restart = aac_sa_restart_adapter;
-   dev->a_ops.adapter_start = aac_sa_start_adapter;
-   dev->a_ops.adapter_intr = aac_sa_intr;
-   dev->a_ops.adapter_deliver = aac_rx_deliver_producer;
-   dev->a_ops.adapter_ioremap = aac_sa_ioremap;
-
/*
 *  First clear out all interrupts.  Then enable the one's that 
 *  we can handle.


-- 
Meelis Roos (mr...@linux.ee)

RE: [PATCH for sg3_utils 1/4] configure.ac: Suppress an autoconf warning

[Bean Huo (beanhuo)]

>a/configure.ac b/configure.ac index 1cce0b25f583..129c2f9d4d4d 100644
>--- a/configure.ac
>+++ b/configure.ac
>@@ -7,13 +7,13 @@ AM_CONFIG_HEADER(config.h)  AC_PROG_CC  #
>AC_PROG_CXX  AC_PROG_INSTALL
>-
>-# Adding libtools to the build seems to bring in C++ environment -
>AC_PROG_LIBTOOL
>
> # AM_PROG_AR is supported and needed since automake v1.12+
>ifdef([AM_PROG_AR], [AM_PROG_AR], [])
>
>+# Adding libtools to the build seems to bring in C++ environment
>+AC_PROG_LIBTOOL
>+
> # check for headers
> AC_HEADER_STDC
>

After moving "AC_PROG_LIBTOOL" to back " ifdef([AM_PROG_AR], [AM_PROG_AR], [])",
"Warning: LT_INIT was called before AM_PROG_AR" disappears.

Tested-by: Bean Huo 

>--
>2.15.1

RE: [PATCH for sg3_utils 0/4] Simplify the sg3_utils configure script

[Bean Huo (beanhuo)]

Hi, Bart and Dong
Do you think we need to submit all changed diff including some generated files 
by automake?
Such as config.h.in, Makefile.in..., in fact, we only manually did a few 
modificaiton on
Some files such as configura.ac,makefile.am.
 
>
>Hello Doug,
>
>The four patches in this series is what I came up with after having reviewed
>the sg3_utils configure script. Please consider to apply these patches on the
>sg3_utils trunk.
>
>Thanks,
>
>Bart.
>
>Bart Van Assche (4):
>  configure.ac: Suppress an autoconf warning
>  configure.ac: Only link with the pthread library if required
>  configure.ac: Avoid code duplication
>  configure.ac, Makefile.am: Use CFLAGS and LIBS instead of os_cflags
>and os_libs
>
> Makefile.in |   3 +-
> config.h.in |   6 +-
> configure   | 672 +++-
> configure.ac|  83 +++
> doc/Makefile.in |   3 +-
> include/Makefile.in |   3 +-
> lib/Makefile.am |   2 +-
> lib/Makefile.in |   5 +-
> scripts/Makefile.in |   3 +-
> src/Makefile.am | 140 ++-
> src/Makefile.in | 145 ++--
> 11 files changed, 425 insertions(+), 640 deletions(-)
>
>--
>2.15.1

Re: [PATCH v2 22/30] scsi: aacraid: Merge adapter setup with resolve luns

[Nikola Pajkovsky]

Raghava Aditya Renukunta  writes:

> The device hotplug events are processed only after retrieving the updated
> lun information from the fw. Does not make sense to keep them separate.
>
> Merge both the hotplug handling and safw adapter setup code into single
> function.
>
> Signed-off-by: Raghava Aditya Renukunta 
> 

According to subsequent commit

  [PATCH v2 23/30] scsi: aacraid: Block concurrent hotplug event handling

this commit is racy, because 23/30 adds ->scan_mutex. Shouldn't be these
commits squashed?

--
Nikola

RE: [PATCH v2 22/30] scsi: aacraid: Merge adapter setup with resolve luns

[Raghava Aditya Renukunta]

Hi Nikola,

> -Original Message-
> From: Nikola Pajkovsky [mailto:npajkov...@suse.cz]
> Sent: Wednesday, January 3, 2018 2:02 AM
> To: Raghava Aditya Renukunta
> 
> Cc: j...@linux.vnet.ibm.com; martin.peter...@oracle.com; linux-
> s...@vger.kernel.org; Scott Benesh ; Tom
> White ; dl-esc-Aacraid Linux Driver
> ; Guilherme G . Piccoli
> ; Bart Van Assche
> 
> Subject: Re: [PATCH v2 22/30] scsi: aacraid: Merge adapter setup with resolve
> luns
> 
> EXTERNAL EMAIL
> 
> 
> Raghava Aditya Renukunta 
> writes:
> 
> > The device hotplug events are processed only after retrieving the updated
> > lun information from the fw. Does not make sense to keep them separate.
> >
> > Merge both the hotplug handling and safw adapter setup code into single
> > function.
> >
> > Signed-off-by: Raghava Aditya Renukunta
> 
> 
> According to subsequent commit
> 
>   [PATCH v2 23/30] scsi: aacraid: Block concurrent hotplug event handling
> 
> this commit is racy, because 23/30 adds ->scan_mutex. Shouldn't be these
> commits squashed?

I tried to make the patches as logically distinct as possible, maybe I got a 
bit too ambitious and I expected the patches to go thru as a set so I don’t 
think it would make any difference. What do you think? 

Thanks
Raghava Aditya

> --
> Nikola

Re: [-next PATCH 2/4] treewide: Use DEVICE_ATTR_RW

[Bartlomiej Zolnierkiewicz]

On Tuesday, December 19, 2017 10:15:07 AM Joe Perches wrote:
> Convert DEVICE_ATTR uses to DEVICE_ATTR_RW where possible.
> 
> Done with perl script:
> 
> $ git grep -w --name-only DEVICE_ATTR | \
>   xargs perl -i -e 'local $/; while (<>) { 
> s/\bDEVICE_ATTR\s*\(\s*(\w+)\s*,\s*\(?(\s*S_IRUGO\s*\|\s*S_IWUSR|\s*S_IWUSR\s*\|\s*S_IRUGO\s*|\s*0644\s*)\)?\s*,\s*\1_show\s*,\s*\1_store\s*\)/DEVICE_ATTR_RW(\1)/g;
>  print;}'
> 
> Signed-off-by: Joe Perches 
> ---
>  arch/s390/kernel/topology.c  |  3 +--
>  arch/tile/kernel/sysfs.c |  2 +-
>  drivers/gpu/drm/i915/i915_sysfs.c|  6 ++---
>  drivers/platform/x86/compal-laptop.c | 18 +--
>  drivers/s390/cio/device.c|  2 +-
>  drivers/scsi/lpfc/lpfc_attr.c| 43 
> 
>  drivers/thermal/thermal_sysfs.c  |  9 
>  drivers/tty/serial/sh-sci.c  |  2 +-
>  drivers/usb/host/xhci-dbgcap.c   |  2 +-
>  drivers/usb/phy/phy-tahvo.c  |  2 +-
>  drivers/video/fbdev/auo_k190x.c  |  4 ++--
>  drivers/video/fbdev/w100fb.c |  4 ++--
>  lib/test_firmware.c  | 14 +---
>  lib/test_kmod.c  | 14 +---
>  sound/soc/omap/mcbsp.c   |  4 ++--
>  15 files changed, 49 insertions(+), 80 deletions(-)

For fbdev changes:

Acked-by: Bartlomiej Zolnierkiewicz 

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R Institute Poland
Samsung Electronics

Re: [PATCH v17 0/4] Replace PCI pool by DMA pool API

[Bjorn Helgaas]

On Tue, Jan 02, 2018 at 04:17:24PM -0600, Bjorn Helgaas wrote:
> On Tue, Jan 02, 2018 at 06:53:52PM +0100, Romain Perier wrote:
> > The current PCI pool API are simple macro functions direct expanded to
> > the appropriate dma pool functions. The prototypes are almost the same
> > and semantically, they are very similar. I propose to use the DMA pool
> > API directly and get rid of the old API.
> > 
> > This set of patches, replaces the old API by the dma pool API
> > and remove the defines.
> > ...
> 
> > Romain Perier (4):
> >   block: DAC960: Replace PCI pool old API
> >   net: e100: Replace PCI pool old API
> >   hinic: Replace PCI pool old API
> >   PCI: Remove PCI pool macro functions
> > 
> >  drivers/block/DAC960.c| 38 
> > +++
> >  drivers/block/DAC960.h|  4 +--
> >  drivers/net/ethernet/huawei/hinic/hinic_hw_cmdq.c | 10 +++---
> >  drivers/net/ethernet/huawei/hinic/hinic_hw_cmdq.h |  2 +-
> >  drivers/net/ethernet/intel/e100.c | 12 +++
> >  include/linux/pci.h   |  9 --
> >  6 files changed, 32 insertions(+), 43 deletions(-)
> 
> Applied to pci/misc for v4.16, thanks!

Oops, my mistake.  I can't remove the macros themselves yet because
some of the uses were removed by patches that were applied via other
trees, and those patches are not in my tree.

To avoid ordering dependencies during the merge window, I dropped the
"PCI: Remove PCI pool macro functions" patch.  Please repost that
after all the removals have made it into Linus' tree.

Bjorn

[PATCH 6/6] cxlflash: Staging to support future accelerators

[Uma Krishnan]

From: "Matthew R. Ochs" 

As staging to support future accelerator transports, add a shim layer
such that the underlying services the cxlflash driver requires can be
conditional upon the accelerator infrastructure.

Signed-off-by: Matthew R. Ochs 
Signed-off-by: Uma Krishnan 
---
 drivers/scsi/cxlflash/Makefile|   2 +-
 drivers/scsi/cxlflash/backend.h   |  41 ++
 drivers/scsi/cxlflash/common.h|   3 +
 drivers/scsi/cxlflash/cxl_hw.c| 168 ++
 drivers/scsi/cxlflash/main.c  |  79 +++---
 drivers/scsi/cxlflash/superpipe.c |  48 +--
 6 files changed, 263 insertions(+), 78 deletions(-)
 create mode 100644 drivers/scsi/cxlflash/backend.h
 create mode 100644 drivers/scsi/cxlflash/cxl_hw.c

diff --git a/drivers/scsi/cxlflash/Makefile b/drivers/scsi/cxlflash/Makefile
index 9e39866..7ec3f6b 100644
--- a/drivers/scsi/cxlflash/Makefile
+++ b/drivers/scsi/cxlflash/Makefile
@@ -1,2 +1,2 @@
 obj-$(CONFIG_CXLFLASH) += cxlflash.o
-cxlflash-y += main.o superpipe.o lunmgt.o vlun.o
+cxlflash-y += main.o superpipe.o lunmgt.o vlun.o cxl_hw.o
diff --git a/drivers/scsi/cxlflash/backend.h b/drivers/scsi/cxlflash/backend.h
new file mode 100644
index 000..339e42b
--- /dev/null
+++ b/drivers/scsi/cxlflash/backend.h
@@ -0,0 +1,41 @@
+/*
+ * CXL Flash Device Driver
+ *
+ * Written by: Matthew R. Ochs , IBM Corporation
+ * Uma Krishnan , IBM Corporation
+ *
+ * Copyright (C) 2018 IBM Corporation
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+extern const struct cxlflash_backend_ops cxlflash_cxl_ops;
+
+struct cxlflash_backend_ops {
+   struct module *module;
+   void __iomem * (*psa_map)(void *);
+   void (*psa_unmap)(void __iomem *);
+   int (*process_element)(void *);
+   int (*map_afu_irq)(void *, int, irq_handler_t, void *, char *);
+   void (*unmap_afu_irq)(void *, int, void *);
+   int (*start_context)(void *);
+   int (*stop_context)(void *);
+   int (*afu_reset)(void *);
+   void (*set_master)(void *);
+   void * (*get_context)(struct pci_dev *, void *);
+   void * (*dev_context_init)(struct pci_dev *, void *);
+   int (*release_context)(void *);
+   void (*perst_reloads_same_image)(void *, bool);
+   ssize_t (*read_adapter_vpd)(struct pci_dev *, void *, size_t);
+   int (*allocate_afu_irqs)(void *, int);
+   void (*free_afu_irqs)(void *);
+   void * (*create_afu)(struct pci_dev *);
+   struct file * (*get_fd)(void *, struct file_operations *, int *);
+   void * (*fops_get_context)(struct file *);
+   int (*start_work)(void *, u64);
+   int (*fd_mmap)(struct file *, struct vm_area_struct *);
+   int (*fd_release)(struct inode *, struct file *);
+};
diff --git a/drivers/scsi/cxlflash/common.h b/drivers/scsi/cxlflash/common.h
index 48df89f..102fd26 100644
--- a/drivers/scsi/cxlflash/common.h
+++ b/drivers/scsi/cxlflash/common.h
@@ -25,6 +25,8 @@
 #include 
 #include 
 
+#include "backend.h"
+
 extern const struct file_operations cxlflash_cxl_fops;
 
 #define MAX_CONTEXTCXLFLASH_MAX_CONTEXT/* num contexts per afu */
@@ -114,6 +116,7 @@ enum cxlflash_hwq_mode {
 struct cxlflash_cfg {
struct afu *afu;
 
+   const struct cxlflash_backend_ops *ops;
struct pci_dev *dev;
struct pci_device_id *dev_id;
struct Scsi_Host *host;
diff --git a/drivers/scsi/cxlflash/cxl_hw.c b/drivers/scsi/cxlflash/cxl_hw.c
new file mode 100644
index 000..db1cada
--- /dev/null
+++ b/drivers/scsi/cxlflash/cxl_hw.c
@@ -0,0 +1,168 @@
+/*
+ * CXL Flash Device Driver
+ *
+ * Written by: Matthew R. Ochs , IBM Corporation
+ * Uma Krishnan , IBM Corporation
+ *
+ * Copyright (C) 2018 IBM Corporation
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#include 
+
+#include "backend.h"
+
+/*
+ * The following routines map the cxlflash backend operations to existing CXL
+ * kernel API function and are largely simple shims that provide an abstraction
+ * for converting generic context and AFU cookies into cxl_context or cxl_afu
+ * pointers.
+ */
+
+static void __iomem *cxlflash_psa_map(void *ctx_cookie)
+{
+   return cxl_psa_map(ctx_cookie);
+}
+
+static void cxlflash_psa_unmap(void __iomem *addr)
+{
+   cxl_psa_unmap(addr);
+}
+
+static int cxlflash_process_element(void *ctx_cookie)
+{
+   return 

[PATCH 2/6] cxlflash: Update cxl-specific arguments to generic cookie

[Uma Krishnan]

Convert cxl-specific pointers to generic cookies to facilitate future
enhancements.

Signed-off-by: Uma Krishnan 
---
 drivers/scsi/cxlflash/common.h|  4 ++--
 drivers/scsi/cxlflash/main.c  | 38 +++---
 drivers/scsi/cxlflash/superpipe.c | 15 +++
 drivers/scsi/cxlflash/superpipe.h |  2 +-
 4 files changed, 29 insertions(+), 30 deletions(-)

diff --git a/drivers/scsi/cxlflash/common.h b/drivers/scsi/cxlflash/common.h
index 6d95e8e..d2a180d 100644
--- a/drivers/scsi/cxlflash/common.h
+++ b/drivers/scsi/cxlflash/common.h
@@ -129,7 +129,7 @@ struct cxlflash_cfg {
int lr_port;
atomic_t scan_host_needed;
 
-   struct cxl_afu *cxl_afu;
+   void *afu_cookie;
 
atomic_t recovery_threads;
struct mutex ctx_recovery_mutex;
@@ -203,7 +203,7 @@ struct hwq {
 * fields after this point
 */
struct afu *afu;
-   struct cxl_context *ctx;
+   void *ctx_cookie;
struct cxl_ioctl_start_work work;
struct sisl_host_map __iomem *host_map; /* MC host map */
struct sisl_ctrl_map __iomem *ctrl_map; /* MC control map */
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 48d3663..3880d52 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -739,7 +739,7 @@ static void term_intr(struct cxlflash_cfg *cfg, enum 
undo_level level,
 
hwq = get_hwq(afu, index);
 
-   if (!hwq->ctx) {
+   if (!hwq->ctx_cookie) {
dev_err(dev, "%s: returning with NULL MC\n", __func__);
return;
}
@@ -748,13 +748,13 @@ static void term_intr(struct cxlflash_cfg *cfg, enum 
undo_level level,
case UNMAP_THREE:
/* SISL_MSI_ASYNC_ERROR is setup only for the primary HWQ */
if (index == PRIMARY_HWQ)
-   cxl_unmap_afu_irq(hwq->ctx, 3, hwq);
+   cxl_unmap_afu_irq(hwq->ctx_cookie, 3, hwq);
case UNMAP_TWO:
-   cxl_unmap_afu_irq(hwq->ctx, 2, hwq);
+   cxl_unmap_afu_irq(hwq->ctx_cookie, 2, hwq);
case UNMAP_ONE:
-   cxl_unmap_afu_irq(hwq->ctx, 1, hwq);
+   cxl_unmap_afu_irq(hwq->ctx_cookie, 1, hwq);
case FREE_IRQ:
-   cxl_free_afu_irqs(hwq->ctx);
+   cxl_free_afu_irqs(hwq->ctx_cookie);
/* fall through */
case UNDO_NOOP:
/* No action required */
@@ -783,15 +783,15 @@ static void term_mc(struct cxlflash_cfg *cfg, u32 index)
 
hwq = get_hwq(afu, index);
 
-   if (!hwq->ctx) {
+   if (!hwq->ctx_cookie) {
dev_err(dev, "%s: returning with NULL MC\n", __func__);
return;
}
 
-   WARN_ON(cxl_stop_context(hwq->ctx));
+   WARN_ON(cxl_stop_context(hwq->ctx_cookie));
if (index != PRIMARY_HWQ)
-   WARN_ON(cxl_release_context(hwq->ctx));
-   hwq->ctx = NULL;
+   WARN_ON(cxl_release_context(hwq->ctx_cookie));
+   hwq->ctx_cookie = NULL;
 
spin_lock_irqsave(>hsq_slock, lock_flags);
flush_pending_cmds(hwq);
@@ -1611,7 +1611,7 @@ static int start_context(struct cxlflash_cfg *cfg, u32 
index)
struct hwq *hwq = get_hwq(cfg->afu, index);
int rc = 0;
 
-   rc = cxl_start_context(hwq->ctx,
+   rc = cxl_start_context(hwq->ctx_cookie,
   hwq->work.work_element_descriptor,
   NULL);
 
@@ -1748,7 +1748,7 @@ static void init_pcr(struct cxlflash_cfg *cfg)
for (i = 0; i < afu->num_hwqs; i++) {
hwq = get_hwq(afu, i);
 
-   hwq->ctx_hndl = (u16) cxl_process_element(hwq->ctx);
+   hwq->ctx_hndl = (u16) cxl_process_element(hwq->ctx_cookie);
hwq->host_map = >afu_map->hosts[hwq->ctx_hndl].host;
hwq->ctrl_map = >afu_map->ctrls[hwq->ctx_hndl].ctrl;
 
@@ -1926,7 +1926,7 @@ static enum undo_level init_intr(struct cxlflash_cfg *cfg,
 struct hwq *hwq)
 {
struct device *dev = >dev->dev;
-   struct cxl_context *ctx = hwq->ctx;
+   void *ctx = hwq->ctx_cookie;
int rc = 0;
enum undo_level level = UNDO_NOOP;
bool is_primary_hwq = (hwq->index == PRIMARY_HWQ);
@@ -1980,7 +1980,7 @@ static enum undo_level init_intr(struct cxlflash_cfg *cfg,
  */
 static int init_mc(struct cxlflash_cfg *cfg, u32 index)
 {
-   struct cxl_context *ctx;
+   void *ctx;
struct device *dev = >dev->dev;
struct hwq *hwq = get_hwq(cfg->afu, index);
int rc = 0;
@@ -1999,8 +1999,8 @@ static int init_mc(struct cxlflash_cfg *cfg, u32 index)
goto err1;
}
 
-   WARN_ON(hwq->ctx);
-   hwq->ctx = ctx;
+   WARN_ON(hwq->ctx_cookie);
+   hwq->ctx_cookie = ctx;
 
/* Set it up as a master with the CXL */
cxl_set_master(ctx);

[PATCH 1/6] cxlflash: Reset command ioasc

[Uma Krishnan]

In the event of a command failure, cxlflash returns the failure to the
upper layers to process. After processing the error, when the command is
queued again, the private command structure will not be zeroed and the
ioasc could be stale. Per the SISLite specification, the AFU only sets the
ioasc in the presence of a failure. Thus, even though the original command
succeeds the second time, the command is considered a failure due to stale
ioasc. This cycle repeats indefinitely and can cause a hang or IO failure.

To fix the issue, clear the ioasc before queuing any command.

Fixes: 479ad8e9d48c ("scsi: cxlflash: Remove zeroing of private command
data")
Signed-off-by: Uma Krishnan 
---
 drivers/scsi/cxlflash/main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 38b3a9c..48d3663 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -620,6 +620,7 @@ static int cxlflash_queuecommand(struct Scsi_Host *host, 
struct scsi_cmnd *scp)
cmd->parent = afu;
cmd->hwq_index = hwq_index;
 
+   cmd->sa.ioasc = 0;
cmd->rcb.ctx_id = hwq->ctx_hndl;
cmd->rcb.msi = SISL_MSI_RRQ_UPDATED;
cmd->rcb.port_sel = CHAN2PORTMASK(scp->device->channel);
-- 
2.1.0

[PATCH 3/6] cxlflash: Explicitly cache number of interrupts per context

[Uma Krishnan]

From: "Matthew R. Ochs" 

The number of interrupts a user requests during a context attach is
presently stored within the CXL work ioctl structure that is nested
alongside the per context metadata. Keeping this data in a structure
that is tied to a particular hardware implementation (CXL) will only
complicate matters when supporting newer accelerator transports.

Instead of relying upon the number of interrupts being cached within
a CXL-specific structure, explicitly cache the value within the context
information structure.

Signed-off-by: Matthew R. Ochs 
Signed-off-by: Uma Krishnan 
---
 drivers/scsi/cxlflash/superpipe.c | 14 +-
 drivers/scsi/cxlflash/superpipe.h |  1 +
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/cxlflash/superpipe.c 
b/drivers/scsi/cxlflash/superpipe.c
index 18f6240..ecfa553 100644
--- a/drivers/scsi/cxlflash/superpipe.c
+++ b/drivers/scsi/cxlflash/superpipe.c
@@ -814,15 +814,18 @@ static struct ctx_info *create_context(struct 
cxlflash_cfg *cfg)
  * @ctxid: Previously obtained process element associated with CXL context.
  * @file:  Previously obtained file associated with CXL context.
  * @perms: User-specified permissions.
+ * @irqs:  User-specified number of interrupts.
  */
 static void init_context(struct ctx_info *ctxi, struct cxlflash_cfg *cfg,
-void *ctx, int ctxid, struct file *file, u32 perms)
+void *ctx, int ctxid, struct file *file, u32 perms,
+u64 irqs)
 {
struct afu *afu = cfg->afu;
 
ctxi->rht_perms = perms;
ctxi->ctrl_map = >afu_map->ctrls[ctxid].ctrl;
ctxi->ctxid = ENCODE_CTXID(ctxi, ctxid);
+   ctxi->irqs = irqs;
ctxi->pid = task_tgid_nr(current); /* tgid = pid */
ctxi->ctx = ctx;
ctxi->cfg = cfg;
@@ -1312,6 +1315,7 @@ static int cxlflash_disk_attach(struct scsi_device *sdev,
int rc = 0;
u32 perms;
int ctxid = -1;
+   u64 irqs = attach->num_interrupts;
u64 flags = 0UL;
u64 rctxid = 0UL;
struct file *file = NULL;
@@ -1320,9 +1324,9 @@ static int cxlflash_disk_attach(struct scsi_device *sdev,
 
int fd = -1;
 
-   if (attach->num_interrupts > 4) {
+   if (irqs > 4) {
dev_dbg(dev, "%s: Cannot support this many interrupts %llu\n",
-   __func__, attach->num_interrupts);
+   __func__, irqs);
rc = -EINVAL;
goto out;
}
@@ -1402,7 +1406,7 @@ static int cxlflash_disk_attach(struct scsi_device *sdev,
}
 
work = >work;
-   work->num_interrupts = attach->num_interrupts;
+   work->num_interrupts = irqs;
work->flags = CXL_START_WORK_NUM_IRQS;
 
rc = cxl_start_work(ctx, work);
@@ -1430,7 +1434,7 @@ static int cxlflash_disk_attach(struct scsi_device *sdev,
perms = SISL_RHT_PERM(attach->hdr.flags + 1);
 
/* Context mutex is locked upon return */
-   init_context(ctxi, cfg, ctx, ctxid, file, perms);
+   init_context(ctxi, cfg, ctx, ctxid, file, perms, irqs);
 
rc = afu_attach(cfg, ctxi);
if (unlikely(rc)) {
diff --git a/drivers/scsi/cxlflash/superpipe.h 
b/drivers/scsi/cxlflash/superpipe.h
index 62097df..b761293 100644
--- a/drivers/scsi/cxlflash/superpipe.h
+++ b/drivers/scsi/cxlflash/superpipe.h
@@ -98,6 +98,7 @@ struct ctx_info {
 
struct cxl_ioctl_start_work work;
u64 ctxid;
+   u64 irqs; /* Number of interrupts requested for context */
pid_t pid;
bool initialized;
bool unavail;
-- 
2.1.0

[PATCH 4/6] cxlflash: Remove embedded CXL work structures

[Uma Krishnan]

From: "Matthew R. Ochs" 

The CXL-specific work structure used to request the number of interrupts
currently resides as a nested member of both the context information and
hardware queue structures. It is used to cache values (specifically the
number of interrupts) required by the CXL layer when starting a context.

To facilitate staging that will ultimately allow the cxlflash core to
become agnostic of the underlying accelerator transport, remove these
embedded work structures.

Signed-off-by: Matthew R. Ochs 
Signed-off-by: Uma Krishnan 
---
 drivers/scsi/cxlflash/common.h|  1 -
 drivers/scsi/cxlflash/main.c  |  4 +---
 drivers/scsi/cxlflash/superpipe.c | 15 +--
 drivers/scsi/cxlflash/superpipe.h |  1 -
 4 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/scsi/cxlflash/common.h b/drivers/scsi/cxlflash/common.h
index d2a180d..48df89f 100644
--- a/drivers/scsi/cxlflash/common.h
+++ b/drivers/scsi/cxlflash/common.h
@@ -204,7 +204,6 @@ struct hwq {
 */
struct afu *afu;
void *ctx_cookie;
-   struct cxl_ioctl_start_work work;
struct sisl_host_map __iomem *host_map; /* MC host map */
struct sisl_ctrl_map __iomem *ctrl_map; /* MC control map */
ctx_hndl_t ctx_hndl;/* master's context handle */
diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 3880d52..b6cadeb 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -1611,9 +1611,7 @@ static int start_context(struct cxlflash_cfg *cfg, u32 
index)
struct hwq *hwq = get_hwq(cfg->afu, index);
int rc = 0;
 
-   rc = cxl_start_context(hwq->ctx_cookie,
-  hwq->work.work_element_descriptor,
-  NULL);
+   rc = cxl_start_context(hwq->ctx_cookie, 0, NULL);
 
dev_dbg(dev, "%s: returning rc=%d\n", __func__, rc);
return rc;
diff --git a/drivers/scsi/cxlflash/superpipe.c 
b/drivers/scsi/cxlflash/superpipe.c
index ecfa553..51f67dc 100644
--- a/drivers/scsi/cxlflash/superpipe.c
+++ b/drivers/scsi/cxlflash/superpipe.c
@@ -1309,7 +1309,7 @@ static int cxlflash_disk_attach(struct scsi_device *sdev,
struct afu *afu = cfg->afu;
struct llun_info *lli = sdev->hostdata;
struct glun_info *gli = lli->parent;
-   struct cxl_ioctl_start_work *work;
+   struct cxl_ioctl_start_work work = { 0 };
struct ctx_info *ctxi = NULL;
struct lun_access *lun_access = NULL;
int rc = 0;
@@ -1405,11 +1405,10 @@ static int cxlflash_disk_attach(struct scsi_device 
*sdev,
goto err;
}
 
-   work = >work;
-   work->num_interrupts = irqs;
-   work->flags = CXL_START_WORK_NUM_IRQS;
+   work.num_interrupts = irqs;
+   work.flags = CXL_START_WORK_NUM_IRQS;
 
-   rc = cxl_start_work(ctx, work);
+   rc = cxl_start_work(ctx, );
if (unlikely(rc)) {
dev_dbg(dev, "%s: Could not start context rc=%d\n",
__func__, rc);
@@ -1534,6 +1533,7 @@ static int recover_context(struct cxlflash_cfg *cfg,
struct file *file;
void *ctx;
struct afu *afu = cfg->afu;
+   struct cxl_ioctl_start_work work = { 0 };
 
ctx = cxl_dev_context_init(cfg->dev);
if (IS_ERR_OR_NULL(ctx)) {
@@ -1543,7 +1543,10 @@ static int recover_context(struct cxlflash_cfg *cfg,
goto out;
}
 
-   rc = cxl_start_work(ctx, >work);
+   work.num_interrupts = ctxi->irqs;
+   work.flags = CXL_START_WORK_NUM_IRQS;
+
+   rc = cxl_start_work(ctx, );
if (unlikely(rc)) {
dev_dbg(dev, "%s: Could not start context rc=%d\n",
__func__, rc);
diff --git a/drivers/scsi/cxlflash/superpipe.h 
b/drivers/scsi/cxlflash/superpipe.h
index b761293..35c3cbf 100644
--- a/drivers/scsi/cxlflash/superpipe.h
+++ b/drivers/scsi/cxlflash/superpipe.h
@@ -96,7 +96,6 @@ struct ctx_info {
struct llun_info **rht_lun;   /* Mapping of RHT entries to LUNs */
u8 *rht_needs_ws;   /* User-desired write-same function per RHTE */
 
-   struct cxl_ioctl_start_work work;
u64 ctxid;
u64 irqs; /* Number of interrupts requested for context */
pid_t pid;
-- 
2.1.0

[PATCH 5/6] cxlflash: Adapter context init can return error

[Uma Krishnan]

Adapter context creation can return either NULL or an error pointer.
Updating the check condition to reflect this.

Signed-off-by: Uma Krishnan 
---
 drivers/scsi/cxlflash/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index b6cadeb..32014e8 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -1992,7 +1992,7 @@ static int init_mc(struct cxlflash_cfg *cfg, u32 index)
ctx = cxl_get_context(cfg->dev);
else
ctx = cxl_dev_context_init(cfg->dev);
-   if (unlikely(!ctx)) {
+   if (IS_ERR_OR_NULL(ctx)) {
rc = -ENOMEM;
goto err1;
}
-- 
2.1.0

Re: [PATCH v2.2] sd: Micro-optimize READ / WRITE CDB encoding

[Douglas Gilbert]

On 2017-10-26 02:31 AM, Martin K. Petersen wrote:


Doug,


The sd_setup_read_write_cmnd() function is on the "fast path" for
block system access to SCSI devices (logical units). Rewrite this
function to improve speed and readability.


Please do any optimizations on top of my scsi-work branch which has the
sd_setup_read_write_cmnd() rewrite. I'm at kernel summit this week but
will rebase when time permits.


The most recent patch I can see on sd.c and specifically the function
sd_setup_read_write_cmnd() is May last year. Until it gets rebased
it is not useful for building further patches on.

Doug Gilbert

[PATCH 0/6] cxlflash: Miscellaneous patches

[Uma Krishnan]

This patch series contains miscellaneous fixes. The first patch fixes a
bug while the rest improve the code structure and prepare the code for
future enhancements.

This series is intended for 4.16 and is bisectable.

Matthew R. Ochs (3):
  cxlflash: Explicitly cache number of interrupts per context
  cxlflash: Remove embedded CXL work structures
  cxlflash: Staging to support future accelerators

Uma Krishnan (3):
  cxlflash: Reset command ioasc
  cxlflash: Update cxl-specific arguments to generic cookie
  cxlflash: Adapter context init can return error

 drivers/scsi/cxlflash/Makefile|   2 +-
 drivers/scsi/cxlflash/backend.h   |  41 ++
 drivers/scsi/cxlflash/common.h|   8 +-
 drivers/scsi/cxlflash/cxl_hw.c| 168 ++
 drivers/scsi/cxlflash/main.c  | 100 +--
 drivers/scsi/cxlflash/superpipe.c |  64 +++
 drivers/scsi/cxlflash/superpipe.h |   4 +-
 7 files changed, 288 insertions(+), 99 deletions(-)
 create mode 100644 drivers/scsi/cxlflash/backend.h
 create mode 100644 drivers/scsi/cxlflash/cxl_hw.c

-- 
2.1.0

[PATCH v2 4/4] scsi: pm8001: fix dma_unmap_sg() parameter

[chenxiang]

For function dma_unmap_sg(), the  parameter should be number of
elements in the scatterlist prior to the mapping, not after the mapping.
Fix this usage.

Cc: Jack Wang 
Cc: lindar_...@usish.com
Fixes: dbf9bfe6("[SCSI]pm8001: add SAS/SATA/HBA driver")
Signed-off-by: Xiang Chen 
---
 drivers/scsi/pm8001/pm8001_sas.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c
index 947d601..576a0f0 100644
--- a/drivers/scsi/pm8001/pm8001_sas.c
+++ b/drivers/scsi/pm8001/pm8001_sas.c
@@ -466,7 +466,7 @@ static int pm8001_task_exec(struct sas_task *task,
dev_printk(KERN_ERR, pm8001_ha->dev, "pm8001 exec failed[%d]!\n", rc);
if (!sas_protocol_ata(t->task_proto))
if (n_elem)
-   dma_unmap_sg(pm8001_ha->dev, t->scatter, n_elem,
+   dma_unmap_sg(pm8001_ha->dev, t->scatter, t->num_scatter,
t->data_dir);
 out_done:
spin_unlock_irqrestore(_ha->lock, flags);
-- 
1.9.1

[PATCH v2 2/4] scsi: isci: fix dma_unmap_sg() parameter

[chenxiang]

For function dma_unmap_sg(), the  parameter should be number of
elements in the scatterlist prior to the mapping, not after the mapping.
Fix this usage.

Cc: Artur Paszkiewicz 
Cc: Dan Williams 
Fixes: d9dcb4ba7("isci: unify isci_host and scic_sds_controller")
Signed-off-by: Xiang Chen 
---
 drivers/scsi/isci/request.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/isci/request.c b/drivers/scsi/isci/request.c
index ed197bc..225d947 100644
--- a/drivers/scsi/isci/request.c
+++ b/drivers/scsi/isci/request.c
@@ -2914,7 +2914,7 @@ static void isci_request_io_request_complete(struct 
isci_host *ihost,
 task->total_xfer_len, task->data_dir);
else  /* unmap the sgl dma addresses */
dma_unmap_sg(>pdev->dev, task->scatter,
-request->num_sg_entries, task->data_dir);
+task->num_scatter, task->data_dir);
break;
case SAS_PROTOCOL_SMP: {
struct scatterlist *sg = >smp_task.smp_req;
-- 
1.9.1

[PATCH v2 0/4] fix dma_unmap_sg() parameter in some scsi drivers

[chenxiang]

According to Documentation/DMA-API.txt, all the parameters of dma_unmap_sg()
must be the same as those and passed in to the scatter/gather mapping API.
But in scsi drivers such as ibmscsi_tgt/iscsi/mvsas/pm8001, the 
parameter of dma_unmap_sg() is number of elements after mapping. So fix them.

Part of the document is as follows:

void
dma_unmap_sg(struct device *dev, struct scatterlist *sg,
int nents, enum dma_data_direction direction)

Unmap the previously mapped scatter/gather list.  All the parameters
must be the same as those and passed in to the scatter/gather mapping
API.

Note:  must be the number you passed in, *not* the number of
DMA address entries returned.

Chang Log:
v1 -> v2:
Split the patch into small patchset, and one patch per driver;

chenxiang (4):
  scsi: ibmvscsis: fix dma_unmap_sg() parameter
  scsi: isci: fix dma_unmap_sg() parameter
  scsi: mvsas: fix dma_unmap_sg() parameter
  scsi: pm8001: fix dma_unmap_sg() parameter

 drivers/scsi/ibmvscsi_tgt/libsrp.c | 6 --
 drivers/scsi/isci/request.c| 2 +-
 drivers/scsi/mvsas/mv_sas.c| 4 ++--
 drivers/scsi/pm8001/pm8001_sas.c   | 2 +-
 4 files changed, 8 insertions(+), 6 deletions(-)

-- 
1.9.1

[PATCH v2 3/4] scsi: mvsas: fix dma_unmap_sg() parameter

[chenxiang]

For function dma_unmap_sg(), the  parameter should be number of
elements in the scatterlist prior to the mapping, not after the mapping.
Fix this usage.

Cc: Xiangliang Yu 
Cc: Jeff Garzik 
Fixes: 0b15fb1fd("[SCSI]mvsas: add support for Task collector mode and fixed 
relative
bugs")
Signed-off-by: Xiang Chen 
---
 drivers/scsi/mvsas/mv_sas.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/mvsas/mv_sas.c b/drivers/scsi/mvsas/mv_sas.c
index cff43bd..4b2cf36 100644
--- a/drivers/scsi/mvsas/mv_sas.c
+++ b/drivers/scsi/mvsas/mv_sas.c
@@ -848,7 +848,7 @@ static int mvs_task_prep(struct sas_task *task, struct 
mvs_info *mvi, int is_tmf
dev_printk(KERN_ERR, mvi->dev, "mvsas prep failed[%d]!\n", rc);
if (!sas_protocol_ata(task->task_proto))
if (n_elem)
-   dma_unmap_sg(mvi->dev, task->scatter, n_elem,
+   dma_unmap_sg(mvi->dev, task->scatter, task->num_scatter,
 task->data_dir);
 prep_out:
return rc;
@@ -899,7 +899,7 @@ static void mvs_slot_task_free(struct mvs_info *mvi, struct 
sas_task *task,
if (!sas_protocol_ata(task->task_proto))
if (slot->n_elem)
dma_unmap_sg(mvi->dev, task->scatter,
-slot->n_elem, task->data_dir);
+task->num_scatter, task->data_dir);
 
switch (task->task_proto) {
case SAS_PROTOCOL_SMP:
-- 
1.9.1

[PATCH v2 1/4] scsi: ibmvscsis: fix dma_unmap_sg() parameter

[chenxiang]

For function dma_unmap_sg(), the  parameter should be number of
elements in the scatterlist prior to the mapping, not after the mapping.
Fix this usage.

Cc: Bryant G. Ly 
Cc: Michael Cyr 
Fixes: 88a678bb("ibmscsis: Initial commit of IBM VSCSI Tgt Driver")
Signed-off-by: Xiang Chen 
---
 drivers/scsi/ibmvscsi_tgt/libsrp.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/ibmvscsi_tgt/libsrp.c 
b/drivers/scsi/ibmvscsi_tgt/libsrp.c
index 5a4cc28..16054c0 100644
--- a/drivers/scsi/ibmvscsi_tgt/libsrp.c
+++ b/drivers/scsi/ibmvscsi_tgt/libsrp.c
@@ -193,7 +193,8 @@ static int srp_direct_data(struct ibmvscsis_cmd *cmd, 
struct srp_direct_buf *md,
err = rdma_io(cmd, sg, nsg, md, 1, dir, len);
 
if (dma_map)
-   dma_unmap_sg(iue->target->dev, sg, nsg, DMA_BIDIRECTIONAL);
+   dma_unmap_sg(iue->target->dev, sg, cmd->se_cmd.t_data_nents,
+   DMA_BIDIRECTIONAL);
 
return err;
 }
@@ -265,7 +266,8 @@ static int srp_indirect_data(struct ibmvscsis_cmd *cmd, 
struct srp_cmd *srp_cmd,
err = rdma_io(cmd, sg, nsg, md, nmd, dir, len);
 
if (dma_map)
-   dma_unmap_sg(iue->target->dev, sg, nsg, DMA_BIDIRECTIONAL);
+   dma_unmap_sg(iue->target->dev, sg, cmd->se_cmd.t_data_nents,
+   DMA_BIDIRECTIONAL);
 
 free_mem:
if (token && dma_map) {
-- 
1.9.1

Re: [PATCH v1] libsas: remove private hex2bin() implementation

[Martin K. Petersen]

Andy,

> The function sas_parse_addr() could be easily substituted by hex2bin()
> which is in kernel library code.

Applied to 4.16/scsi-queue. Thank you!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] scsi: sd: Use warn for logs about no caching info

[Martin K. Petersen]

Hi Jeremy,

> When no caching mode information can be found for a disk, use the
> warning log level rather than error. It is common for this to occur
> with cheap USB sticks.

Just because something is common occurrence doesn't mean it's not an
error. What's the rationale behind demoting this to a warning?

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] mpt3sas: Proper handling of set/clear of "ATA command pending" flag.

[Martin K. Petersen]

Chaitra,

>  This patch fixes above two issues by setting of "ATA command pending" flag
>  after checking for whether device deleted, invalid device handle, device
>  busy with task management. And by setting "ATA command pending" flag to
>  false in all of the qcmd failure return paths after setting the flag.

Applied to 4.16/scsi-queue, thanks!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH 10/12] hpsa: drop unneeded newline

[Martin K. Petersen]

Julia,

> hpsa_show_dev_msg prints other information and a newline after the
> message string, so the message string does not need to include a
> newline explicitly.  Done using Coccinelle.

Applied to 4.16/scsi-queue, thanks.

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] scsi: remove extra white space at the end of the line

[Martin K. Petersen]

Johannes,

> All in all I'm so much in favour of this cleanup patch (and even would
> apprechiate more of these, especailly getting rid of all those
> camelCase foo we have in the scsi hba drivers).
>
> Let's see how Martin and James deceide about it.

I am thoroughly annoyed by all the legacy whitespace problems. I've been
working on two different sd patch series over the holidays and both
caused me no end of grief due to legacy formatting issues.

I have had an unbreak-sd patch sitting in my queue for several years but
never pulled the trigger on it. For the usual reasons.

I'm not particularly worried about bisection. But fixing whitespace does
make it harder on the distro backporting front (Very pleased that you
have now inadvertently volunteered to deal with all the issues that may
arise at SUSE from such a subsystem-wide cleanup :).

Anyway. I'm OK with fixing up the core pieces since they are the ones
that annoy me the most. But I'm not sure we should enforce cleanups on
drivers without an ack from the relevant maintainer. And for the
unmaintained legacy baggage, I'm just not sure it's worth the hassle to
clean things up. Fixing the crufty old things gives an illusion of the
driver being actively worked on. I'd rather see dead code being left as
such. Gives us a good indication of when it's safe to drop.

One thing I specifically don't want is to open the flood gates for
drive-by whitespace patches. I have no interest in wasting cycles on
that. I generally only take arbitrary 3rd party cleanups if a driver is
actively maintained and the maintainer specifically acks the change.

PS. I'll at least partially unbreak sd.c as part of the series I'll be
posting shortly.

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] scsi: libiscsi: Allow sd_shutdown on bad transport

[Martin K. Petersen]

Rafael,

> If, for any reason, userland shuts down iscsi transport interfaces
> before proper logouts - like when logging in to LUNs manually, without
> logging out on server shutdown, or when automated scripts can't
> umount/logout from logged LUNs - kernel will hang forever on its
> sd_sync_cache() logic, after issuing the SYNCHRONIZE_CACHE cmd to all
> still existent paths.

Applied to 4.16/scsi-queue. Thank you!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] scsi: scsi_transport_fc: fix typos on 64/128 GBit define names

[Martin K. Petersen]

James,

> The define names specified 64Bit/128Bit, not 64GBIT/128GBIT.
> Correct the names.

Applied to 4.16/scsi-queue. Thanks!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] scsi: lpfc: fix a couple of minor indentation issues

[Martin K. Petersen]

Colin,

> Several statements are indented too far, fix these

Applied to 4.16/scsi-queue.

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] scsi: lpfc: don't dereference localport before it has been null checked

[Martin K. Petersen]

Colin,

> localport is being dereferenced to assign lport and then immediately
> afterwards localport is being sanity checked to see if it is null.
> Fix this by only dereferencing localport until after it has been
> null checked.

Applied to 4.16/scsi-queue, thanks!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH 00/11] mpt3sas: Enable scsi MQ & lockless command submission

[Martin K. Petersen]

Suganath,

> This patch set is initially posted by Hannes Reinecke to enable scsi
> multiqueue for the mpt3sas driver.  While the HBA only has a single
> mailbox register for submitting commands, it does have individual
> receive queues per MSI-X interrupt and as such does benefit from
> converting it to full multiqueue support.

This does not apply to 4.16/scsi-queue. Please rebase.

Also, the first patch has some out-of-tree debris in it.

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] scsi: fix iscsi-related kernel-doc warnings

[Martin K. Petersen]

Randy,

> Fix kernel-doc warnings in drivers/scsi/ that are related to iscsi
> support interfaces.
>
> Fixes these kernel-doc warnings: (tested by adding these files to a new
> target.rst documentation file: WIP)

Applied to 4.16/scsi-queue, thanks!

> Cc:   "Nicholas A. Bellinger" 
> Cc:   linux-scsi@vger.kernel.org
> Cc:   target-de...@vger.kernel.org
> Cc:   Sagi Grimberg 
> Cc:   linux-r...@vger.kernel.org
> Cc:   "James E.J. Bottomley" 
> Cc:   "Martin K. Petersen" 

 ^^^ checkpatch complains about these spaces.

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH 00/30] aacraid: Refactor for sas transport and bug fixes

[Martin K. Petersen]

Raghava,

> This patchset primarily lays the foundation for adding sas transport
> support to the aacraid driver. Being said these patches are mainly
> code refactors, bug fixes and improvements.

Applied to 4.16/scsi-queue. Thank you!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH v2 00/37] qla2xxx: driver updates

[Martin K. Petersen]

Hi Himanshu!

> This series contains number of improvments in handling of switch
> registration commands in the driver. Switch commands are now submitted
> via IOCB patch asynchronously instead of mailbox interface.

Applied to 4.16/scsi-queue. However, ...

>  17 files changed, 2891 insertions(+), 1131 deletions(-)

Next time please don't submit a series this big. It is really, really
painful to review. Especially when some of the patches are huge.

I would much rather have 3-4 sets of 8-10 patches scattered throughout
the development cycle. Or a slow, steady trickle of bug fixes with maybe
a few bigger "new feature" series of logically related patches posted
separately to facilitate review.

Thanks!
Martin

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] scsi: fix iscsi-related kernel-doc warnings

[Randy Dunlap]

On 01/03/2018 08:11 PM, Martin K. Petersen wrote:
> 
> Randy,
> 
>> Fix kernel-doc warnings in drivers/scsi/ that are related to iscsi
>> support interfaces.
>>
>> Fixes these kernel-doc warnings: (tested by adding these files to a new
>> target.rst documentation file: WIP)
> 
> Applied to 4.16/scsi-queue, thanks!
> 
>> Cc:  "Nicholas A. Bellinger" 
>> Cc:  linux-scsi@vger.kernel.org
>> Cc:  target-de...@vger.kernel.org
>> Cc:  Sagi Grimberg 
>> Cc:  linux-r...@vger.kernel.org
>> Cc:  "James E.J. Bottomley" 
>> Cc:  "Martin K. Petersen" 
> 
>  ^^^ checkpatch complains about these spaces.

Hah, those are tabs, not spaces.

scripts/checkpatch.pl (comment) says something about this being a style thing:
# Check signature styles

WARNING: Use a single space after Cc:
#29: 
Cc: "Martin K. Petersen" 



Email standards (RFCs) sure don't require a single space.

and I would like to be able to copy N lines from MAINTAINERS file and then
replace either the M: and/or L: (actually just the 'M' or 'L') with
"To" or "Cc" and be done with it, instead of having to replace TAB on each
line with a space also.

Does anyone know where this "signature style" check came from?
I don't see it in any style guide.

thanks,
-- 
~Randy

Re: [PATCH v5 0/7] Enhance libsas hotplug feature

[Martin K. Petersen]

John,

> At this point we feel that we have a decent solution to the
> long-standing libsas hotplug issues.
>
> Hannes has kindly reviewed the series.
>
> Can you let us know what else you require for acceptance? More
> independent review or testing?

According to my notes, Hannes had some concerns wrt. one of the
patches. That's why I didn't merge the series.

Please address the comments and we'll take it from there.

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH] aacraid driver oops with dead battery

[Martin K. Petersen]

Meelis,

> The battery in my HP NetRAID-4M died of old age, and the aacraid
> driver started oopsing with NULL pointer dereference on startup after
> that.
>
> Fix it by reordering the init sequence to fill in function pointers
> before ioremapping memory, or dev->a_ops.adapter_ioremap pointer will
> be NULL.

Applied to 4.16/scsi-queue, thanks!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH 2/9] scsi: qla2xxx: Use zeroing allocator rather than allocator/memset

[Martin K. Petersen]

Himanshu,

> Use dma_zalloc_coherent and vzalloc instead of dma_alloc_coherent and
> vmalloc respectively, followed by memset 0.

Applied to 4.16/scsi-queue.

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH 1/9] scsi: qla4xxx: Use zeroing allocator rather than allocator/memset

[Martin K. Petersen]

Himanshu,

> Use dma_zalloc_coherent instead of dma_alloc_coherent followed by memset
> 0.

Applied to 4.16/scsi-queue.

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH 9/9] scsi: bnx2i: Use zeroing allocator rather than allocator/memset

[Martin K. Petersen]

Himanshu,

> Use dma_zalloc_coherent instead of dma_alloc_coherent followed by
> memset 0.

Applied to 4.16/scsi-queue.

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH v2 00/37] qla2xxx: driver updates

[Madhani, Himanshu]

Hi Martin, 

> On Jan 3, 2018, at 9:59 PM, Martin K. Petersen  
> wrote:
> 
> 
> Hi Himanshu!
> 
>> This series contains number of improvments in handling of switch
>> registration commands in the driver. Switch commands are now submitted
>> via IOCB patch asynchronously instead of mailbox interface.
> 
> Applied to 4.16/scsi-queue. However, ...
> 
>> 17 files changed, 2891 insertions(+), 1131 deletions(-)
> 
> Next time please don't submit a series this big. It is really, really
> painful to review. Especially when some of the patches are huge.
> 
> I would much rather have 3-4 sets of 8-10 patches scattered throughout
> the development cycle. Or a slow, steady trickle of bug fixes with maybe
> a few bigger "new feature" series of logically related patches posted
> separately to facilitate review.
> 

Understood. Will make sure to send out smaller series for future. 

> Thanks!
> Martin
> 
> -- 
> Martin K. PetersenOracle Linux Engineering

Thanks,
- Himanshu

Re: [PATCH 8/9] scsi: bfa: Use zeroing allocator rather than allocator/memset

[Martin K. Petersen]

Himanshu,

> Use vzalloc instead of vmalloc followed by memset 0.

Applied to 4.16/scsi-queue.

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH 3/9] scsi: qedi: Use zeroing allocator instead of allocator/memset

[Martin K. Petersen]

Himanshu,

> Use dma_zalloc_coherent instead of dma_alloc_coherent followed by memset
> 0.

Does not apply to 4.16/scsi-queue. Please resubmit. Thanks!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH 7/9] scsi: bnx2fc: Use zeroing allocator rather than allocator/memset

[Martin K. Petersen]

Himanshu,

> Use dma_zalloc_coherent instead of dma_alloc_coherent followed by
> memset 0.

Does not apply to 4.16/scsi-queue. Please resubmit. Thanks!

-- 
Martin K. Petersen  Oracle Linux Engineering

Re: [PATCH 1/6] cxlflash: Reset command ioasc

[Andrew Donnellan]

On 04/01/18 09:54, Uma Krishnan wrote:

In the event of a command failure, cxlflash returns the failure to the
upper layers to process. After processing the error, when the command is
queued again, the private command structure will not be zeroed and the
ioasc could be stale. Per the SISLite specification, the AFU only sets the
ioasc in the presence of a failure. Thus, even though the original command
succeeds the second time, the command is considered a failure due to stale
ioasc. This cycle repeats indefinitely and can cause a hang or IO failure.

To fix the issue, clear the ioasc before queuing any command.

Fixes: 479ad8e9d48c ("scsi: cxlflash: Remove zeroing of private command
data")
Signed-off-by: Uma Krishnan 


Should this go to stable?



---
  drivers/scsi/cxlflash/main.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
index 38b3a9c..48d3663 100644
--- a/drivers/scsi/cxlflash/main.c
+++ b/drivers/scsi/cxlflash/main.c
@@ -620,6 +620,7 @@ static int cxlflash_queuecommand(struct Scsi_Host *host, 
struct scsi_cmnd *scp)
cmd->parent = afu;
cmd->hwq_index = hwq_index;

+   cmd->sa.ioasc = 0;
cmd->rcb.ctx_id = hwq->ctx_hndl;
cmd->rcb.msi = SISL_MSI_RRQ_UPDATED;
cmd->rcb.port_sel = CHAN2PORTMASK(scp->device->channel);



--
Andrew Donnellan  OzLabs, ADL Canberra
andrew.donnel...@au1.ibm.com  IBM Australia Limited

Re: [PATCH 7/9] scsi: bnx2fc: Use zeroing allocator rather than allocator/memset

[Himanshu Jha]

Hello Martin,

On Thu, Jan 04, 2018 at 01:17:52AM -0500, Martin K. Petersen wrote:
> 
> Himanshu,
> 
> > Use dma_zalloc_coherent instead of dma_alloc_coherent followed by
> > memset 0.
> 
> Does not apply to 4.16/scsi-queue. Please resubmit. Thanks!

No problem, I will resend the patch.

scsi: qla4xxx: Use zeroing allocator rather than allocator/memset --Applied
scsi: qla2xxx: Use zeroing allocator rather than allocator/memset --Applied
scsi: qedi: Use zeroing allocator instead of allocator/memset --Resend
scsi: mvsas: Use zeroing allocator rather than allocator/memset
scsi: fnic: Use zeroing allocator rather than allocator/memset
scsi: dpt_i2o: Use zeroing allocator rather than allocator/memset
scsi: bnx2fc: Use zeroing allocator rather than allocator/memset --Resend
scsi: bfa: Use zeroing allocator rather than allocator/memset --Applied
scsi: bnx2i: Use zeroing allocator rather than allocator/memset --Applied

But do I also need to send those mentioned above, which didn't receive
any response from you ?

Also, I later sent a patch for fnic after this series which can be
applied after or before this series whichever is suitable to you.

Thanks
Himanshu Jha

RE: [PATCH] aacraid driver oops with dead battery

[Raghava Aditya Renukunta]

> -Original Message-
> From: mr...@math.ut.ee [mailto:mr...@math.ut.ee] On Behalf Of Meelis
> Roos
> Sent: Wednesday, January 3, 2018 1:11 AM
> To: linux-scsi@vger.kernel.org; dl-esc-Aacraid Linux Driver
> 
> Subject: [PATCH] aacraid driver oops with dead battery
> 
> EXTERNAL EMAIL
> 
> 
> The battery in my HP NetRAID-4M died of old age, and the aacraid driver
> started oopsing with NULL pointer dereference on startup after that.
> 
> Fix it by reordering the init sequence to fill in function pointers
> before ioremapping memory, or dev->a_ops.adapter_ioremap pointer will
> be
> NULL.
> 
> Other subtypes of aacraid seem to have the order already correct.
> 
> This was the call trace:
> 
>  ? aac_probe_one+0x7a5/0xb30 [aacraid]
>  pci_device_probe+0xc0/0x1a0
>  driver_probe_device+0x1df/0x3b0
>  __driver_attach+0xa9/0xe0
>  ? driver_probe_device+0x3b0/0x3b0
>  bus_for_each_dev+0x4c/0x90
>  driver_attach+0x1d/0x40
>  ? driver_probe_device+0x3b0/0x3b0
>  bus_add_driver+0x1a7/0x2a0
>  driver_register+0x6e/0x130
>  __pci_register_driver+0x54/0x90
>  ? 0xf81f4000
>  aac_init+0x2b/0x1000 [aacraid]
>  do_one_initcall+0x45/0x1e0
>  ? kfree_skbmem+0x74/0xa0
>  ? kfree+0x16d/0x240
>  ? kvfree+0x45/0x50
>  ? kvfree+0x45/0x50
>  ? __vunmap+0x99/0x120
>  ? do_init_module+0x1a/0x245
>  do_init_module+0x83/0x245
>  load_module+0x2764/0x34a0
>  ? kernel_read_file+0x150/0x320
>  SyS_finit_module+0x82/0xa0
>  do_fast_syscall_32+0xba/0x340
> 
> Signed-off-by: Meelis Roos 
.

Reviewed-by: Raghava Aditya Renukunta 

Re: 4.15-rcX 32-bit SCSI suspend2ram warning

[Bart Van Assche]

On Wed, 2018-01-03 at 12:16 -0500, Woody Suwalski wrote:
> Starting with a 32-bit 4.15-rcX I have observed a warning in dmesg,
> happening when the system goes to suspend2ram. If it is just a warning
> - could we have it quiet'et down?

(+linux-scsi)

Thank you for having reported this. The comment above the warning that has
been triggered is as follows:

/*
 * It is allowed to call scsi_device_quiesce() multiple times from
 * the same context but concurrent scsi_device_quiesce() calls are
 * not allowed.
 */
WARN_ON_ONCE(sdev->quiesced_by && sdev->quiesced_by != current);

That means that suppressing the warning would be wrong - it would hide the
race condition but not fix it. Anyway, would it be possible to test the
(entirely untested) patch below? Sorry but I currently don't have access to
a test setup that triggers the SPI DV code.

Thanks,

Bart.


diff --git a/drivers/scsi/scsi_transport_spi.c 
b/drivers/scsi/scsi_transport_spi.c
index 10ebb213ddb3..abd48cffb282 100644
--- a/drivers/scsi/scsi_transport_spi.c
+++ b/drivers/scsi/scsi_transport_spi.c
@@ -26,6 +26,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include "scsi_priv.h"
 #include 
@@ -1009,11 +1010,20 @@ spi_dv_device(struct scsi_device *sdev)
u8 *buffer;
const int len = SPI_MAX_ECHO_BUFFER_SIZE*2;
 
+   /*
+* Because this function and the power management code both call
+* scsi_device_quiesce(), it is not safe to perform domain validation
+* while suspend or resume is in progress. Hence the
+* lock/unlock_system_sleep() calls.
+*/
+   lock_system_sleep();
+
if (unlikely(spi_dv_in_progress(starget)))
-   return;
+   goto unlock;
 
if (unlikely(scsi_device_get(sdev)))
-   return;
+   goto unlock;
+
spi_dv_in_progress(starget) = 1;
 
buffer = kzalloc(len, GFP_KERNEL);
@@ -1044,11 +1054,13 @@ spi_dv_device(struct scsi_device *sdev)
 
spi_initial_dv(starget) = 1;
 
- out_free:
+out_free:
kfree(buffer);
- out_put:
+out_put:
spi_dv_in_progress(starget) = 0;
scsi_device_put(sdev);
+unlock:
+   unlock_system_sleep();
 }
 EXPORT_SYMBOL(spi_dv_device);
 
-- 
2.15.1