[PATCH v2] st: convert DRIVER_ATTR macros to DRIVER_ATTR_RO

[Seymour, Shane M]

Convert DRIVER_ATTR macros to DRIVER_ATTR_RO requested by
Greg KH. Also switched to using scnprintf instead of snprintf
per Documentation/filesystems/sysfs.txt.

Suggested-by: Greg Kroah-Hartman 
Signed-off-by: Shane Seymour 
---
This patch was implemented on top of the previous patch to
convert to using driver attr groups.

Changes from v1:
- switched to scnprintf from sprintf after feedback from Sergey
Senozhatsky.
--- a/drivers/scsi/st.c 2015-06-22 20:38:16.061386739 -0500
+++ b/drivers/scsi/st.c 2015-06-23 19:59:21.302775649 -0500
@@ -4427,29 +4427,29 @@ module_exit(exit_st);
 
 
 /* The sysfs driver interface. Read-only at the moment */
-static ssize_t st_try_direct_io_show(struct device_driver *ddp, char *buf)
+static ssize_t try_direct_io_show(struct device_driver *ddp, char *buf)
 {
-   return snprintf(buf, PAGE_SIZE, "%d\n", try_direct_io);
+   return scnprintf(buf, PAGE_SIZE, "%d\n", try_direct_io);
 }
-static DRIVER_ATTR(try_direct_io, S_IRUGO, st_try_direct_io_show, NULL);
+static DRIVER_ATTR_RO(try_direct_io);
 
-static ssize_t st_fixed_buffer_size_show(struct device_driver *ddp, char *buf)
+static ssize_t fixed_buffer_size_show(struct device_driver *ddp, char *buf)
 {
-   return snprintf(buf, PAGE_SIZE, "%d\n", st_fixed_buffer_size);
+   return scnprintf(buf, PAGE_SIZE, "%d\n", st_fixed_buffer_size);
 }
-static DRIVER_ATTR(fixed_buffer_size, S_IRUGO, st_fixed_buffer_size_show, 
NULL);
+static DRIVER_ATTR_RO(fixed_buffer_size);
 
-static ssize_t st_max_sg_segs_show(struct device_driver *ddp, char *buf)
+static ssize_t max_sg_segs_show(struct device_driver *ddp, char *buf)
 {
-   return snprintf(buf, PAGE_SIZE, "%d\n", st_max_sg_segs);
+   return scnprintf(buf, PAGE_SIZE, "%d\n", st_max_sg_segs);
 }
-static DRIVER_ATTR(max_sg_segs, S_IRUGO, st_max_sg_segs_show, NULL);
+static DRIVER_ATTR_RO(max_sg_segs);
 
-static ssize_t st_version_show(struct device_driver *ddd, char *buf)
+static ssize_t version_show(struct device_driver *ddd, char *buf)
 {
-   return snprintf(buf, PAGE_SIZE, "[%s]\n", verstr);
+   return scnprintf(buf, PAGE_SIZE, "[%s]\n", verstr);
 }
-static DRIVER_ATTR(version, S_IRUGO, st_version_show, NULL);
+static DRIVER_ATTR_RO(version);
 
 static struct attribute *st_drv_attrs[] = {
&driver_attr_try_direct_io.attr,
--
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] st: convert DRIVER_ATTR macros to DRIVER_ATTR_RO

[Sergey Senozhatsky]

On (06/24/15 06:10), Seymour, Shane M wrote:
[..]
>  
>  /* The sysfs driver interface. Read-only at the moment */
> -static ssize_t st_try_direct_io_show(struct device_driver *ddp, char *buf)
> +static ssize_t try_direct_io_show(struct device_driver *ddp, char *buf)
>  {
> - return snprintf(buf, PAGE_SIZE, "%d\n", try_direct_io);
> + return sprintf(buf, "%d\n", try_direct_io);
>  }

a nitpick,

per Documentation/filesystems/sysfs.txt

:
: - show() should always use scnprintf().
:

  -ss

> -static DRIVER_ATTR(try_direct_io, S_IRUGO, st_try_direct_io_show, NULL);
> +static DRIVER_ATTR_RO(try_direct_io);
>  
> -static ssize_t st_fixed_buffer_size_show(struct device_driver *ddp, char 
> *buf)
> +static ssize_t fixed_buffer_size_show(struct device_driver *ddp, char *buf)
>  {
> - return snprintf(buf, PAGE_SIZE, "%d\n", st_fixed_buffer_size);
> + return sprintf(buf, "%d\n", st_fixed_buffer_size);
>  }
> -static DRIVER_ATTR(fixed_buffer_size, S_IRUGO, st_fixed_buffer_size_show, 
> NULL);
> +static DRIVER_ATTR_RO(fixed_buffer_size);
>  
> -static ssize_t st_max_sg_segs_show(struct device_driver *ddp, char *buf)
> +static ssize_t max_sg_segs_show(struct device_driver *ddp, char *buf)
>  {
> - return snprintf(buf, PAGE_SIZE, "%d\n", st_max_sg_segs);
> + return sprintf(buf, "%d\n", st_max_sg_segs);
>  }
> -static DRIVER_ATTR(max_sg_segs, S_IRUGO, st_max_sg_segs_show, NULL);
> +static DRIVER_ATTR_RO(max_sg_segs);
>  
> -static ssize_t st_version_show(struct device_driver *ddd, char *buf)
> +static ssize_t version_show(struct device_driver *ddd, char *buf)
>  {
> - return snprintf(buf, PAGE_SIZE, "[%s]\n", verstr);
> + return sprintf(buf, "[%s]\n", verstr);
>  }
> -static DRIVER_ATTR(version, S_IRUGO, st_version_show, NULL);
> +static DRIVER_ATTR_RO(version);
>  
>  static struct attribute *st_drv_attrs[] = {
>   &driver_attr_try_direct_io.attr,
> --
--
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] st: convert DRIVER_ATTR macros to DRIVER_ATTR_RO

[Seymour, Shane M]

Convert DRIVER_ATTR macros to DRIVER_ATTR_RO as requested by
Greg KH. Also switched to using sprintf as nothing printed should
exceed PAGE_SIZE - based on feedback from Greg when implementing
show functions for tape stats.

Suggested-by: Greg Kroah-Hartman 
Signed-off-by: Shane Seymour 
---
This patch was implemented on top of the previous patch to
convert to using driver attr groups.

Resending with [PATCH] at the front since I forgot to add it.
--- a/drivers/scsi/st.c 2015-06-22 20:38:16.061386739 -0500
+++ b/drivers/scsi/st.c 2015-06-23 17:29:03.547867682 -0500
@@ -4427,29 +4427,29 @@ module_exit(exit_st);
 
 
 /* The sysfs driver interface. Read-only at the moment */
-static ssize_t st_try_direct_io_show(struct device_driver *ddp, char *buf)
+static ssize_t try_direct_io_show(struct device_driver *ddp, char *buf)
 {
-   return snprintf(buf, PAGE_SIZE, "%d\n", try_direct_io);
+   return sprintf(buf, "%d\n", try_direct_io);
 }
-static DRIVER_ATTR(try_direct_io, S_IRUGO, st_try_direct_io_show, NULL);
+static DRIVER_ATTR_RO(try_direct_io);
 
-static ssize_t st_fixed_buffer_size_show(struct device_driver *ddp, char *buf)
+static ssize_t fixed_buffer_size_show(struct device_driver *ddp, char *buf)
 {
-   return snprintf(buf, PAGE_SIZE, "%d\n", st_fixed_buffer_size);
+   return sprintf(buf, "%d\n", st_fixed_buffer_size);
 }
-static DRIVER_ATTR(fixed_buffer_size, S_IRUGO, st_fixed_buffer_size_show, 
NULL);
+static DRIVER_ATTR_RO(fixed_buffer_size);
 
-static ssize_t st_max_sg_segs_show(struct device_driver *ddp, char *buf)
+static ssize_t max_sg_segs_show(struct device_driver *ddp, char *buf)
 {
-   return snprintf(buf, PAGE_SIZE, "%d\n", st_max_sg_segs);
+   return sprintf(buf, "%d\n", st_max_sg_segs);
 }
-static DRIVER_ATTR(max_sg_segs, S_IRUGO, st_max_sg_segs_show, NULL);
+static DRIVER_ATTR_RO(max_sg_segs);
 
-static ssize_t st_version_show(struct device_driver *ddd, char *buf)
+static ssize_t version_show(struct device_driver *ddd, char *buf)
 {
-   return snprintf(buf, PAGE_SIZE, "[%s]\n", verstr);
+   return sprintf(buf, "[%s]\n", verstr);
 }
-static DRIVER_ATTR(version, S_IRUGO, st_version_show, NULL);
+static DRIVER_ATTR_RO(version);
 
 static struct attribute *st_drv_attrs[] = {
&driver_attr_try_direct_io.attr,
--
To unsubscribe from this list: send the line "unsubscribe linux-api" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
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

st: convert DRIVER_ATTR macros to DRIVER_ATTR_RO

[Seymour, Shane M]

Convert DRIVER_ATTR macros to DRIVER_ATTR_RO as requested by
Greg KH. Also switched to using sprintf as nothing printed should
exceed PAGE_SIZE - based on feedback from Greg when implementing
show functions for tape stats.

Suggested-by: Greg Kroah-Hartman 
Signed-off-by: Shane Seymour 
---
This patch was implemented on top of the previous patch to
convert to using driver attr groups.
--- a/drivers/scsi/st.c 2015-06-22 20:38:16.061386739 -0500
+++ b/drivers/scsi/st.c 2015-06-23 17:29:03.547867682 -0500
@@ -4427,29 +4427,29 @@ module_exit(exit_st);
 
 
 /* The sysfs driver interface. Read-only at the moment */
-static ssize_t st_try_direct_io_show(struct device_driver *ddp, char *buf)
+static ssize_t try_direct_io_show(struct device_driver *ddp, char *buf)
 {
-   return snprintf(buf, PAGE_SIZE, "%d\n", try_direct_io);
+   return sprintf(buf, "%d\n", try_direct_io);
 }
-static DRIVER_ATTR(try_direct_io, S_IRUGO, st_try_direct_io_show, NULL);
+static DRIVER_ATTR_RO(try_direct_io);
 
-static ssize_t st_fixed_buffer_size_show(struct device_driver *ddp, char *buf)
+static ssize_t fixed_buffer_size_show(struct device_driver *ddp, char *buf)
 {
-   return snprintf(buf, PAGE_SIZE, "%d\n", st_fixed_buffer_size);
+   return sprintf(buf, "%d\n", st_fixed_buffer_size);
 }
-static DRIVER_ATTR(fixed_buffer_size, S_IRUGO, st_fixed_buffer_size_show, 
NULL);
+static DRIVER_ATTR_RO(fixed_buffer_size);
 
-static ssize_t st_max_sg_segs_show(struct device_driver *ddp, char *buf)
+static ssize_t max_sg_segs_show(struct device_driver *ddp, char *buf)
 {
-   return snprintf(buf, PAGE_SIZE, "%d\n", st_max_sg_segs);
+   return sprintf(buf, "%d\n", st_max_sg_segs);
 }
-static DRIVER_ATTR(max_sg_segs, S_IRUGO, st_max_sg_segs_show, NULL);
+static DRIVER_ATTR_RO(max_sg_segs);
 
-static ssize_t st_version_show(struct device_driver *ddd, char *buf)
+static ssize_t version_show(struct device_driver *ddd, char *buf)
 {
-   return snprintf(buf, PAGE_SIZE, "[%s]\n", verstr);
+   return sprintf(buf, "[%s]\n", verstr);
 }
-static DRIVER_ATTR(version, S_IRUGO, st_version_show, NULL);
+static DRIVER_ATTR_RO(version);
 
 static struct attribute *st_drv_attrs[] = {
&driver_attr_try_direct_io.attr,
--
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] megaraid:Make functions static in the file megaraid_sas_base.c

[Sumit Saxena]

-Original Message-
From: Nicholas Krause [mailto:xerofo...@gmail.com]
Sent: Wednesday, June 24, 2015 5:43 AM
To: kashyap.de...@avagotech.com
Cc: sumit.sax...@avagotech.com; uday.ling...@avagotech.com;
jbottom...@odin.com; megaraidlinux@avagotech.com;
linux-scsi@vger.kernel.org; linux-ker...@vger.kernel.org
Subject: [PATCH] megaraid:Make functions static in the file
megaraid_sas_base.c

This makes various functions that have no external callers outside their
definition/declaration in the file megaraid_sas_base.c to be declared as
static now.

Signed-off-by: Nicholas Krause 
---
 drivers/scsi/megaraid/megaraid_sas_base.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c
b/drivers/scsi/megaraid/megaraid_sas_base.c
index 4c3fc0e..dad2393 100644
--- a/drivers/scsi/megaraid/megaraid_sas_base.c
+++ b/drivers/scsi/megaraid/megaraid_sas_base.c
@@ -190,7 +190,7 @@ static int megasas_get_ld_vf_affiliation(struct
megasas_instance *instance,  int megasas_check_mpio_paths(struct
megasas_instance *instance,
 struct scsi_cmnd *scmd);

-void
+static void
 megasas_issue_dcmd(struct megasas_instance *instance, struct megasas_cmd
*cmd)  {
instance->instancet->fire_cmd(instance,
@@ -1698,7 +1698,7 @@ static int megasas_slave_alloc(struct scsi_device
*sdev)
 * @instance:   Adapter soft state
 *
 */
-void megasas_complete_outstanding_ioctls(struct megasas_instance
*instance)
+static void megasas_complete_outstanding_ioctls(struct megasas_instance
+*instance)
 {
int i;
struct megasas_cmd *cmd_mfi;
@@ -1856,7 +1856,7 @@ megasas_internal_reset_defer_cmds(struct
megasas_instance *instance);  static void
process_fw_state_change_wq(struct work_struct *work);

-void megasas_do_ocr(struct megasas_instance *instance)
+static void megasas_do_ocr(struct megasas_instance *instance)
 {
if ((instance->pdev->device == PCI_DEVICE_ID_LSI_SAS1064R) ||
(instance->pdev->device == PCI_DEVICE_ID_DELL_PERC5) ||

Acked-by: Sumit Saxena 

--
2.1.4
--
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 V2 9/9] [SCSI] aacraid: Update driver version

[Mahesh Rajashekhara]

Reviewed-by: Mahesh Rajashekhara 


-Original Message-
From: Rajinikanth Pandurangan 
Sent: Thursday, June 11, 2015 7:13 AM
To: jbottom...@parallels.com; linux-scsi@vger.kernel.org
Cc: aacr...@pmc-sierra.com; Harry Yang; Mahesh Rajashekhara; Rich Bono; Achim 
Leubner; Murthy Bhat; Rajinikanth Pandurangan
Subject: [Patch V2 9/9] [SCSI] aacraid: Update driver version

From: Rajinikanth Pandurangan 

Signed-off-by: Rajinikanth Pandurangan 
---
 drivers/scsi/aacraid/aacraid.h | 2 +-
 drivers/scsi/aacraid/linit.c   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h 
index 7b95227..73c3384 100644
--- a/drivers/scsi/aacraid/aacraid.h
+++ b/drivers/scsi/aacraid/aacraid.h
@@ -62,7 +62,7 @@ enum {
 #definePMC_GLOBAL_INT_BIT0 0x0001
 
 #ifndef AAC_DRIVER_BUILD
-# define AAC_DRIVER_BUILD 40709
+# define AAC_DRIVER_BUILD 41010
 # define AAC_DRIVER_BRANCH "-ms"
 #endif
 #define MAXIMUM_NUM_CONTAINERS 32
diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c index 
3df0dfb..1627928 100644
--- a/drivers/scsi/aacraid/linit.c
+++ b/drivers/scsi/aacraid/linit.c
@@ -56,7 +56,7 @@
 
 #include "aacraid.h"
 
-#define AAC_DRIVER_VERSION "1.2-1"
+#define AAC_DRIVER_VERSION "1.2-2"
 #ifndef AAC_DRIVER_BRANCH
 #define AAC_DRIVER_BRANCH  ""
 #endif
--
1.9.3

--
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 V2 8/9] [SCSI] aacraid: Send commit-config to controller firmware

[Mahesh Rajashekhara]

Reviewed-by: Mahesh Rajashekhara 


-Original Message-
From: Rajinikanth Pandurangan 
Sent: Thursday, June 11, 2015 7:13 AM
To: jbottom...@parallels.com; linux-scsi@vger.kernel.org
Cc: aacr...@pmc-sierra.com; Harry Yang; Mahesh Rajashekhara; Rich Bono; Achim 
Leubner; Murthy Bhat; Rajinikanth Pandurangan
Subject: [Patch V2 8/9] [SCSI] aacraid: Send commit-config to controller 
firmware

From: Rajinikanth Pandurangan 

Description:
Controller BIOS/UEFI driver used to send this request.  But for
IBM-Power system there is no BIOS/UEFI driver.  So this change is
required for IBM, otherwise controller will be read-only mode.

Signed-off-by: Rajinikanth Pandurangan 
---
 drivers/scsi/aacraid/linit.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c index 
1142c28..3df0dfb 100644
--- a/drivers/scsi/aacraid/linit.c
+++ b/drivers/scsi/aacraid/linit.c
@@ -1270,8 +1270,11 @@ static int aac_probe_one(struct pci_dev *pdev, const 
struct pci_device_id *id)
shost->max_channel = aac->maximum_num_channels;
else
shost->max_channel = 0;
-
+#if defined(__powerpc__) || defined(__PPC__) || defined(__ppc__)
+   aac_get_config_status(aac, 1);
+#else
aac_get_config_status(aac, 0);
+#endif
aac_get_containers(aac);
list_add(&aac->entry, insert);
 
--
1.9.3

--
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 V2 7/9] [SCSI] aacraid: Unblock IOCTLs to controller once system resumed from suspend

[Mahesh Rajashekhara]

Reviewed-by: Mahesh Rajashekhara 


-Original Message-
From: Rajinikanth Pandurangan 
Sent: Thursday, June 11, 2015 7:12 AM
To: jbottom...@parallels.com; linux-scsi@vger.kernel.org
Cc: aacr...@pmc-sierra.com; Harry Yang; Mahesh Rajashekhara; Rich Bono; Achim 
Leubner; Murthy Bhat; Rajinikanth Pandurangan
Subject: [Patch V2 7/9] [SCSI] aacraid: Unblock IOCTLs to controller once 
system resumed from suspend

From: Rajinikanth Pandurangan 

Description:
Driver blocks ioctls once it received shutdown/suspend request during
suspend/hybernation. This patch unblocks ioctls on resume path.

Signed-off-by: Rajinikanth Pandurangan 
---
 drivers/scsi/aacraid/linit.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c index 
8020348..1142c28 100644
--- a/drivers/scsi/aacraid/linit.c
+++ b/drivers/scsi/aacraid/linit.c
@@ -1448,6 +1448,11 @@ static int aac_resume(struct pci_dev *pdev)
pci_set_master(pdev);
if (aac_acquire_resources(aac))
goto fail_device;
+   /*
+   * reset this flag to unblock ioctl() as it was set at
+   * aac_send_shutdown() to block ioctls from upperlayer
+   */
+   aac->adapter_shutdown = 0;
scsi_unblock_requests(shost);
 
return 0;
--
1.9.3

--
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 V2 6/9] [SCSI] aacraid: Reset irq affinity hints before releasing irq

[Mahesh Rajashekhara]

Reviewed-by: Mahesh Rajashekhara 


-Original Message-
From: Rajinikanth Pandurangan 
Sent: Thursday, June 11, 2015 7:12 AM
To: jbottom...@parallels.com; linux-scsi@vger.kernel.org
Cc: aacr...@pmc-sierra.com; Harry Yang; Mahesh Rajashekhara; Rich Bono; Achim 
Leubner; Murthy Bhat; Rajinikanth Pandurangan
Subject: [Patch V2 6/9] [SCSI] aacraid: Reset irq affinity hints before 
releasing irq

From: Rajinikanth Pandurangan 

Description:
Reset irq affinity hints before releasing IRQ
Removed duplicate code of IRQ acquire/release

Signed-off-by: Rajinikanth Pandurangan 
---
 drivers/scsi/aacraid/aacraid.h |   2 +
 drivers/scsi/aacraid/commsup.c | 113 ++---
 drivers/scsi/aacraid/src.c |  48 ++---
 3 files changed, 88 insertions(+), 75 deletions(-)

diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h 
index e54f597..7b95227 100644
--- a/drivers/scsi/aacraid/aacraid.h
+++ b/drivers/scsi/aacraid/aacraid.h
@@ -2110,6 +2110,8 @@ static inline unsigned int cap_to_cyls(sector_t capacity, 
unsigned divisor)
 #define AAC_OWNER_ERROR_HANDLER0x103
 #define AAC_OWNER_FIRMWARE 0x106
 
+int aac_acquire_irq(struct aac_dev *dev); void aac_free_irq(struct 
+aac_dev *dev);
 const char *aac_driverinfo(struct Scsi_Host *);  struct fib 
*aac_fib_alloc(struct aac_dev *dev);  int aac_fib_setup(struct aac_dev *dev); 
diff --git a/drivers/scsi/aacraid/commsup.c b/drivers/scsi/aacraid/commsup.c 
index 4da5749..a1f90fe 100644
--- a/drivers/scsi/aacraid/commsup.c
+++ b/drivers/scsi/aacraid/commsup.c
@@ -1270,13 +1270,12 @@ retry_next:
 static int _aac_reset_adapter(struct aac_dev *aac, int forced)  {
int index, quirks;
-   int retval, i;
+   int retval;
struct Scsi_Host *host;
struct scsi_device *dev;
struct scsi_cmnd *command;
struct scsi_cmnd *command_list;
int jafo = 0;
-   int cpu;
 
/*
 * Assumptions:
@@ -1339,35 +1338,7 @@ static int _aac_reset_adapter(struct aac_dev *aac, int 
forced)
aac->comm_phys = 0;
kfree(aac->queues);
aac->queues = NULL;
-   cpu = cpumask_first(cpu_online_mask);
-   if (aac->pdev->device == PMC_DEVICE_S6 ||
-   aac->pdev->device == PMC_DEVICE_S7 ||
-   aac->pdev->device == PMC_DEVICE_S8 ||
-   aac->pdev->device == PMC_DEVICE_S9) {
-   if (aac->max_msix > 1) {
-   for (i = 0; i < aac->max_msix; i++) {
-   if (irq_set_affinity_hint(
-   aac->msixentry[i].vector,
-   NULL)) {
-   printk(KERN_ERR "%s%d: Failed to reset 
IRQ affinity for cpu %d\n",
-   aac->name,
-   aac->id,
-   cpu);
-   }
-   cpu = cpumask_next(cpu,
-   cpu_online_mask);
-   free_irq(aac->msixentry[i].vector,
-&(aac->aac_msix[i]));
-   }
-   pci_disable_msix(aac->pdev);
-   } else {
-   free_irq(aac->pdev->irq, &(aac->aac_msix[0]));
-   }
-   } else {
-   free_irq(aac->pdev->irq, aac);
-   }
-   if (aac->msi)
-   pci_disable_msi(aac->pdev);
+   aac_free_irq(aac);
kfree(aac->fsa_dev);
aac->fsa_dev = NULL;
quirks = aac_get_driver_ident(index)->quirks;
@@ -1978,3 +1949,83 @@ int aac_command_thread(void *data)
dev->aif_thread = 0;
return 0;
 }
+
+int aac_acquire_irq(struct aac_dev *dev) {
+   int i;
+   int j;
+   int ret = 0;
+   int cpu;
+
+   cpu = cpumask_first(cpu_online_mask);
+   if (!dev->sync_mode && dev->msi_enabled && dev->max_msix > 1) {
+   for (i = 0; i < dev->max_msix; i++) {
+   dev->aac_msix[i].vector_no = i;
+   dev->aac_msix[i].dev = dev;
+   if (request_irq(dev->msixentry[i].vector,
+   dev->a_ops.adapter_intr,
+   0, "aacraid", &(dev->aac_msix[i]))) {
+   printk(KERN_ERR "%s%d: Failed to register IRQ 
for vector %d.\n",
+   dev->name, dev->id, i);
+   for (j = 0 ; j < i ; j++)
+   free_irq(dev->msixentry[j].vector,
+&(dev->aac_msix[j]));
+   pci_disable_msix(dev->pdev);
+   ret = -1;
+   }
+   if (irq_set_affinity_hint(dev->msixentry[i].

RE: [Patch V2 5/9] [SCSI] aacraid: Tune response path if IsFastPath bit set

[Mahesh Rajashekhara]

Reviewed-by: Mahesh Rajashekhara 


-Original Message-
From: Rajinikanth Pandurangan 
Sent: Thursday, June 11, 2015 7:12 AM
To: jbottom...@parallels.com; linux-scsi@vger.kernel.org
Cc: aacr...@pmc-sierra.com; Harry Yang; Mahesh Rajashekhara; Rich Bono; Achim 
Leubner; Murthy Bhat; Rajinikanth Pandurangan
Subject: [Patch V2 5/9] [SCSI] aacraid: Tune response path if IsFastPath bit set

From: Rajinikanth Pandurangan 

Description:
If 'IsFastPath' bit is set, then response path assumes no error
and skips error check.

Signed-off-by: Rajinikanth Pandurangan 
---
 drivers/scsi/aacraid/aachba.c | 259 ++
 1 file changed, 137 insertions(+), 122 deletions(-)

diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c 
index fe59b00..864e9f6 100644
--- a/drivers/scsi/aacraid/aachba.c
+++ b/drivers/scsi/aacraid/aachba.c
@@ -2977,11 +2977,16 @@ static void aac_srb_callback(void *context, struct fib 
* fibptr)
return;
 
BUG_ON(fibptr == NULL);
-
dev = fibptr->dev;
 
-   srbreply = (struct aac_srb_reply *) fib_data(fibptr);
+   scsi_dma_unmap(scsicmd);
 
+   /* expose physical device if expose_physicald flag is on */
+   if (scsicmd->cmnd[0] == INQUIRY && !(scsicmd->cmnd[1] & 0x01)
+ && expose_physicals > 0)
+   aac_expose_phy_device(scsicmd);
+
+   srbreply = (struct aac_srb_reply *) fib_data(fibptr);
scsicmd->sense_buffer[0] = '\0';  /* Initialize sense valid flag to 
false */
 
if (fibptr->flags & FIB_CONTEXT_FLAG_FASTRESP) { @@ -2994,147 +2999,157 
@@ static void aac_srb_callback(void *context, struct fib * fibptr)
 */
scsi_set_resid(scsicmd, scsi_bufflen(scsicmd)
   - le32_to_cpu(srbreply->data_xfer_length));
-   }
-
-   scsi_dma_unmap(scsicmd);
-
-   /* expose physical device if expose_physicald flag is on */
-   if (scsicmd->cmnd[0] == INQUIRY && !(scsicmd->cmnd[1] & 0x01)
- && expose_physicals > 0)
-   aac_expose_phy_device(scsicmd);
+   /*
+* First check the fib status
+*/
 
-   /*
-* First check the fib status
-*/
+   if (le32_to_cpu(srbreply->status) != ST_OK) {
+   int len;
 
-   if (le32_to_cpu(srbreply->status) != ST_OK){
-   int len;
-   printk(KERN_WARNING "aac_srb_callback: srb failed, status = 
%d\n", le32_to_cpu(srbreply->status));
-   len = min_t(u32, le32_to_cpu(srbreply->sense_data_size),
-   SCSI_SENSE_BUFFERSIZE);
-   scsicmd->result = DID_ERROR << 16 | COMMAND_COMPLETE << 8 | 
SAM_STAT_CHECK_CONDITION;
-   memcpy(scsicmd->sense_buffer, srbreply->sense_data, len);
-   }
+   printk(KERN_WARNING "aac_srb_callback: srb failed, 
status = %d\n", le32_to_cpu(srbreply->status));
+   len = min_t(u32, le32_to_cpu(srbreply->sense_data_size),
+   SCSI_SENSE_BUFFERSIZE);
+   scsicmd->result = DID_ERROR << 16
+   | COMMAND_COMPLETE << 8
+   | SAM_STAT_CHECK_CONDITION;
+   memcpy(scsicmd->sense_buffer,
+   srbreply->sense_data, len);
+   }
 
-   /*
-* Next check the srb status
-*/
-   switch( (le32_to_cpu(srbreply->srb_status))&0x3f){
-   case SRB_STATUS_ERROR_RECOVERY:
-   case SRB_STATUS_PENDING:
-   case SRB_STATUS_SUCCESS:
-   scsicmd->result = DID_OK << 16 | COMMAND_COMPLETE << 8;
-   break;
-   case SRB_STATUS_DATA_OVERRUN:
-   switch(scsicmd->cmnd[0]){
-   case  READ_6:
-   case  WRITE_6:
-   case  READ_10:
-   case  WRITE_10:
-   case  READ_12:
-   case  WRITE_12:
-   case  READ_16:
-   case  WRITE_16:
-   if (le32_to_cpu(srbreply->data_xfer_length) < 
scsicmd->underflow) {
-   printk(KERN_WARNING"aacraid: SCSI CMD 
underflow\n");
-   } else {
-   printk(KERN_WARNING"aacraid: SCSI CMD Data 
Overrun\n");
+   /*
+* Next check the srb status
+*/
+   switch ((le32_to_cpu(srbreply->srb_status))&0x3f) {
+   case SRB_STATUS_ERROR_RECOVERY:
+   case SRB_STATUS_PENDING:
+   case SRB_STATUS_SUCCESS:
+   scsicmd->result = DID_OK << 16 | COMMAND_COMPLETE << 8;
+   break;
+   case SRB_STATUS_DATA_OVERRUN:
+   switch (scsicmd->cmnd[0]) {
+   case  READ_6:
+   cas

RE: [Patch V2 3/9] [SCSI] aacraid: Enable MSI interrupt for series-6 controller

[Mahesh Rajashekhara]

Reviewed-by: Mahesh Rajashekhara 


-Original Message-
From: Rajinikanth Pandurangan 
Sent: Thursday, June 11, 2015 7:12 AM
To: jbottom...@parallels.com; linux-scsi@vger.kernel.org
Cc: aacr...@pmc-sierra.com; Harry Yang; Mahesh Rajashekhara; Rich Bono; Achim 
Leubner; Murthy Bhat; Rajinikanth Pandurangan
Subject: [Patch V2 3/9] [SCSI] aacraid: Enable MSI interrupt for series-6 
controller

From: Rajinikanth Pandurangan 

Description:
Enable MSI interrupt mode for series-6 controller.

Signed-off-by: Rajinikanth Pandurangan 
---
 drivers/scsi/aacraid/src.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/aacraid/src.c b/drivers/scsi/aacraid/src.c index 
b147341..eb07b3d 100644
--- a/drivers/scsi/aacraid/src.c
+++ b/drivers/scsi/aacraid/src.c
@@ -742,7 +742,7 @@ int aac_src_init(struct aac_dev *dev)
if (dev->comm_interface != AAC_COMM_MESSAGE_TYPE1)
goto error_iounmap;
 
-   dev->msi = aac_msi && !pci_enable_msi(dev->pdev);
+   dev->msi = !pci_enable_msi(dev->pdev);
 
dev->aac_msix[0].vector_no = 0;
dev->aac_msix[0].dev = dev;
--
1.9.3

--
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 V2 2/9] [SCSI] aacraid: Add Power Management support

[Mahesh Rajashekhara]

Reviewed-by: Mahesh Rajashekhara 


-Original Message-
From: Rajinikanth Pandurangan 
Sent: Thursday, June 11, 2015 7:12 AM
To: jbottom...@parallels.com; linux-scsi@vger.kernel.org
Cc: aacr...@pmc-sierra.com; Harry Yang; Mahesh Rajashekhara; Rich Bono; Achim 
Leubner; Murthy Bhat; Rajinikanth Pandurangan
Subject: [Patch V2 2/9] [SCSI] aacraid: Add Power Management support

From: Rajinikanth Pandurangan 

Description:
* .suspend() and .resume() routines implemented in the driver
* aac_release_resources() initiates firmware shutdown
* aac_acquire_resources re-initializes the host interface

Signed-off-by: Rajinikanth Pandurangan 
---
 drivers/scsi/aacraid/aacraid.h  |   5 ++
 drivers/scsi/aacraid/comminit.c | 154 
 drivers/scsi/aacraid/linit.c| 147 ++
 drivers/scsi/aacraid/rx.c   |   1 +
 drivers/scsi/aacraid/sa.c   |   1 +
 drivers/scsi/aacraid/src.c  |   2 +
 6 files changed, 232 insertions(+), 78 deletions(-)

diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h 
index 40fe65c..62b0999 100644
--- a/drivers/scsi/aacraid/aacraid.h
+++ b/drivers/scsi/aacraid/aacraid.h
@@ -547,6 +547,7 @@ struct adapter_ops
int  (*adapter_sync_cmd)(struct aac_dev *dev, u32 command, u32 p1, u32 
p2, u32 p3, u32 p4, u32 p5, u32 p6, u32 *status, u32 *r1, u32 *r2, u32 *r3, u32 
*r4);
int  (*adapter_check_health)(struct aac_dev *dev);
int  (*adapter_restart)(struct aac_dev *dev, int bled);
+   void (*adapter_start)(struct aac_dev *dev);
/* Transport operations */
int  (*adapter_ioremap)(struct aac_dev * dev, u32 size);
irq_handler_t adapter_intr;
@@ -1247,6 +1248,9 @@ struct aac_dev
 #define aac_adapter_restart(dev,bled) \
(dev)->a_ops.adapter_restart(dev,bled)
 
+#define aac_adapter_start(dev) \
+   ((dev)->a_ops.adapter_start(dev))
+
 #define aac_adapter_ioremap(dev, size) \
(dev)->a_ops.adapter_ioremap(dev, size)
 
@@ -2127,6 +2131,7 @@ int aac_sa_init(struct aac_dev *dev);  int 
aac_src_init(struct aac_dev *dev);  int aac_srcv_init(struct aac_dev *dev);  
int aac_queue_get(struct aac_dev * dev, u32 * index, u32 qid, struct hw_fib * 
hw_fib, int wait, struct fib * fibptr, unsigned long *nonotify);
+void aac_define_int_mode(struct aac_dev *dev);
 unsigned int aac_response_normal(struct aac_queue * q);  unsigned int 
aac_command_normal(struct aac_queue * q);  unsigned int aac_intr_normal(struct 
aac_dev *dev, u32 Index, diff --git a/drivers/scsi/aacraid/comminit.c 
b/drivers/scsi/aacraid/comminit.c index 45db84a..e0a76d5 100644
--- a/drivers/scsi/aacraid/comminit.c
+++ b/drivers/scsi/aacraid/comminit.c
@@ -43,8 +43,6 @@
 
 #include "aacraid.h"
 
-static void aac_define_int_mode(struct aac_dev *dev);
-
 struct aac_common aac_config = {
.irq_mod = 1
 };
@@ -338,6 +336,82 @@ static int aac_comm_init(struct aac_dev * dev)
return 0;
 }
 
+void aac_define_int_mode(struct aac_dev *dev) {
+   int i, msi_count;
+
+   msi_count = i = 0;
+   /* max. vectors from GET_COMM_PREFERRED_SETTINGS */
+   if (dev->max_msix == 0 ||
+   dev->pdev->device == PMC_DEVICE_S6 ||
+   dev->sync_mode) {
+   dev->max_msix = 1;
+   dev->vector_cap =
+   dev->scsi_host_ptr->can_queue +
+   AAC_NUM_MGT_FIB;
+   return;
+   }
+
+   /* Don't bother allocating more MSI-X vectors than cpus */
+   msi_count = min(dev->max_msix,
+   (unsigned int)num_online_cpus());
+
+   dev->max_msix = msi_count;
+
+   if (msi_count > AAC_MAX_MSIX)
+   msi_count = AAC_MAX_MSIX;
+
+   for (i = 0; i < msi_count; i++)
+   dev->msixentry[i].entry = i;
+
+   if (msi_count > 1 &&
+   pci_find_capability(dev->pdev, PCI_CAP_ID_MSIX)) {
+   i = pci_enable_msix_exact(dev->pdev,
+   dev->msixentry,
+   msi_count);
+/* Check how many MSIX vectors are allocated */
+   if (i >= 0) {
+   dev->msi_enabled = 1;
+   if (i) {
+   msi_count = i;
+   if (pci_enable_msix_exact(dev->pdev,
+   dev->msixentry,
+   msi_count)) {
+   dev->msi_enabled = 0;
+   printk(KERN_ERR "%s%d: MSIX not 
supported!! Will try MSI 0x%x.\n",
+   dev->name, dev->id, i);
+   }
+   }
+   } else {
+   dev->msi_enabled = 0;
+   printk(KERN_ERR "%s%d: MSIX not supported!! Will try 
MSI 0x%x.\n",
+   dev->name,

RE: [Patch V2 1/9] [SCSI] aacraid: Fix for logical device name and UID not exposed to the OS

[Mahesh Rajashekhara]

Reviewed-by: Mahesh Rajashekhara 


-Original Message-
From: Rajinikanth Pandurangan 
Sent: Thursday, June 11, 2015 7:12 AM
To: jbottom...@parallels.com; linux-scsi@vger.kernel.org
Cc: aacr...@pmc-sierra.com; Harry Yang; Mahesh Rajashekhara; Rich Bono; Achim 
Leubner; Murthy Bhat; Rajinikanth Pandurangan
Subject: [Patch V2 1/9] [SCSI] aacraid: Fix for logical device name and UID not 
exposed to the OS

From: Rajinikanth Pandurangan 

Description:
Driver sends the right size of the response buffer.

Signed-off-by: Rajinikanth Pandurangan 
---
 drivers/scsi/aacraid/aachba.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/aacraid/aachba.c b/drivers/scsi/aacraid/aachba.c 
index 9b3dd6e..fe59b00 100644
--- a/drivers/scsi/aacraid/aachba.c
+++ b/drivers/scsi/aacraid/aachba.c
@@ -570,7 +570,7 @@ static int aac_get_container_name(struct scsi_cmnd * 
scsicmd)
 
status = aac_fib_send(ContainerCommand,
  cmd_fibcontext,
- sizeof (struct aac_get_name),
+ sizeof(struct aac_get_name_resp),
  FsaNormal,
  0, 1,
  (fib_callback)get_container_name_callback,
@@ -1052,7 +1052,7 @@ static int aac_get_container_serial(struct scsi_cmnd * 
scsicmd)
 
status = aac_fib_send(ContainerCommand,
  cmd_fibcontext,
- sizeof (struct aac_get_serial),
+ sizeof(struct aac_get_serial_resp),
  FsaNormal,
  0, 1,
  (fib_callback) get_container_serial_callback,
--
1.9.3

--
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 V2 4/9] [SCSI] aacraid: Enable 64-bit write to controller register

[Mahesh Rajashekhara]

Reviewed-by: Mahesh Rajashekhara 


-Original Message-
From: Rajinikanth Pandurangan 
Sent: Thursday, June 11, 2015 7:12 AM
To: jbottom...@parallels.com; linux-scsi@vger.kernel.org
Cc: aacr...@pmc-sierra.com; Harry Yang; Mahesh Rajashekhara; Rich Bono; Achim 
Leubner; Murthy Bhat; Rajinikanth Pandurangan
Subject: [Patch V2 4/9] [SCSI] aacraid: Enable 64-bit write to controller 
register

From: Rajinikanth Pandurangan 

Description:
If writeq() not supported, then do atomic two 32bit write

Signed-off-by: Rajinikanth Pandurangan 
---
 drivers/scsi/aacraid/aacraid.h  |  9 +  
drivers/scsi/aacraid/comminit.c |  1 +
 drivers/scsi/aacraid/src.c  | 12 ++--
 3 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h 
index 62b0999..e54f597 100644
--- a/drivers/scsi/aacraid/aacraid.h
+++ b/drivers/scsi/aacraid/aacraid.h
@@ -844,6 +844,10 @@ struct src_registers {
&((AEP)->regs.src.bar0->CSR))
 #define src_writel(AEP, CSR, value)writel(value, \
&((AEP)->regs.src.bar0->CSR))
+#if defined(writeq)
+#definesrc_writeq(AEP, CSR, value) writeq(value, \
+   &((AEP)->regs.src.bar0->CSR))
+#endif
 
 #define SRC_ODR_SHIFT  12
 #define SRC_IDR_SHIFT  9
@@ -1163,6 +1167,11 @@ struct aac_dev
struct fsa_dev_info *fsa_dev;
struct task_struct  *thread;
int cardtype;
+   /*
+*This lock will protect the two 32-bit
+*writes to the Inbound Queue
+*/
+   spinlock_t  iq_lock;
 
/*
 *  The following is the device specific extension.
diff --git a/drivers/scsi/aacraid/comminit.c b/drivers/scsi/aacraid/comminit.c 
index e0a76d5..e4ff47e 100644
--- a/drivers/scsi/aacraid/comminit.c
+++ b/drivers/scsi/aacraid/comminit.c
@@ -424,6 +424,7 @@ struct aac_dev *aac_init_adapter(struct aac_dev *dev)
dev->management_fib_count = 0;
spin_lock_init(&dev->manage_lock);
spin_lock_init(&dev->sync_lock);
+   spin_lock_init(&dev->iq_lock);
dev->max_fib_size = sizeof(struct hw_fib);
dev->sg_tablesize = host->sg_tablesize = (dev->max_fib_size
- sizeof(struct aac_fibhdr)
diff --git a/drivers/scsi/aacraid/src.c b/drivers/scsi/aacraid/src.c index 
eb07b3d..1409a0b 100644
--- a/drivers/scsi/aacraid/src.c
+++ b/drivers/scsi/aacraid/src.c
@@ -447,6 +447,10 @@ static int aac_src_deliver_message(struct fib *fib)
u32 fibsize;
dma_addr_t address;
struct aac_fib_xporthdr *pFibX;
+#if !defined(writeq)
+   unsigned long flags;
+#endif
+
u16 hdr_size = le16_to_cpu(fib->hw_fib_va->header.Size);
 
atomic_inc(&q->numpending);
@@ -511,10 +515,14 @@ static int aac_src_deliver_message(struct fib *fib)
return -EINVAL;
address |= fibsize;
}
-
+#if defined(writeq)
+   src_writeq(dev, MUnit.IQ_L, (u64)address); #else
+   spin_lock_irqsave(&fib->dev->iq_lock, flags);
src_writel(dev, MUnit.IQ_H, upper_32_bits(address) & 0x);
src_writel(dev, MUnit.IQ_L, address & 0x);
-
+   spin_unlock_irqrestore(&fib->dev->iq_lock, flags); #endif
return 0;
 }
 
--
1.9.3

--
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][SCSI] hptiop: Support HighPoint RR36xx HBAs and Support SAS tape and SAS media changer

[linux]

Support HighPoint RR36xx HBAs which are based on Marvell Frey.
Support SAS tape and SAS media changer.

Signed-off-by: HighPoint Linux Team 

 drivers/scsi/hptiop.c |  104
+++
 drivers/scsi/hptiop.h |6 +--
 2 files changed, 69 insertions(+), 41 deletions(-)

diff -urN linux.git/drivers/scsi/hptiop.c linux/drivers/scsi/hptiop.c
--- linux.git/drivers/scsi/hptiop.c 2015-06-08 01:48:38.09375 -0400
+++ linux/drivers/scsi/hptiop.c 2015-06-08 02:45:33.234375000 -0400
@@ -1,6 +1,6 @@
 /*
  * HighPoint RR3xxx/4xxx controller driver for Linux
- * Copyright (C) 2006-2012 HighPoint Technologies, Inc. All Rights
Reserved.
+ * Copyright (C) 2006-2015 HighPoint Technologies, Inc. All Rights
Reserved.
  *
  * 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
@@ -42,7 +42,7 @@
 
 static char driver_name[] = "hptiop";
 static const char driver_name_long[] = "RocketRAID 3xxx/4xxx Controller
driver";
-static const char driver_ver[] = "v1.8";
+static const char driver_ver[] = "v1.10.0";
 
 static int iop_send_sync_msg(struct hptiop_hba *hba, u32 msg, u32
millisec);
 static void hptiop_finish_scsi_req(struct hptiop_hba *hba, u32 tag,
@@ -764,9 +764,7 @@
scsi_set_resid(scp,
scsi_bufflen(scp) -
le32_to_cpu(req->dataxfer_length));
scp->result = SAM_STAT_CHECK_CONDITION;
-   memcpy(scp->sense_buffer, &req->sg_list,
-   min_t(size_t, SCSI_SENSE_BUFFERSIZE,
-   le32_to_cpu(req->dataxfer_length)));
+   memcpy(scp->sense_buffer, &req->sg_list,
SCSI_SENSE_BUFFERSIZE);
goto skip_resid;
break;
 
@@ -1036,9 +1034,10 @@
_req->index, _req->req_virt);
 
scp->result = 0;
-
-   if (scp->device->channel || scp->device->lun ||
-   scp->device->id > hba->max_devices) {
+   
+   if (scp->device->channel ||
+   (scp->device->id > hba->max_devices) ||
+   ((scp->device->id == (hba->max_devices-1)) &&
scp->device->lun)) {
scp->result = DID_BAD_TARGET << 16;
free_req(hba, _req);
goto cmd_done;
@@ -1168,6 +1167,14 @@
NULL
 };
 
+static int hptiop_slave_config(struct scsi_device *sdev)
+{
+   if (sdev->type == TYPE_TAPE)
+   blk_queue_max_hw_sectors(sdev->request_queue, 8192);
+
+   return 0;
+}
+
 static struct scsi_host_template driver_template = {
.module = THIS_MODULE,
.name   = driver_name,
@@ -1179,6 +1186,7 @@
.use_clustering = ENABLE_CLUSTERING,
.proc_name  = driver_name,
.shost_attrs= hptiop_attrs,
+   .slave_configure= hptiop_slave_config,
.this_id= -1,
.change_queue_depth = hptiop_adjust_disk_queue_depth,
 };
@@ -1323,7 +1331,8 @@
}
 
hba = (struct hptiop_hba *)host->hostdata;
-
+   memset(hba, 0, sizeof(struct hptiop_hba));
+   
hba->ops = iop_ops;
hba->pcidev = pcidev;
hba->host = host;
@@ -1336,7 +1345,7 @@
init_waitqueue_head(&hba->reset_wq);
init_waitqueue_head(&hba->ioctl_wq);
 
-   host->max_lun = 1;
+   host->max_lun = 128;
host->max_channel = 0;
host->io_port = 0;
host->n_io_port = 0;
@@ -1428,34 +1437,33 @@
dprintk("req_size=%d, max_requests=%d\n", req_size,
hba->max_requests);
 
hba->req_size = req_size;
-   start_virt = dma_alloc_coherent(&pcidev->dev,
-   hba->req_size*hba->max_requests + 0x20,
-   &start_phy, GFP_KERNEL);
-
-   if (!start_virt) {
-   printk(KERN_ERR "scsi%d: fail to alloc request mem\n",
-   hba->host->host_no);
-   goto free_request_irq;
-   }
-
-   hba->dma_coherent = start_virt;
-   hba->dma_coherent_handle = start_phy;
-
-   if ((start_phy & 0x1f) != 0) {
-   offset = ((start_phy + 0x1f) & ~0x1f) - start_phy;
-   start_phy += offset;
-   start_virt += offset;
-   }
-
hba->req_list = NULL;
+   
for (i = 0; i < hba->max_requests; i++) {
+   start_virt = dma_alloc_coherent(&pcidev->dev,
+   hba->req_size + 0x20,
+   &start_phy, GFP_KERNEL);
+
+   if (!start_virt) {
+   printk(KERN_ERR "scsi%d: fail to alloc request
mem\n",
+   hba->host->host_no);
+   goto free_request_mem;
+   }
+
+   hba->dma_coherent[i] = start_v

Re: configurable discard parameters

[Martin K. Petersen]

> "Tom" == Tom Yan  writes:

Tom> total untrimmed sectors: (310635 - 1 - 2410) / 32 = 9632 total
Tom> ranges: (43 GiB * (1024 ^ 3) / 512) / 65535 ~= 1376 average
Tom> untrimmed sectors per range: (9632 / 1376) = 7 = (65535 % 8)

Every type of drive has its own internal restrictions. Unless the drive
guarantees zero after TRIM it is perfectly normal for heads, tails or
random pieces in-between to be left untouched.

I am surprised that the common case of contiguous range entries was not
handled properly by your drive. Most of them deal with that just fine
(regardless of their internal granularity and alignment constraints). It
is normally only the partial block tracking between command invocations
that causes grief.

In any case. Unless discard_zeroes_data is 1 (and that requires
guarantees above and beyond what can be discovered via the ATA Command
Set) all bets are off wrt. dependable behavior.

The code below is an untested proof of concept to see what it would take
to alleviate your particular issue. I am, however, not at all convinced
that introducing such a change is worth the risk. I know of at least a
couple of devices that would blow up as a result of this patch...

-- 
Martin K. Petersen  Oracle Linux Engineering

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 3131adcc1f87..9c270a303f0b 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -2238,8 +2238,24 @@ static unsigned int ata_scsiop_inq_b0(struct 
ata_scsi_args *args, u8 *rbuf)
 * with the unmap bit set.
 */
if (ata_id_has_trim(args->id)) {
-   put_unaligned_be64(65535 * 512 / 8, &rbuf[36]);
-   put_unaligned_be32(1, &rbuf[28]);
+   /*
+* If the drive supports reliable zero after trim we set
+* the granularity to 1 and the blocks per range to
+* 0x. Otherwise we set set the granularity to 8 and
+* restrict the blocks per range to 0xfff8. This is done
+* to accommodate older drives which prefer 4K-alignment.
+*/
+
+   if (ata_id_has_zero_after_trim(args->id) &&
+   args->dev->horkage & ATA_HORKAGE_ZERO_AFTER_TRIM) {
+   put_unaligned_be64(ATA_MAX_TRIM_RANGE * 512 / 8,
+  &rbuf[36]);
+   put_unaligned_be32(1, &rbuf[28]);
+   } else {
+   put_unaligned_be64(ATA_ALIGNED_TRIM_RANGE * 512 / 8,
+  &rbuf[36]);
+   put_unaligned_be32(8, &rbuf[28]);
+   }
}
 
return 0;
@@ -3168,7 +3184,14 @@ static unsigned int ata_scsi_write_same_xlat(struct 
ata_queued_cmd *qc)
goto invalid_fld;
 
buf = page_address(sg_page(scsi_sglist(scmd)));
-   size = ata_set_lba_range_entries(buf, 512, block, n_block);
+
+   if (ata_id_has_zero_after_trim(dev->id) &&
+   dev->horkage & ATA_HORKAGE_ZERO_AFTER_TRIM)
+   size = ata_set_lba_range_entries(buf, 512, block, n_block,
+ATA_MAX_TRIM_RANGE);
+   else
+   size = ata_set_lba_range_entries(buf, 512, block, n_block,
+ATA_ALIGNED_TRIM_RANGE);
 
if (ata_ncq_enabled(dev) && ata_fpdma_dsm_supported(dev)) {
/* Newer devices support queued TRIM commands */
diff --git a/include/linux/ata.h b/include/linux/ata.h
index fed36418dd1c..8a17fa22cdbe 100644
--- a/include/linux/ata.h
+++ b/include/linux/ata.h
@@ -47,6 +47,8 @@ enum {
ATA_MAX_SECTORS = 256,
ATA_MAX_SECTORS_LBA48   = 65535,/* TODO: 65536? */
ATA_MAX_SECTORS_TAPE= 65535,
+   ATA_MAX_TRIM_RANGE  = 0x,
+   ATA_ALIGNED_TRIM_RANGE  = 0xfff8,
 
ATA_ID_WORDS= 256,
ATA_ID_CONFIG   = 0,
@@ -1012,19 +1014,20 @@ static inline void ata_id_to_hd_driveid(u16 *id)
  * TO NV CACHE PINNED SET.
  */
 static inline unsigned ata_set_lba_range_entries(void *_buffer,
-   unsigned buf_size, u64 sector, unsigned long count)
+   unsigned buf_size, u64 sector, unsigned long count,
+   u16 bpe)
 {
__le64 *buffer = _buffer;
unsigned i = 0, used_bytes;
 
while (i < buf_size / 8 ) { /* 6-byte LBA + 2-byte range per entry */
u64 entry = sector |
-   ((u64)(count > 0x ? 0x : count) << 48);
+   ((u64)(count > bpe ? bpe : count) << 48);
buffer[i++] = __cpu_to_le64(entry);
-   if (count <= 0x)
+   if (count <= bpe)
break;
-   count -= 0x;
-   sector += 0x;
+   count -= bpe;
+   sector += bpe;
}
 
used_bytes = ALIGN(i * 8, 512);
--
To unsubscribe 

Re: [PATCH] st: convert to using driver attr groups for sysfs

[Greg KH]

On Tue, Jun 23, 2015 at 08:11:00AM +, Seymour, Shane M wrote:
> This patch changes the st driver to use attribute groups so
> driver sysfs files are created automatically. See the
> following for reference:
> 
> http://kroah.com/log/blog/2013/06/26/how-to-create-a-sysfs-file-correctly/
> 
> Signed-off-by: Shane Seymour 

Very nice, thanks for doing this.

Acked-by: Greg Kroah-Hartman 

> ---
> --- a/drivers/scsi/st.c   2015-06-22 14:20:40.829612661 -0500
> +++ b/drivers/scsi/st.c   2015-06-22 15:49:49.357248393 -0500
> @@ -85,6 +85,7 @@ static int debug_flag;
>  
>  static struct class st_sysfs_class;
>  static const struct attribute_group *st_dev_groups[];
> +static const struct attribute_group *st_drv_groups[];
>  
>  MODULE_AUTHOR("Kai Makisara");
>  MODULE_DESCRIPTION("SCSI tape (st) driver");
> @@ -198,15 +199,13 @@ static int sgl_unmap_user_pages(struct s
>  static int st_probe(struct device *);
>  static int st_remove(struct device *);
>  
> -static int do_create_sysfs_files(void);
> -static void do_remove_sysfs_files(void);
> -
>  static struct scsi_driver st_template = {
>   .gendrv = {
>   .name   = "st",
>   .owner  = THIS_MODULE,
>   .probe  = st_probe,
>   .remove = st_remove,
> + .groups = st_drv_groups,
>   },
>  };
>  
> @@ -4404,14 +4403,8 @@ static int __init init_st(void)
>   if (err)
>   goto err_chrdev;
>  
> - err = do_create_sysfs_files();
> - if (err)
> - goto err_scsidrv;
> -
>   return 0;
>  
> -err_scsidrv:
> - scsi_unregister_driver(&st_template.gendrv);
>  err_chrdev:
>   unregister_chrdev_region(MKDEV(SCSI_TAPE_MAJOR, 0),
>ST_MAX_TAPE_ENTRIES);
> @@ -4422,7 +4415,6 @@ err_class:
>  
>  static void __exit exit_st(void)
>  {
> - do_remove_sysfs_files();
>   scsi_unregister_driver(&st_template.gendrv);
>   unregister_chrdev_region(MKDEV(SCSI_TAPE_MAJOR, 0),
>ST_MAX_TAPE_ENTRIES);
> @@ -4459,44 +4451,14 @@ static ssize_t st_version_show(struct de
>  }
>  static DRIVER_ATTR(version, S_IRUGO, st_version_show, NULL);

For a future patch, you might want to convert these type of declarations
to use DRIVER_ATTR_RO() and friends.

thanks,

greg k-h
--
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: configurable discard parameters

[Tom Yan]

First of all let me add another "statistic" about the issue:

[tom@localhost ~]$ sudo shred -n 1 /dev/sda3
[tom@localhost ~]$ sudo blkdiscard /dev/sda3
[tom@localhost ~]$ sudo hexdump /dev/sda3 | wc -l
310635
[tom@localhost ~]$ sudo hexdump /dev/sda3 | pcregrep -M ' 
     \n\*' | wc -l
2410

total untrimmed sectors: (310635 - 1 - 2410) / 32 = 9632
total ranges: (43 GiB * (1024 ^ 3) / 512) / 65535 ~= 1376
average untrimmed sectors per range: (9632 / 1376) = 7 = (65535 % 8)

Also, FWIW:

[tom@localhost ~]$ sudo shred -n 1 /dev/sda3
[tom@localhost ~]$ let step=(65535-65535%8)*512
[tom@localhost ~]$ echo $step
33550336
[tom@localhost ~]$ sudo blkdiscard -p "$step" /dev/sda3
[tom@localhost ~]$ sudo hexdump /dev/sda3
000        
*
ac000

On 24 June 2015 at 02:26, Martin K. Petersen  wrote:
>
> What happens if you discard sectors 0-6 and then sector 7?
>

[tom@localhost ~]$ sudo shred -n 1 /dev/sda3
[tom@localhost ~]$ sudo blkdiscard -l 3584 /dev/sda3
[tom@localhost ~]$ sudo blkdiscard -o 3584 -l 512 /dev/sda3
[tom@localhost ~]$ sudo hexdump -n 4096 /dev/sda3
000 f06d 8365 5e1b 616c 7362 4d61 2182 02fb
...
ff0 54ef 9579 51bc 9042 115a 375e c28f 4dcc
0001000

>
> This is on the Intel 530? What does the drive report in
> /sys/block/sdN/queue/discard_zeroes_data?
>

Yes. It reports 0. In `hdparm -I`:
   *Data Set Management TRIM supported (limit 1 block)
   *Deterministic read data after TRIM

You may also want to check out and compare the two attached test case files.


sandisk_test
Description: Binary data


intel_test
Description: Binary data

[PATCH] mpt3sas: Abort initialization if no memory I/O resources detected

[Timothy Pearson]

The mpt3sas driver crashes if the BIOS does not set up at least one
memory I/O resource.  This failure can happen if the device is too
slow to respond during POST and is missed by the BIOS, but Linux
then detects the device later in the boot process.

Signed-off-by: Timothy Pearson 
---
 drivers/scsi/mpt3sas/mpt3sas_base.c |7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c 
b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 14a781b..4460d48 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -1865,6 +1865,13 @@ mpt3sas_base_map_resources(struct MPT3SAS_ADAPTER *ioc)
}
}
 
+   if (ioc->chip == NULL) {
+   printk(MPT3SAS_FMT "unable to map "
+   "adapter memory (resource not found)!\n", ioc->name);
+   r = -EINVAL;
+   goto out_fail;
+   }
+
_base_mask_interrupts(ioc);
 
r = _base_get_ioc_facts(ioc, CAN_SLEEP);
-- 
1.7.9.5

-- 
Timothy Pearson
Raptor Engineering
+1 (415) 727-8645
http://www.raptorengineeringinc.com 
--
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 v2] mpt2sas: Abort initialization if no memory I/O resources detected

[Timothy Pearson]

The mpt2sas driver crashes if the BIOS does not set up at least one
memory I/O resource.  This failure can happen if the device is too
slow to respond during POST and is missed by the BIOS, but Linux
then detects the device later in the boot process.

Signed-off-by: Timothy Pearson 
Tested-by: Timothy Pearson 
---
 drivers/scsi/mpt2sas/mpt2sas_base.c |7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c 
b/drivers/scsi/mpt2sas/mpt2sas_base.c
index 11248de..b70fa5a 100644
--- a/drivers/scsi/mpt2sas/mpt2sas_base.c
+++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
@@ -1582,6 +1582,13 @@ mpt2sas_base_map_resources(struct MPT2SAS_ADAPTER *ioc)
}
}
 
+   if (ioc->chip == NULL) {
+   printk(MPT2SAS_ERR_FMT "unable to map "
+   "adapter memory (resource not found)!\n", ioc->name);
+   r = -EINVAL;
+   goto out_fail;
+   }
+
_base_mask_interrupts(ioc);
 
r = _base_get_ioc_facts(ioc, CAN_SLEEP);
-- 
1.7.9.5

-- 
Timothy Pearson
Raptor Engineering
+1 (415) 727-8645
http://www.raptorengineeringinc.com 
--
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: configurable discard parameters

[Martin K. Petersen]

> "Tom" == Tom Yan  writes:

Tom> So when libata issue ATA commands with ranges of 65535 sectors,
Tom> only 65535-(65535%8) = 65528 sectors are discarded,

That's unfortunate but TRIM is advisory so the drive is free to ignore
all or parts of the request.

What happens if you discard sectors 0-6 and then sector 7?

Tom> I can workaround this by specifying --step in blkdiscard, but I
Tom> think the kernel should have a param configurable for general.

This is on the Intel 530? What does the drive report in
/sys/block/sdN/queue/discard_zeroes_data?

-- 
Martin K. Petersen  Oracle Linux Engineering
--
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 RESEND] Documentation/scsi: Documentation about scsi_cmnd lifecycle

[Rajat Jain]

Hello James,

I haven't heard any feedback on this patch, so I was wondering if this
documentation patch is something you're considering to review?

Many thanks in advance,

Rajat

On Tue, Jun 9, 2015 at 10:43 AM, Rajat Jain  wrote:
> Add documentation to describe the various scenarios that the scsi_cmnd
> may go through in its life time in the mid level driver - aborts,
> failures, retries, error handling etc. The documentation has lots of
> details including examples.
>
> Signed-off-by: Rajat Jain 
> ---
> Hello James / linux-scsi,
>
> Resending the patch since it couldn't get any attention in a month. I'd
> appreciate if you could please review it and provide your valuable feedback.
>
> Thanks,
>
> Rajat
>
>
>  Documentation/scsi/life_of_a_scsi_cmnd.txt | 667 
> +
>  1 file changed, 667 insertions(+)
>  create mode 100644 Documentation/scsi/life_of_a_scsi_cmnd.txt
>
> diff --git a/Documentation/scsi/life_of_a_scsi_cmnd.txt 
> b/Documentation/scsi/life_of_a_scsi_cmnd.txt
> new file mode 100644
> index 000..b09b2a2
> --- /dev/null
> +++ b/Documentation/scsi/life_of_a_scsi_cmnd.txt
> @@ -0,0 +1,667 @@
> + ==
> + Life of a SCSI Command (scsi_cmnd)
> + ==
> +
> +  Rajat Jain  on 12-May-2015
> +
> +(This document roughly matches the Linux kernel 4.0)
> +
> +This documents describes the various phases of a SCSI command (struct 
> scsi_cmnd)
> +lifecycle, as it flows though different parts of the SCSI mid level driver. 
> It
> +describes under what conditions and how a scsi_cmnd may be aborted, or 
> retried,
> +or scheduled for error handling, and how is it recovered, and in general how 
> a
> +block request is handled by the SCSI mid level driver. It goes into detail 
> about
> +what functions get called and the purpose for each one of them etc.
> +
> +To help explain with an example, it takes example of a scsi_cmnd that goes
> +through it all - timeout, abort, error handling, retry (also results in
> +CHECK_CONDITION and gets sense info). The last section traces the path taken 
> by
> +this example scsi_cmnd in its lifetime.
> +
> +TABLE OF CONTENTS
> +
> +[1] Lifecycle of a scsi_cmnd
> +[2] How does a scsi_cmnd get queued to the LLD for processing?
> +[3] How does a scsi_cmnd complete?
> +[3.1] Command completing via scsi_softirq_done()
> +[3.2] Command completing via scsi_times_out()$
> +[4] SCSI Error Handling
> +[4.1] How did we Get here?
> +[4.2] When does Error Handling actually run?
> +[4.3] SCSI Error Handler thread
> +[5] SCSI Commands can be "hijacked"
> +[6] SCSI Command Aborts
> +[6.1] When would mid level try to abort a command?
> +[6.2] How SCSI command abort works?
> +[6.3] Aborts can fail too
> +[7] SCSI command Retries
> +[7.1] When would mid level retry a command?
> +[7.2] Eligibility criteria for Retry
> +[8] Example: Following a scsi_cmnd (that results in CHECK_CONDITION)
> +[8.1] High level view of path taken by example scsi_cmnd
> +[8.2] Actual Path taken
> +[9] References
> +
> +1. Lifecycle of a scsi_cmnd
> +   
> +   SCSI Mid level interfaces with the block layer just like any other block
> +   driver. For each block device that SCSI ML adds to the system, it 
> indicates
> +   a bunch of functions to serve the corresponding request queue.
> +
> +   The following functions are relevant to the scsi_cmnd in its lifetime. 
> Note
> +   that depending on the situations, it may not go thourgh some of these
> +   stages, or may have to go through some stages multiple times.
> +
> +   scsi_prep_fn()
> + is called by the blocklayer to prepare the request. This
> + function actually allocates a new scsi_cmnd for the request (from
> + scsi_host->cmd_pool) and sets it up. This is where a scsi_smnd is 
> "born".
> + Note, a new scsi_cmnd is allocated only if the blk req did not already 
> have
> + one associated with it (req->special != NULL). A req may already have a
> + scsi_cmnd if the req was tried by SCSI earlier, and it resulted in a
> + decision to retry later (and hence req was put back on the queue).
> +
> +   scsi_request_fn()
> + is the actual function to serve the request queue. It basically checks
> + whether the host is ready for new commands, and if so, it submits it to 
> the
> + LLD:
> + scsi_request_fn()
> +   ->scsi_dispatch_cmd()
> +   ->hostt->queue_command()
> + In case a scsi_cmnd could not be queued to LLD for some reason, the req
> + is put back on the original request queue (for retry later).
> +
> +   scsi_softirq_done()
> + is the handler that gets called once the LLD indicates command 
> completed.
> + scsi_done()
> +   ->blk_complete_request()
> +   ->causes softirq
> +   ->blk_done_softirq()
> +   ->scsi_softirq_done()
> + The 

Re: [PATCH] mpt2sas: Abort initialization if no memory I/O resources, detected

[James Bottomley]

On Tue, 2015-06-23 at 12:59 -0500, Timothy Pearson wrote:
> On 06/23/2015 12:56 PM, James Bottomley wrote:
> > On Tue, 2015-06-23 at 12:47 -0500, Timothy Pearson wrote:
> >> On 06/23/2015 12:45 PM, James Bottomley wrote:
> >>> On Tue, 2015-06-23 at 12:33 -0500, Timothy Pearson wrote:
>  On 06/23/2015 08:35 AM, James Bottomley wrote:
> > On Tue, 2015-06-23 at 18:06 +0530, Sreekanth Reddy wrote:
> >> Hi,
> >>
> >> Upstream contributor should not add copyright to this driver code.
> >
> > I can't agree with that as a general rule: it depends on the
> > significance of the contribution.  The somewhat ill defined standard for
> > this is the contribution must be "an original work of authorship".
> >
> > I can agree that a trivial bug fix like this is not an original work of
> > authorship, so in this case adding copyright isn't appropriate.
> >
> > James
> >
> >> Regards,
> >> Sreekanth
> >>
> >> On Tue, Jun 23, 2015 at 9:24 AM, Joe 
> >> Lawrencewrote:
> >>>
> >>>
> >>> On 06/21/2015 02:46 PM, Timothy Pearson wrote:
>  On 06/16/2015 01:49 PM, Timothy Pearson wrote:
> > On 06/16/2015 12:42 PM, Joe Lawrence wrote:
> >> On 06/16/2015 12:28 PM, Timothy Pearson wrote:
> >>> On 06/12/2015 05:05 PM, Timothy Pearson wrote:
>  The mpt2sas driver crashes if the BIOS does not set up at least 
>  one
>  memory I/O resource. This failure can happen if the device is too
>  slow to respond during POST and is missed by the BIOS, but Linux
>  then detects the device later in the boot process.
> 
>  This patch aborts initialization and prints a warning if no 
>  memory I/O
>  resources are found.
> 
>  Signed-off-by: Timothy Pearson
>  Tested-by: Timothy Pearson
>  ---
>  drivers/scsi/mpt2sas/mpt2sas_base.c | 9 +
>  1 file changed, 9 insertions(+)
> 
>  diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c
>  b/drivers/scsi/mpt2sas/mpt2sas_base.c
>  index 11248de..15c9504 100644
>  --- a/drivers/scsi/mpt2sas/mpt2sas_base.c
>  +++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
>  @@ -6,6 +6,8 @@
>  * Copyright (C) 2007-2014 LSI Corporation
>  * Copyright (C) 20013-2014 Avago Technologies
>  * (mailto: mpt-fusionlinux@avagotech.com)
>  + * Copyright (C) 2015 Raptor Engineering
>  + * (mailto: supp...@araptorengineeringinc.com)
>  *
>  * This program is free software; you can redistribute it and/or
>  * modify it under the terms of the GNU General Public License
>  @@ -1582,6 +1584,13 @@ mpt2sas_base_map_resources(struct
>  MPT2SAS_ADAPTER
>  *ioc)
>  }
>  }
> 
>  + if (ioc->chip == NULL) {
>  + printk(MPT2SAS_ERR_FMT "unable to map "
>  + "adapter memory (resource not found)!\n", ioc->name);
>  + r = -EINVAL;
>  + goto out_fail;
>  + }
>  +
>  _base_mask_interrupts(ioc);
> 
>  r = _base_get_ioc_facts(ioc, CAN_SLEEP);
> >>>
> >>> Just following up on this patch as I have not yet received any
> >>> response.
> >>>
> >>> Thanks!
> >>
> >> Hi Tim -- just curious, why was the similar check on ioc->chip 
> >> just a
> >> few lines above the one added by the patch insufficient?
> >>
> >> That loop block sets memap_sz when it finds an IORESOURCE_MEM so 
> >> that it
> >> only sets ioc->chip once. I wonder if the fix might be simpler if 
> >> the
> >> existing ioc->chip check relocated entirely to where you put it 
> >> (maybe
> >> also pulling the entire error text onto one line for easier 
> >> grepping).
> >>
> >> Regards,
> >>
> >> -- Joe
> >
> > If there are no IORESOURCE_MEM resources allocated by the BIOS 
> > (i.e. if
> > the BIOS does not run resource allocation on the mpt2sas device) 
> > then
> > the check you are referring to is not executed, and the driver 
> > attempts
> > to perform operations on a null ioc->chip pointer.
> >
> > I can relocate the check if desired.
> >
> 
>  On looking more closely at the code I think having the two separate
>  checks is useful for debugging.  The first error message is 
>  triggered if
>  the resource exists but Linux cannot map it for some reason, and the
>  second error message is triggered if the resource does not 

Re: [PATCH] mpt2sas: Abort initialization if no memory I/O resources, detected

[Timothy Pearson]

On 06/23/2015 12:56 PM, James Bottomley wrote:

On Tue, 2015-06-23 at 12:47 -0500, Timothy Pearson wrote:

On 06/23/2015 12:45 PM, James Bottomley wrote:

On Tue, 2015-06-23 at 12:33 -0500, Timothy Pearson wrote:

On 06/23/2015 08:35 AM, James Bottomley wrote:

On Tue, 2015-06-23 at 18:06 +0530, Sreekanth Reddy wrote:

Hi,

Upstream contributor should not add copyright to this driver code.


I can't agree with that as a general rule: it depends on the
significance of the contribution.  The somewhat ill defined standard for
this is the contribution must be "an original work of authorship".

I can agree that a trivial bug fix like this is not an original work of
authorship, so in this case adding copyright isn't appropriate.

James


Regards,
Sreekanth

On Tue, Jun 23, 2015 at 9:24 AM, Joe Lawrence
wrote:



On 06/21/2015 02:46 PM, Timothy Pearson wrote:

On 06/16/2015 01:49 PM, Timothy Pearson wrote:

On 06/16/2015 12:42 PM, Joe Lawrence wrote:

On 06/16/2015 12:28 PM, Timothy Pearson wrote:

On 06/12/2015 05:05 PM, Timothy Pearson wrote:

The mpt2sas driver crashes if the BIOS does not set up at least one
memory I/O resource. This failure can happen if the device is too
slow to respond during POST and is missed by the BIOS, but Linux
then detects the device later in the boot process.

This patch aborts initialization and prints a warning if no memory I/O
resources are found.

Signed-off-by: Timothy Pearson
Tested-by: Timothy Pearson
---
drivers/scsi/mpt2sas/mpt2sas_base.c | 9 +
1 file changed, 9 insertions(+)

diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c
b/drivers/scsi/mpt2sas/mpt2sas_base.c
index 11248de..15c9504 100644
--- a/drivers/scsi/mpt2sas/mpt2sas_base.c
+++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
@@ -6,6 +6,8 @@
* Copyright (C) 2007-2014 LSI Corporation
* Copyright (C) 20013-2014 Avago Technologies
* (mailto: mpt-fusionlinux@avagotech.com)
+ * Copyright (C) 2015 Raptor Engineering
+ * (mailto: supp...@araptorengineeringinc.com)
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
@@ -1582,6 +1584,13 @@ mpt2sas_base_map_resources(struct
MPT2SAS_ADAPTER
*ioc)
}
}

+ if (ioc->chip == NULL) {
+ printk(MPT2SAS_ERR_FMT "unable to map "
+ "adapter memory (resource not found)!\n", ioc->name);
+ r = -EINVAL;
+ goto out_fail;
+ }
+
_base_mask_interrupts(ioc);

r = _base_get_ioc_facts(ioc, CAN_SLEEP);


Just following up on this patch as I have not yet received any
response.

Thanks!


Hi Tim -- just curious, why was the similar check on ioc->chip just a
few lines above the one added by the patch insufficient?

That loop block sets memap_sz when it finds an IORESOURCE_MEM so that it
only sets ioc->chip once. I wonder if the fix might be simpler if the
existing ioc->chip check relocated entirely to where you put it (maybe
also pulling the entire error text onto one line for easier grepping).

Regards,

-- Joe


If there are no IORESOURCE_MEM resources allocated by the BIOS (i.e. if
the BIOS does not run resource allocation on the mpt2sas device) then
the check you are referring to is not executed, and the driver attempts
to perform operations on a null ioc->chip pointer.

I can relocate the check if desired.



On looking more closely at the code I think having the two separate
checks is useful for debugging.  The first error message is triggered if
the resource exists but Linux cannot map it for some reason, and the
second error message is triggered if the resource does not exist at all
(buggy BIOS).


Fair enough.  The two error messages are unique, so grepping will lead
to the correct check.

Only other nits would be the addition of a copyright (up to Avago I
guess) and an mpt3sas version (these drivers are 99% the same code).

Acked-by: Joe Lawrence

-- Joe








Thank you for clarifying the guidelines on copyright lines.  I wasn't
sure if this was significant enough of a contribution to justify the
line or not.

Do I need to re-send or can I just state here that the patch can be
merged without the copyright line?


It's easier if you send a clean copy rather than risk me botching
something in the hand edit.


OK, will do.


I don't have any mpt3sas hardware to test with; should I just send in a
patch anyway without a Tested-by line?


Well, if you haven't actually tested it, adding a tested-by line
wouldn't be correct.


Of course not.  My question was, should I send in a patch anyway (with
appropriate commit message fields) even though it has not been tested?


You mean do you need to retest for a non-material change to a patch
still to have a tested-by on it?  Ideally, yes, but as long as there are
no actual code changes, I don't think it's required.

James


I guess I'm being somewhat confusing today, my apologies for that.  I 
understand there is no real need to re-test when removing the copyright 
line for the mpt2sas driver patch.


My question is related to your earlier comment about the mpt3sas driver,

Re: [PATCH] mpt2sas: Abort initialization if no memory I/O resources, detected

[James Bottomley]

On Tue, 2015-06-23 at 12:47 -0500, Timothy Pearson wrote:
> On 06/23/2015 12:45 PM, James Bottomley wrote:
> > On Tue, 2015-06-23 at 12:33 -0500, Timothy Pearson wrote:
> >> On 06/23/2015 08:35 AM, James Bottomley wrote:
> >>> On Tue, 2015-06-23 at 18:06 +0530, Sreekanth Reddy wrote:
>  Hi,
> 
>  Upstream contributor should not add copyright to this driver code.
> >>>
> >>> I can't agree with that as a general rule: it depends on the
> >>> significance of the contribution.  The somewhat ill defined standard for
> >>> this is the contribution must be "an original work of authorship".
> >>>
> >>> I can agree that a trivial bug fix like this is not an original work of
> >>> authorship, so in this case adding copyright isn't appropriate.
> >>>
> >>> James
> >>>
>  Regards,
>  Sreekanth
> 
>  On Tue, Jun 23, 2015 at 9:24 AM, Joe Lawrence  
>   wrote:
> >
> >
> > On 06/21/2015 02:46 PM, Timothy Pearson wrote:
> >> On 06/16/2015 01:49 PM, Timothy Pearson wrote:
> >>> On 06/16/2015 12:42 PM, Joe Lawrence wrote:
>  On 06/16/2015 12:28 PM, Timothy Pearson wrote:
> > On 06/12/2015 05:05 PM, Timothy Pearson wrote:
> >> The mpt2sas driver crashes if the BIOS does not set up at least one
> >> memory I/O resource. This failure can happen if the device is too
> >> slow to respond during POST and is missed by the BIOS, but Linux
> >> then detects the device later in the boot process.
> >>
> >> This patch aborts initialization and prints a warning if no memory 
> >> I/O
> >> resources are found.
> >>
> >> Signed-off-by: Timothy Pearson
> >> Tested-by: Timothy Pearson
> >> ---
> >> drivers/scsi/mpt2sas/mpt2sas_base.c | 9 +
> >> 1 file changed, 9 insertions(+)
> >>
> >> diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c
> >> b/drivers/scsi/mpt2sas/mpt2sas_base.c
> >> index 11248de..15c9504 100644
> >> --- a/drivers/scsi/mpt2sas/mpt2sas_base.c
> >> +++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
> >> @@ -6,6 +6,8 @@
> >> * Copyright (C) 2007-2014 LSI Corporation
> >> * Copyright (C) 20013-2014 Avago Technologies
> >> * (mailto: mpt-fusionlinux@avagotech.com)
> >> + * Copyright (C) 2015 Raptor Engineering
> >> + * (mailto: supp...@araptorengineeringinc.com)
> >> *
> >> * This program is free software; you can redistribute it and/or
> >> * modify it under the terms of the GNU General Public License
> >> @@ -1582,6 +1584,13 @@ mpt2sas_base_map_resources(struct
> >> MPT2SAS_ADAPTER
> >> *ioc)
> >> }
> >> }
> >>
> >> + if (ioc->chip == NULL) {
> >> + printk(MPT2SAS_ERR_FMT "unable to map "
> >> + "adapter memory (resource not found)!\n", ioc->name);
> >> + r = -EINVAL;
> >> + goto out_fail;
> >> + }
> >> +
> >> _base_mask_interrupts(ioc);
> >>
> >> r = _base_get_ioc_facts(ioc, CAN_SLEEP);
> >
> > Just following up on this patch as I have not yet received any
> > response.
> >
> > Thanks!
> 
>  Hi Tim -- just curious, why was the similar check on ioc->chip just a
>  few lines above the one added by the patch insufficient?
> 
>  That loop block sets memap_sz when it finds an IORESOURCE_MEM so 
>  that it
>  only sets ioc->chip once. I wonder if the fix might be simpler if the
>  existing ioc->chip check relocated entirely to where you put it 
>  (maybe
>  also pulling the entire error text onto one line for easier 
>  grepping).
> 
>  Regards,
> 
>  -- Joe
> >>>
> >>> If there are no IORESOURCE_MEM resources allocated by the BIOS (i.e. 
> >>> if
> >>> the BIOS does not run resource allocation on the mpt2sas device) then
> >>> the check you are referring to is not executed, and the driver 
> >>> attempts
> >>> to perform operations on a null ioc->chip pointer.
> >>>
> >>> I can relocate the check if desired.
> >>>
> >>
> >> On looking more closely at the code I think having the two separate
> >> checks is useful for debugging.  The first error message is triggered 
> >> if
> >> the resource exists but Linux cannot map it for some reason, and the
> >> second error message is triggered if the resource does not exist at all
> >> (buggy BIOS).
> >
> > Fair enough.  The two error messages are unique, so grepping will lead
> > to the correct check.
> >
> > Only other nits would be the addition of a copyright (up to Avago I
> > guess) and an mpt3sas version (these drivers are 99% the same code).
> >
> > Acked-by: Joe Lawrence
> >
> > -- Joe
> 
> 

Re: [PATCH] mpt2sas: Abort initialization if no memory I/O resources, detected

[Timothy Pearson]

On 06/23/2015 12:45 PM, James Bottomley wrote:

On Tue, 2015-06-23 at 12:33 -0500, Timothy Pearson wrote:

On 06/23/2015 08:35 AM, James Bottomley wrote:

On Tue, 2015-06-23 at 18:06 +0530, Sreekanth Reddy wrote:

Hi,

Upstream contributor should not add copyright to this driver code.


I can't agree with that as a general rule: it depends on the
significance of the contribution.  The somewhat ill defined standard for
this is the contribution must be "an original work of authorship".

I can agree that a trivial bug fix like this is not an original work of
authorship, so in this case adding copyright isn't appropriate.

James


Regards,
Sreekanth

On Tue, Jun 23, 2015 at 9:24 AM, Joe Lawrence   wrote:



On 06/21/2015 02:46 PM, Timothy Pearson wrote:

On 06/16/2015 01:49 PM, Timothy Pearson wrote:

On 06/16/2015 12:42 PM, Joe Lawrence wrote:

On 06/16/2015 12:28 PM, Timothy Pearson wrote:

On 06/12/2015 05:05 PM, Timothy Pearson wrote:

The mpt2sas driver crashes if the BIOS does not set up at least one
memory I/O resource. This failure can happen if the device is too
slow to respond during POST and is missed by the BIOS, but Linux
then detects the device later in the boot process.

This patch aborts initialization and prints a warning if no memory I/O
resources are found.

Signed-off-by: Timothy Pearson
Tested-by: Timothy Pearson
---
drivers/scsi/mpt2sas/mpt2sas_base.c | 9 +
1 file changed, 9 insertions(+)

diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c
b/drivers/scsi/mpt2sas/mpt2sas_base.c
index 11248de..15c9504 100644
--- a/drivers/scsi/mpt2sas/mpt2sas_base.c
+++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
@@ -6,6 +6,8 @@
* Copyright (C) 2007-2014 LSI Corporation
* Copyright (C) 20013-2014 Avago Technologies
* (mailto: mpt-fusionlinux@avagotech.com)
+ * Copyright (C) 2015 Raptor Engineering
+ * (mailto: supp...@araptorengineeringinc.com)
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
@@ -1582,6 +1584,13 @@ mpt2sas_base_map_resources(struct
MPT2SAS_ADAPTER
*ioc)
}
}

+ if (ioc->chip == NULL) {
+ printk(MPT2SAS_ERR_FMT "unable to map "
+ "adapter memory (resource not found)!\n", ioc->name);
+ r = -EINVAL;
+ goto out_fail;
+ }
+
_base_mask_interrupts(ioc);

r = _base_get_ioc_facts(ioc, CAN_SLEEP);


Just following up on this patch as I have not yet received any
response.

Thanks!


Hi Tim -- just curious, why was the similar check on ioc->chip just a
few lines above the one added by the patch insufficient?

That loop block sets memap_sz when it finds an IORESOURCE_MEM so that it
only sets ioc->chip once. I wonder if the fix might be simpler if the
existing ioc->chip check relocated entirely to where you put it (maybe
also pulling the entire error text onto one line for easier grepping).

Regards,

-- Joe


If there are no IORESOURCE_MEM resources allocated by the BIOS (i.e. if
the BIOS does not run resource allocation on the mpt2sas device) then
the check you are referring to is not executed, and the driver attempts
to perform operations on a null ioc->chip pointer.

I can relocate the check if desired.



On looking more closely at the code I think having the two separate
checks is useful for debugging.  The first error message is triggered if
the resource exists but Linux cannot map it for some reason, and the
second error message is triggered if the resource does not exist at all
(buggy BIOS).


Fair enough.  The two error messages are unique, so grepping will lead
to the correct check.

Only other nits would be the addition of a copyright (up to Avago I
guess) and an mpt3sas version (these drivers are 99% the same code).

Acked-by: Joe Lawrence

-- Joe








Thank you for clarifying the guidelines on copyright lines.  I wasn't
sure if this was significant enough of a contribution to justify the
line or not.

Do I need to re-send or can I just state here that the patch can be
merged without the copyright line?


It's easier if you send a clean copy rather than risk me botching
something in the hand edit.


OK, will do.


I don't have any mpt3sas hardware to test with; should I just send in a
patch anyway without a Tested-by line?


Well, if you haven't actually tested it, adding a tested-by line
wouldn't be correct.


Of course not.  My question was, should I send in a patch anyway (with 
appropriate commit message fields) even though it has not been tested?


Thanks!

--
Timothy Pearson
Raptor Engineering
+1 (415) 727-8645
http://www.raptorengineeringinc.com
--
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] mpt2sas: Abort initialization if no memory I/O resources, detected

[James Bottomley]

On Tue, 2015-06-23 at 12:33 -0500, Timothy Pearson wrote:
> On 06/23/2015 08:35 AM, James Bottomley wrote:
> > On Tue, 2015-06-23 at 18:06 +0530, Sreekanth Reddy wrote:
> >> Hi,
> >>
> >> Upstream contributor should not add copyright to this driver code.
> >
> > I can't agree with that as a general rule: it depends on the
> > significance of the contribution.  The somewhat ill defined standard for
> > this is the contribution must be "an original work of authorship".
> >
> > I can agree that a trivial bug fix like this is not an original work of
> > authorship, so in this case adding copyright isn't appropriate.
> >
> > James
> >
> >> Regards,
> >> Sreekanth
> >>
> >> On Tue, Jun 23, 2015 at 9:24 AM, Joe Lawrence  
> >> wrote:
> >>>
> >>>
> >>> On 06/21/2015 02:46 PM, Timothy Pearson wrote:
>  On 06/16/2015 01:49 PM, Timothy Pearson wrote:
> > On 06/16/2015 12:42 PM, Joe Lawrence wrote:
> >> On 06/16/2015 12:28 PM, Timothy Pearson wrote:
> >>> On 06/12/2015 05:05 PM, Timothy Pearson wrote:
>  The mpt2sas driver crashes if the BIOS does not set up at least one
>  memory I/O resource. This failure can happen if the device is too
>  slow to respond during POST and is missed by the BIOS, but Linux
>  then detects the device later in the boot process.
> 
>  This patch aborts initialization and prints a warning if no memory 
>  I/O
>  resources are found.
> 
>  Signed-off-by: Timothy Pearson
>  Tested-by: Timothy Pearson
>  ---
>  drivers/scsi/mpt2sas/mpt2sas_base.c | 9 +
>  1 file changed, 9 insertions(+)
> 
>  diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c
>  b/drivers/scsi/mpt2sas/mpt2sas_base.c
>  index 11248de..15c9504 100644
>  --- a/drivers/scsi/mpt2sas/mpt2sas_base.c
>  +++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
>  @@ -6,6 +6,8 @@
>  * Copyright (C) 2007-2014 LSI Corporation
>  * Copyright (C) 20013-2014 Avago Technologies
>  * (mailto: mpt-fusionlinux@avagotech.com)
>  + * Copyright (C) 2015 Raptor Engineering
>  + * (mailto: supp...@araptorengineeringinc.com)
>  *
>  * This program is free software; you can redistribute it and/or
>  * modify it under the terms of the GNU General Public License
>  @@ -1582,6 +1584,13 @@ mpt2sas_base_map_resources(struct
>  MPT2SAS_ADAPTER
>  *ioc)
>  }
>  }
> 
>  + if (ioc->chip == NULL) {
>  + printk(MPT2SAS_ERR_FMT "unable to map "
>  + "adapter memory (resource not found)!\n", ioc->name);
>  + r = -EINVAL;
>  + goto out_fail;
>  + }
>  +
>  _base_mask_interrupts(ioc);
> 
>  r = _base_get_ioc_facts(ioc, CAN_SLEEP);
> >>>
> >>> Just following up on this patch as I have not yet received any
> >>> response.
> >>>
> >>> Thanks!
> >>
> >> Hi Tim -- just curious, why was the similar check on ioc->chip just a
> >> few lines above the one added by the patch insufficient?
> >>
> >> That loop block sets memap_sz when it finds an IORESOURCE_MEM so that 
> >> it
> >> only sets ioc->chip once. I wonder if the fix might be simpler if the
> >> existing ioc->chip check relocated entirely to where you put it (maybe
> >> also pulling the entire error text onto one line for easier grepping).
> >>
> >> Regards,
> >>
> >> -- Joe
> >
> > If there are no IORESOURCE_MEM resources allocated by the BIOS (i.e. if
> > the BIOS does not run resource allocation on the mpt2sas device) then
> > the check you are referring to is not executed, and the driver attempts
> > to perform operations on a null ioc->chip pointer.
> >
> > I can relocate the check if desired.
> >
> 
>  On looking more closely at the code I think having the two separate
>  checks is useful for debugging.  The first error message is triggered if
>  the resource exists but Linux cannot map it for some reason, and the
>  second error message is triggered if the resource does not exist at all
>  (buggy BIOS).
> >>>
> >>> Fair enough.  The two error messages are unique, so grepping will lead
> >>> to the correct check.
> >>>
> >>> Only other nits would be the addition of a copyright (up to Avago I
> >>> guess) and an mpt3sas version (these drivers are 99% the same code).
> >>>
> >>> Acked-by: Joe Lawrence
> >>>
> >>> -- Joe
> >>
> >>
> >>
> >
> 
> Thank you for clarifying the guidelines on copyright lines.  I wasn't 
> sure if this was significant enough of a contribution to justify the 
> line or not.
> 
> Do I need to re-send or can I just state here that the patch can be 
> merged without the copyright line?

It's easier if you send a clean copy rather than risk me botching
something in the hand edit.

> I don't have any mpt

Re: [PATCH] mpt2sas: Abort initialization if no memory I/O resources, detected

[Timothy Pearson]

On 06/23/2015 08:35 AM, James Bottomley wrote:

On Tue, 2015-06-23 at 18:06 +0530, Sreekanth Reddy wrote:

Hi,

Upstream contributor should not add copyright to this driver code.


I can't agree with that as a general rule: it depends on the
significance of the contribution.  The somewhat ill defined standard for
this is the contribution must be "an original work of authorship".

I can agree that a trivial bug fix like this is not an original work of
authorship, so in this case adding copyright isn't appropriate.

James


Regards,
Sreekanth

On Tue, Jun 23, 2015 at 9:24 AM, Joe Lawrence  wrote:



On 06/21/2015 02:46 PM, Timothy Pearson wrote:

On 06/16/2015 01:49 PM, Timothy Pearson wrote:

On 06/16/2015 12:42 PM, Joe Lawrence wrote:

On 06/16/2015 12:28 PM, Timothy Pearson wrote:

On 06/12/2015 05:05 PM, Timothy Pearson wrote:

The mpt2sas driver crashes if the BIOS does not set up at least one
memory I/O resource. This failure can happen if the device is too
slow to respond during POST and is missed by the BIOS, but Linux
then detects the device later in the boot process.

This patch aborts initialization and prints a warning if no memory I/O
resources are found.

Signed-off-by: Timothy Pearson
Tested-by: Timothy Pearson
---
drivers/scsi/mpt2sas/mpt2sas_base.c | 9 +
1 file changed, 9 insertions(+)

diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c
b/drivers/scsi/mpt2sas/mpt2sas_base.c
index 11248de..15c9504 100644
--- a/drivers/scsi/mpt2sas/mpt2sas_base.c
+++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
@@ -6,6 +6,8 @@
* Copyright (C) 2007-2014 LSI Corporation
* Copyright (C) 20013-2014 Avago Technologies
* (mailto: mpt-fusionlinux@avagotech.com)
+ * Copyright (C) 2015 Raptor Engineering
+ * (mailto: supp...@araptorengineeringinc.com)
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
@@ -1582,6 +1584,13 @@ mpt2sas_base_map_resources(struct
MPT2SAS_ADAPTER
*ioc)
}
}

+ if (ioc->chip == NULL) {
+ printk(MPT2SAS_ERR_FMT "unable to map "
+ "adapter memory (resource not found)!\n", ioc->name);
+ r = -EINVAL;
+ goto out_fail;
+ }
+
_base_mask_interrupts(ioc);

r = _base_get_ioc_facts(ioc, CAN_SLEEP);


Just following up on this patch as I have not yet received any
response.

Thanks!


Hi Tim -- just curious, why was the similar check on ioc->chip just a
few lines above the one added by the patch insufficient?

That loop block sets memap_sz when it finds an IORESOURCE_MEM so that it
only sets ioc->chip once. I wonder if the fix might be simpler if the
existing ioc->chip check relocated entirely to where you put it (maybe
also pulling the entire error text onto one line for easier grepping).

Regards,

-- Joe


If there are no IORESOURCE_MEM resources allocated by the BIOS (i.e. if
the BIOS does not run resource allocation on the mpt2sas device) then
the check you are referring to is not executed, and the driver attempts
to perform operations on a null ioc->chip pointer.

I can relocate the check if desired.



On looking more closely at the code I think having the two separate
checks is useful for debugging.  The first error message is triggered if
the resource exists but Linux cannot map it for some reason, and the
second error message is triggered if the resource does not exist at all
(buggy BIOS).


Fair enough.  The two error messages are unique, so grepping will lead
to the correct check.

Only other nits would be the addition of a copyright (up to Avago I
guess) and an mpt3sas version (these drivers are 99% the same code).

Acked-by: Joe Lawrence

-- Joe








Thank you for clarifying the guidelines on copyright lines.  I wasn't 
sure if this was significant enough of a contribution to justify the 
line or not.


Do I need to re-send or can I just state here that the patch can be 
merged without the copyright line?


I don't have any mpt3sas hardware to test with; should I just send in a 
patch anyway without a Tested-by line?


Thanks!

--
Timothy Pearson
Raptor Engineering
+1 (415) 727-8645
http://www.raptorengineeringinc.com
--
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: configurable discard parameters

[Tom Yan]

On 24 June 2015 at 01:03, Martin K. Petersen  wrote:
>
> It don't know what these "lower limits" you are talking about are.
>

[tom@localhost ~]$ sudo shred -v -n 1 /dev/sda3

[tom@localhost ~]$ sudo blkdiscard -l 512 /dev/sda3
[tom@localhost ~]$ sudo hexdump -n 4096 /dev/sda3 | head
000 58c7 f3b0 0f97 2a9a 2da8 7c44 11f5 7852
010 4f1d d19c 682b a3d8 6be9 61df 4eb3 a4ee
020 f28b f2f6 ef36 c244 f1b0 eadd 5ad0 c5ec
030 54c4 abb4 f9c3 7ca0 d807 63ad 81cd 45e6
040 2b30 709c 550a 5e52 6928 4468 78c9 f671
050 eef1 dfa7 2a6d 3150 64f8 f9a2 240d bbf4
060 2cff 02e0 c893 16a7 67c2 8cd9 af76 71b6
070 4740 83cc 5b15 3db0 64b2 1a6d 3f40 5a98
080 09d0 035c b5ae 2cd2 1850 ea70 7a09 0491
090 6b27 9d81 9d74 a087 afd9 e75a 85ea adf6

[tom@localhost ~]$ sudo blkdiscard -l 3584 /dev/sda3
[tom@localhost ~]$ sudo hexdump -n 4096 /dev/sda3 | head
000 58c7 f3b0 0f97 2a9a 2da8 7c44 11f5 7852
010 4f1d d19c 682b a3d8 6be9 61df 4eb3 a4ee
020 f28b f2f6 ef36 c244 f1b0 eadd 5ad0 c5ec
030 54c4 abb4 f9c3 7ca0 d807 63ad 81cd 45e6
040 2b30 709c 550a 5e52 6928 4468 78c9 f671
050 eef1 dfa7 2a6d 3150 64f8 f9a2 240d bbf4
060 2cff 02e0 c893 16a7 67c2 8cd9 af76 71b6
070 4740 83cc 5b15 3db0 64b2 1a6d 3f40 5a98
080 09d0 035c b5ae 2cd2 1850 ea70 7a09 0491
090 6b27 9d81 9d74 a087 afd9 e75a 85ea adf6

[tom@localhost ~]$ sudo blkdiscard -l 4096 /dev/sda3
[tom@localhost ~]$ sudo hexdump -n 4096 /dev/sda3 | head
000        
*
0001000

So when libata issue ATA commands with ranges of 65535 sectors, only
65535-(65535%8) = 65528 sectors are discarded, yet it wouldn't know
about that so the next range will start from LBA 65536. I can
workaround this by specifying --step in blkdiscard, but I think the
kernel should have a param configurable for general.

>
> What hdparm tells you is that DSM TRIM on the Intel drive supports 1
> block (512 bytes) of range payload data. Whereas the SanDisk drive
> supports a full 4KB page of range payload data (8 * 512 bytes).
>

Oh so they are not reporting garbage. That's good to know. I guess
that's why hdparm actually splits the range list I provided by
512-range per ATA command (which works) for the SanDisk drive.

>
> We did try to enable payloads bigger than 512 bytes back in the day but
> most drives that reported supporting the bigger payload actually didn't
> and all hell broke loose. So we reverted to a single sector payload for
> libata.
>
> I still have the payload patch in my SSD test branch and regularly test
> with it. But there are still drives that fail in mysterious ways with it
> in place and so far I haven't felt compelled to maintain yet another
> blacklist.
>

For now I have no problem with the single sector payload thing.
--
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: configurable discard parameters

[Martin K. Petersen]

> "Tom" == Tom Yan  writes:

Tom> Currently what the kernel does is assume all devices support 1
Tom> sector granularity.

The ATA Command Set does not allow for any other granularity than 1
sector.

Bigger granularity reporting is supported in SCSI SBC to allow for
thinly provisioned disk arrays with a bigger allocation unit. It is
common for disk arrays to provision LUN space in units of 1MB or more.

Tom> For my Intel SSD, which actually has a lower limit of 8 sectors, it
Tom> shows: Data Set Management TRIM supported (limit 1 block) while for
Tom> the SanDisk Extreme USB, which actually has a lower limit of 256
Tom> sectors, it shows: Data Set Management TRIM supported (limit 8
Tom> blocks)

It don't know what these "lower limits" you are talking about are.

What hdparm tells you is that DSM TRIM on the Intel drive supports 1
block (512 bytes) of range payload data. Whereas the SanDisk drive
supports a full 4KB page of range payload data (8 * 512 bytes).

We did try to enable payloads bigger than 512 bytes back in the day but
most drives that reported supporting the bigger payload actually didn't
and all hell broke loose. So we reverted to a single sector payload for
libata.

I still have the payload patch in my SSD test branch and regularly test
with it. But there are still drives that fail in mysterious ways with it
in place and so far I haven't felt compelled to maintain yet another
blacklist.

-- 
Martin K. Petersen  Oracle Linux Engineering
--
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: [Open-FCoE] [PATCH] [trivial] scsi:fcoe: Fix typo "a ethernet" in fcoe_transport.c

[Vasu Dev]

On Thu, 2015-06-04 at 00:54 +0900, Masanari Iida wrote:
> This patch fix some "a ethernet" in MODULE_DESCRIPTIONS in
> fcoe_transport.c
> 
> Signed-off-by: Masanari Iida 
> ---
>  drivers/scsi/fcoe/fcoe_transport.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/scsi/fcoe/fcoe_transport.c 
> b/drivers/scsi/fcoe/fcoe_transport.c
> index bdc8989..d7597c0 100644
> --- a/drivers/scsi/fcoe/fcoe_transport.c
> +++ b/drivers/scsi/fcoe/fcoe_transport.c
> @@ -58,7 +58,7 @@ MODULE_PARM_DESC(show, " Show attached FCoE transports");
>  module_param_call(create, fcoe_transport_create, NULL,
> (void *)FIP_MODE_FABRIC, S_IWUSR);
>  __MODULE_PARM_TYPE(create, "string");
> -MODULE_PARM_DESC(create, " Creates fcoe instance on a ethernet interface");
> +MODULE_PARM_DESC(create, " Creates fcoe instance on an ethernet interface");
>  
>  module_param_call(create_vn2vn, fcoe_transport_create, NULL,
> (void *)FIP_MODE_VN2VN, S_IWUSR);
> @@ -68,15 +68,15 @@ MODULE_PARM_DESC(create_vn2vn, " Creates a VN_node to 
> VN_node FCoE instance "
>  
>  module_param_call(destroy, fcoe_transport_destroy, NULL, NULL, S_IWUSR);
>  __MODULE_PARM_TYPE(destroy, "string");
> -MODULE_PARM_DESC(destroy, " Destroys fcoe instance on a ethernet interface");
> +MODULE_PARM_DESC(destroy, " Destroys fcoe instance on an ethernet 
> interface");
>  
>  module_param_call(enable, fcoe_transport_enable, NULL, NULL, S_IWUSR);
>  __MODULE_PARM_TYPE(enable, "string");
> -MODULE_PARM_DESC(enable, " Enables fcoe on a ethernet interface.");
> +MODULE_PARM_DESC(enable, " Enables fcoe on an ethernet interface.");
>  
>  module_param_call(disable, fcoe_transport_disable, NULL, NULL, S_IWUSR);
>  __MODULE_PARM_TYPE(disable, "string");
> -MODULE_PARM_DESC(disable, " Disables fcoe on a ethernet interface.");
> +MODULE_PARM_DESC(disable, " Disables fcoe on an ethernet interface.");
>  
>  /* notification function for packets from net device */
>  static struct notifier_block libfcoe_notifier = {

I don't this patch applied yet, Looks good.

Acked-by: Vasu Dev 

--
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: configurable discard parameters

[Tom Yan]

On 23 June 2015 at 23:36, Martin K. Petersen  wrote:
>
> You haven't given us any reason to. We are not aware of any ATA drives
> that put constraints on the range block count.
>

What I have been doing is trying to show you example of those
constraints. When I talked about the block count limit of the SanDisk
Extreme USB, I am not asking you to fix anything for the drive I HAVE
(because I can only use hdparm to TRIM it anyway), but to show you
that some devices MIGHT have such limit. I just can't be 100% sure of
it, even though from what I see in the different results on payload
size and range sector count, I don't think the bridge is intervening
at all (but just the limit of the SATA controller behind).

But for my Intel 530 SSD, the granularity problem is obvious and solid
enough because it's connected directly with SATA. Currently what the
kernel does is assume all devices support 1 sector granularity. And
the problem doesn't even lie on the current "hardcoded" granularity in
libata, because blkdev_issue_discard() only does "mod check" against
the granularity on the maximum sector counts, so even if it's allowed
to be configured, it wouldn't really help. And I have yet to see
anything which actually does "mod check" against ANY granularity on
the sector count per range. This is what the example in my last mail
was about.

And the only info I have ever saw about TRIM block limit of a SATA
drive is in `hdparm -I`. For my Intel SSD, which actually has a lower
limit of 8 sectors, it shows:
Data Set Management TRIM supported (limit 1 block)
while for the SanDisk Extreme USB, which actually has a lower limit of
256 sectors, it shows:
Data Set Management TRIM supported (limit 8 blocks)
Inspite of the accuracy, I don't see the kernel actually read this limit.
--
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 v1 09/20] [PATCH 09/20] [SCSI] mpt3sas: MPI 2.5 Rev J (2.5.5) specification and 2.00.34 header files

[Martin K. Petersen]

> Sreekanth Reddy  writes:

> Following is the change set, 1. Added more defines for the BiosOptions
> field of MPI2_CONFIG_PAGE_BIOS_1.  2. Added
> MPI2_TOOLBOX_CLEAN_BIT26_PRODUCT_SPECIFIC definition.

Reviewed-by: Martin K. Petersen 

-- 
Martin K. Petersen  Oracle Linux Engineering
--
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 v2 01/20] [SCSI] mpt3sas: Added Combined Reply Queue feature to extend up-to 96 MSIX vector support

[Martin K. Petersen]

> "Sreekanth" == Sreekanth Reddy  writes:

Sreekanth> In this patch, increased the number of MSIX vector support
Sreekanth> for SAS3 C0 HBAs to up-to 96.

Reviewed-by: Martin K. Petersen 

-- 
Martin K. Petersen  Oracle Linux Engineering
--
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 2/2] scsi: Remove VPD quirk for Seagate drives

[Martin K. Petersen]

Now that we sanity check the optimal I/O size reported by the device we
no longer need to blacklist the VPD pages on certain Seagate drives.

Signed-off-by: Martin K. Petersen 
Cc: sta...@vger.kernel.org
---
 drivers/scsi/scsi_devinfo.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/scsi/scsi_devinfo.c b/drivers/scsi/scsi_devinfo.c
index 9f77d23239a2..f04f1b672962 100644
--- a/drivers/scsi/scsi_devinfo.c
+++ b/drivers/scsi/scsi_devinfo.c
@@ -232,7 +232,6 @@ static struct {
{"SanDisk", "ImageMate CF-SD1", NULL, BLIST_FORCELUN},
{"SEAGATE", "ST34555N", "0930", BLIST_NOTQ},/* Chokes on tagged 
INQUIRY */
{"SEAGATE", "ST3390N", "9546", BLIST_NOTQ},
-   {"SEAGATE", "ST900MM0006", NULL, BLIST_SKIP_VPD_PAGES},
{"SGI", "RAID3", "*", BLIST_SPARSELUN},
{"SGI", "RAID5", "*", BLIST_SPARSELUN},
{"SGI", "TP9100", "*", BLIST_REPORTLUN2},
-- 
2.4.3

--
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 1/2] sd: Sanity check the optimal I/O size

[Martin K. Petersen]

We have come across a couple of devices that report unreasonable values
in the optimal I/O size in the Block Limits VPD page. Since this is a
32-bit entity that gets multiplied by the logical block size we can get
disproportionately large values reported to the block layer.

Cap io_opt at 256 MB.

Signed-off-by: Martin K. Petersen 
Reported-by: Chris Friesen 
Cc: sta...@vger.kernel.org
---
 drivers/scsi/sd.c | 3 ++-
 drivers/scsi/sd.h | 9 +
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index a20da8c25b4f..118b336e0ddf 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -2560,7 +2560,8 @@ static void sd_read_block_limits(struct scsi_disk *sdkp)
blk_queue_io_min(sdkp->disk->queue,
 get_unaligned_be16(&buffer[6]) * sector_sz);
blk_queue_io_opt(sdkp->disk->queue,
-get_unaligned_be32(&buffer[12]) * sector_sz);
+min_t(unsigned int, SD_MAX_IO_OPT_BYTES,
+  get_unaligned_be32(&buffer[12]) * sector_sz));
 
if (buffer[3] == 0x3c) {
unsigned int lba_count, desc_count;
diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
index 63ba5ca7f9a1..f175a3f2944a 100644
--- a/drivers/scsi/sd.h
+++ b/drivers/scsi/sd.h
@@ -44,10 +44,11 @@ enum {
 };
 
 enum {
-   SD_DEF_XFER_BLOCKS = 0x,
-   SD_MAX_XFER_BLOCKS = 0x,
-   SD_MAX_WS10_BLOCKS = 0x,
-   SD_MAX_WS16_BLOCKS = 0x7f,
+   SD_DEF_XFER_BLOCKS  = 0x,
+   SD_MAX_XFER_BLOCKS  = 0x,
+   SD_MAX_WS10_BLOCKS  = 0x,
+   SD_MAX_WS16_BLOCKS  = 0x7f,
+   SD_MAX_IO_OPT_BYTES = 256 * 1024 * 1024,
 };
 
 enum {
-- 
2.4.3

--
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] sd: Fix maximum I/O size for BLOCK_PC requests

[Martin K. Petersen]

Commit bcdb247c6b6a ("sd: Limit transfer length") clamped the maximum
size of an I/O request to the MAXIMUM TRANSFER LENGTH field in the BLOCK
LIMITS VPD. This had the unfortunate effect of also limiting the maximum
size of non-filesystem requests sent to the device through sg/bsg.

Avoid using blk_queue_max_hw_sectors() and set the max_sectors queue
limit directly.

Also update the comment in blk_limits_max_hw_sectors() to clarify that
max_hw_sectors defines the limit for the I/O controller only.

Signed-off-by: Martin K. Petersen 
Reported-by: Brian King 
Tested-by: Brian King 
Cc: sta...@vger.kernel.org # 3.17+
---
 block/blk-settings.c | 4 ++--
 drivers/scsi/sd.c| 6 +++---
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/block/blk-settings.c b/block/blk-settings.c
index 12600bfffca9..e0057d035200 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -241,8 +241,8 @@ EXPORT_SYMBOL(blk_queue_bounce_limit);
  * Description:
  *Enables a low level driver to set a hard upper limit,
  *max_hw_sectors, on the size of requests.  max_hw_sectors is set by
- *the device driver based upon the combined capabilities of I/O
- *controller and storage device.
+ *the device driver based upon the capabilities of the I/O
+ *controller.
  *
  *max_sectors is a soft limit imposed by the block layer for
  *filesystem type requests.  This value can be overridden on a
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 3b2fcb4fada0..a20da8c25b4f 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -2770,9 +2770,9 @@ static int sd_revalidate_disk(struct gendisk *disk)
max_xfer = sdkp->max_xfer_blocks;
max_xfer <<= ilog2(sdp->sector_size) - 9;
 
-   max_xfer = min_not_zero(queue_max_hw_sectors(sdkp->disk->queue),
-   max_xfer);
-   blk_queue_max_hw_sectors(sdkp->disk->queue, max_xfer);
+   sdkp->disk->queue->limits.max_sectors =
+   min_not_zero(queue_max_hw_sectors(sdkp->disk->queue), max_xfer);
+
set_capacity(disk, sdkp->capacity);
sd_config_write_same(sdkp);
kfree(buffer);
-- 
2.4.3

--
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: configurable discard parameters

[Martin K. Petersen]

> "Tom" == Tom Yan  writes:

Tom> I don't know whether the USB bridging or the way hdparm does TRIM
Tom> matters, but it seems that some devices can't really handle limit
Tom> like 0x3fffc0 blocks.

The 0x3fffc0 limit is for SATA devices connected through Linux' libata
SCSI ATA translation. This number corresponds to the biggest range we
can discard while maintaining a 1:1 mapping between the SCSI command and
the ATA ditto.

If you connect the SATA drive to a USB/UAS bridge you are entirely at
the bridge's whim when it comes to translation of WRITE SAME(10/16) w/
UNMAP or the UNMAP command to DSM TRIM. libata is not involved and
neither is the 0x3fffc0 limit (although chances are that the drive has
the same limit internally). Even when using hdparm the bridge device
still needs to correctly translate the ATA PASSTHROUGH commands.

Tom> So I still think that the kernel should allow users to configure
Tom> limits that can make libata send reliable ATA TRIM commands.

You haven't given us any reason to. We are not aware of any ATA drives
that put constraints on the range block count.

Again, if you want to help please focus on your bridge devices and what,
if anything, can be done to uniquely identify them and override any
incorrect values they might report to the SCSI stack. Up to and
including us disabling discard entirely on these devices if their
translation isn't verifiable and bullet proof.

-- 
Martin K. Petersen  Oracle Linux Engineering
--
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] USB: storage: add "no SYNCHRONIZE CACHE" quirk

[David Laight]

From: Of James Bottomley
> Sent: 22 June 2015 18:36
> To: Alan Stern
...
> > > Obviously, for a disk with a writeback cache that can't do flush, that
> > > window is much wider and the real solution should be to try to switch
> > > the cache to write through.
> >
> > I agree.  Doing the switch manually (by writing to the "cache_type"
> > attribute file) works, but it's a nuisance to do this when you have a
> > portable USB drive that gets moved among a bunch of machines.
> 
> Perhaps it might be wise to do this to every USB device ... for external
> devices, the small performance gain doesn't really make up for the
> potential data loss.

What about systems running from USB memory/disk directly plugged into the
motherboard and shut inside the case?

Since they shouldn't be removed you don't want to bypass any write cache.
(Any more than you'd want to on a real disk.)

There is an additional problem caused by very temporary USB disconnects
(easily caused by electrical noise causing (probably) Vbus to bounce).
In these conditions you don't want to signal a USB remove at all - at
least not to the filesystem - until the device has remained disconnected
for a short time.

David

N�r��yb�X��ǧv�^�)޺{.n�+{���"�{ay�ʇڙ�,j��f���h���z��w���
���j:+v���w�j�mzZ+�ݢj"��!�i

Re: configurable discard parameters

[Tom Yan]

Still, I wonder if the kernel should allow the user to configure the
real granularity and send ATA commands that are rounded off by it
(which works just like --step in blkdiscard). For example,

(64 * 65535) % 8 = 0

but

65535 % 8 = 7

So the block count limit for the ATA commands would be 65535 - (65535
% real_gran) per range.


Also, by experimenting on my SanDisk Extreme USB with hdparm TRIM and
hexdump, I can see that the maximum block/sector allowed to be
discarded per ATA command is 65536, inspite of the payload size. I
don't know whether the USB bridging or the way hdparm does TRIM
matters, but it seems that some devices can't really handle limit like
0x3fffc0 blocks.

So I still think that the kernel should allow users to configure
limits that can make libata send reliable ATA TRIM commands.

On 23 June 2015 at 04:57, Martin K. Petersen  wrote:
>
> Tom> So it actually tells. And I hope that the kernel wouldn't "falsify"
> Tom> anything for devices which do provide some VPD(s). : \
>
> SATA doesn't have VPDs.
>

I know now, that's why I think it's "alright" for libata to do so. I
just don't know whether the kernel would tamper with them for devices
like USB or real SCSI drives.
--
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] mpt2sas: Abort initialization if no memory I/O resources, detected

[Sreekanth Reddy]

On Tue, Jun 23, 2015 at 7:05 PM, James Bottomley
 wrote:
> On Tue, 2015-06-23 at 18:06 +0530, Sreekanth Reddy wrote:
>> Hi,
>>
>> Upstream contributor should not add copyright to this driver code.
>
> I can't agree with that as a general rule: it depends on the
> significance of the contribution.  The somewhat ill defined standard for
> this is the contribution must be "an original work of authorship".

Thanks James. I am not aware of this and this is a learning point for
me. As most of the time Avago is the major contributor for this driver
so Initial I thought like that.

Thanks,
Sreekanth

>
> I can agree that a trivial bug fix like this is not an original work of
> authorship, so in this case adding copyright isn't appropriate.
>
> James
>
>> Regards,
>> Sreekanth
>>
>> On Tue, Jun 23, 2015 at 9:24 AM, Joe Lawrence  
>> wrote:
>> >
>> >
>> > On 06/21/2015 02:46 PM, Timothy Pearson wrote:
>> >> On 06/16/2015 01:49 PM, Timothy Pearson wrote:
>> >>> On 06/16/2015 12:42 PM, Joe Lawrence wrote:
>>  On 06/16/2015 12:28 PM, Timothy Pearson wrote:
>> > On 06/12/2015 05:05 PM, Timothy Pearson wrote:
>> >> The mpt2sas driver crashes if the BIOS does not set up at least one
>> >> memory I/O resource. This failure can happen if the device is too
>> >> slow to respond during POST and is missed by the BIOS, but Linux
>> >> then detects the device later in the boot process.
>> >>
>> >> This patch aborts initialization and prints a warning if no memory I/O
>> >> resources are found.
>> >>
>> >> Signed-off-by: Timothy Pearson
>> >> Tested-by: Timothy Pearson
>> >> ---
>> >> drivers/scsi/mpt2sas/mpt2sas_base.c | 9 +
>> >> 1 file changed, 9 insertions(+)
>> >>
>> >> diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c
>> >> b/drivers/scsi/mpt2sas/mpt2sas_base.c
>> >> index 11248de..15c9504 100644
>> >> --- a/drivers/scsi/mpt2sas/mpt2sas_base.c
>> >> +++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
>> >> @@ -6,6 +6,8 @@
>> >> * Copyright (C) 2007-2014 LSI Corporation
>> >> * Copyright (C) 20013-2014 Avago Technologies
>> >> * (mailto: mpt-fusionlinux@avagotech.com)
>> >> + * Copyright (C) 2015 Raptor Engineering
>> >> + * (mailto: supp...@araptorengineeringinc.com)
>> >> *
>> >> * This program is free software; you can redistribute it and/or
>> >> * modify it under the terms of the GNU General Public License
>> >> @@ -1582,6 +1584,13 @@ mpt2sas_base_map_resources(struct
>> >> MPT2SAS_ADAPTER
>> >> *ioc)
>> >> }
>> >> }
>> >>
>> >> + if (ioc->chip == NULL) {
>> >> + printk(MPT2SAS_ERR_FMT "unable to map "
>> >> + "adapter memory (resource not found)!\n", ioc->name);
>> >> + r = -EINVAL;
>> >> + goto out_fail;
>> >> + }
>> >> +
>> >> _base_mask_interrupts(ioc);
>> >>
>> >> r = _base_get_ioc_facts(ioc, CAN_SLEEP);
>> >
>> > Just following up on this patch as I have not yet received any
>> > response.
>> >
>> > Thanks!
>> 
>>  Hi Tim -- just curious, why was the similar check on ioc->chip just a
>>  few lines above the one added by the patch insufficient?
>> 
>>  That loop block sets memap_sz when it finds an IORESOURCE_MEM so that it
>>  only sets ioc->chip once. I wonder if the fix might be simpler if the
>>  existing ioc->chip check relocated entirely to where you put it (maybe
>>  also pulling the entire error text onto one line for easier grepping).
>> 
>>  Regards,
>> 
>>  -- Joe
>> >>>
>> >>> If there are no IORESOURCE_MEM resources allocated by the BIOS (i.e. if
>> >>> the BIOS does not run resource allocation on the mpt2sas device) then
>> >>> the check you are referring to is not executed, and the driver attempts
>> >>> to perform operations on a null ioc->chip pointer.
>> >>>
>> >>> I can relocate the check if desired.
>> >>>
>> >>
>> >> On looking more closely at the code I think having the two separate
>> >> checks is useful for debugging.  The first error message is triggered if
>> >> the resource exists but Linux cannot map it for some reason, and the
>> >> second error message is triggered if the resource does not exist at all
>> >> (buggy BIOS).
>> >
>> > Fair enough.  The two error messages are unique, so grepping will lead
>> > to the correct check.
>> >
>> > Only other nits would be the addition of a copyright (up to Avago I
>> > guess) and an mpt3sas version (these drivers are 99% the same code).
>> >
>> > Acked-by: Joe Lawrence 
>> >
>> > -- Joe
>>
>>
>>
>
>
>



-- 

Regards,
Sreekanth
--
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] mpt2sas: Abort initialization if no memory I/O resources, detected

[James Bottomley]

On Tue, 2015-06-23 at 18:06 +0530, Sreekanth Reddy wrote:
> Hi,
> 
> Upstream contributor should not add copyright to this driver code.

I can't agree with that as a general rule: it depends on the
significance of the contribution.  The somewhat ill defined standard for
this is the contribution must be "an original work of authorship".

I can agree that a trivial bug fix like this is not an original work of
authorship, so in this case adding copyright isn't appropriate.

James

> Regards,
> Sreekanth
> 
> On Tue, Jun 23, 2015 at 9:24 AM, Joe Lawrence  
> wrote:
> >
> >
> > On 06/21/2015 02:46 PM, Timothy Pearson wrote:
> >> On 06/16/2015 01:49 PM, Timothy Pearson wrote:
> >>> On 06/16/2015 12:42 PM, Joe Lawrence wrote:
>  On 06/16/2015 12:28 PM, Timothy Pearson wrote:
> > On 06/12/2015 05:05 PM, Timothy Pearson wrote:
> >> The mpt2sas driver crashes if the BIOS does not set up at least one
> >> memory I/O resource. This failure can happen if the device is too
> >> slow to respond during POST and is missed by the BIOS, but Linux
> >> then detects the device later in the boot process.
> >>
> >> This patch aborts initialization and prints a warning if no memory I/O
> >> resources are found.
> >>
> >> Signed-off-by: Timothy Pearson
> >> Tested-by: Timothy Pearson
> >> ---
> >> drivers/scsi/mpt2sas/mpt2sas_base.c | 9 +
> >> 1 file changed, 9 insertions(+)
> >>
> >> diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c
> >> b/drivers/scsi/mpt2sas/mpt2sas_base.c
> >> index 11248de..15c9504 100644
> >> --- a/drivers/scsi/mpt2sas/mpt2sas_base.c
> >> +++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
> >> @@ -6,6 +6,8 @@
> >> * Copyright (C) 2007-2014 LSI Corporation
> >> * Copyright (C) 20013-2014 Avago Technologies
> >> * (mailto: mpt-fusionlinux@avagotech.com)
> >> + * Copyright (C) 2015 Raptor Engineering
> >> + * (mailto: supp...@araptorengineeringinc.com)
> >> *
> >> * This program is free software; you can redistribute it and/or
> >> * modify it under the terms of the GNU General Public License
> >> @@ -1582,6 +1584,13 @@ mpt2sas_base_map_resources(struct
> >> MPT2SAS_ADAPTER
> >> *ioc)
> >> }
> >> }
> >>
> >> + if (ioc->chip == NULL) {
> >> + printk(MPT2SAS_ERR_FMT "unable to map "
> >> + "adapter memory (resource not found)!\n", ioc->name);
> >> + r = -EINVAL;
> >> + goto out_fail;
> >> + }
> >> +
> >> _base_mask_interrupts(ioc);
> >>
> >> r = _base_get_ioc_facts(ioc, CAN_SLEEP);
> >
> > Just following up on this patch as I have not yet received any
> > response.
> >
> > Thanks!
> 
>  Hi Tim -- just curious, why was the similar check on ioc->chip just a
>  few lines above the one added by the patch insufficient?
> 
>  That loop block sets memap_sz when it finds an IORESOURCE_MEM so that it
>  only sets ioc->chip once. I wonder if the fix might be simpler if the
>  existing ioc->chip check relocated entirely to where you put it (maybe
>  also pulling the entire error text onto one line for easier grepping).
> 
>  Regards,
> 
>  -- Joe
> >>>
> >>> If there are no IORESOURCE_MEM resources allocated by the BIOS (i.e. if
> >>> the BIOS does not run resource allocation on the mpt2sas device) then
> >>> the check you are referring to is not executed, and the driver attempts
> >>> to perform operations on a null ioc->chip pointer.
> >>>
> >>> I can relocate the check if desired.
> >>>
> >>
> >> On looking more closely at the code I think having the two separate
> >> checks is useful for debugging.  The first error message is triggered if
> >> the resource exists but Linux cannot map it for some reason, and the
> >> second error message is triggered if the resource does not exist at all
> >> (buggy BIOS).
> >
> > Fair enough.  The two error messages are unique, so grepping will lead
> > to the correct check.
> >
> > Only other nits would be the addition of a copyright (up to Avago I
> > guess) and an mpt3sas version (these drivers are 99% the same code).
> >
> > Acked-by: Joe Lawrence 
> >
> > -- Joe
> 
> 
> 



--
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] mpt2sas: Abort initialization if no memory I/O resources, detected

[Sreekanth Reddy]

Hi,

Upstream contributor should not add copyright to this driver code.

Regards,
Sreekanth

On Tue, Jun 23, 2015 at 9:24 AM, Joe Lawrence  wrote:
>
>
> On 06/21/2015 02:46 PM, Timothy Pearson wrote:
>> On 06/16/2015 01:49 PM, Timothy Pearson wrote:
>>> On 06/16/2015 12:42 PM, Joe Lawrence wrote:
 On 06/16/2015 12:28 PM, Timothy Pearson wrote:
> On 06/12/2015 05:05 PM, Timothy Pearson wrote:
>> The mpt2sas driver crashes if the BIOS does not set up at least one
>> memory I/O resource. This failure can happen if the device is too
>> slow to respond during POST and is missed by the BIOS, but Linux
>> then detects the device later in the boot process.
>>
>> This patch aborts initialization and prints a warning if no memory I/O
>> resources are found.
>>
>> Signed-off-by: Timothy Pearson
>> Tested-by: Timothy Pearson
>> ---
>> drivers/scsi/mpt2sas/mpt2sas_base.c | 9 +
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c
>> b/drivers/scsi/mpt2sas/mpt2sas_base.c
>> index 11248de..15c9504 100644
>> --- a/drivers/scsi/mpt2sas/mpt2sas_base.c
>> +++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
>> @@ -6,6 +6,8 @@
>> * Copyright (C) 2007-2014 LSI Corporation
>> * Copyright (C) 20013-2014 Avago Technologies
>> * (mailto: mpt-fusionlinux@avagotech.com)
>> + * Copyright (C) 2015 Raptor Engineering
>> + * (mailto: supp...@araptorengineeringinc.com)
>> *
>> * This program is free software; you can redistribute it and/or
>> * modify it under the terms of the GNU General Public License
>> @@ -1582,6 +1584,13 @@ mpt2sas_base_map_resources(struct
>> MPT2SAS_ADAPTER
>> *ioc)
>> }
>> }
>>
>> + if (ioc->chip == NULL) {
>> + printk(MPT2SAS_ERR_FMT "unable to map "
>> + "adapter memory (resource not found)!\n", ioc->name);
>> + r = -EINVAL;
>> + goto out_fail;
>> + }
>> +
>> _base_mask_interrupts(ioc);
>>
>> r = _base_get_ioc_facts(ioc, CAN_SLEEP);
>
> Just following up on this patch as I have not yet received any
> response.
>
> Thanks!

 Hi Tim -- just curious, why was the similar check on ioc->chip just a
 few lines above the one added by the patch insufficient?

 That loop block sets memap_sz when it finds an IORESOURCE_MEM so that it
 only sets ioc->chip once. I wonder if the fix might be simpler if the
 existing ioc->chip check relocated entirely to where you put it (maybe
 also pulling the entire error text onto one line for easier grepping).

 Regards,

 -- Joe
>>>
>>> If there are no IORESOURCE_MEM resources allocated by the BIOS (i.e. if
>>> the BIOS does not run resource allocation on the mpt2sas device) then
>>> the check you are referring to is not executed, and the driver attempts
>>> to perform operations on a null ioc->chip pointer.
>>>
>>> I can relocate the check if desired.
>>>
>>
>> On looking more closely at the code I think having the two separate
>> checks is useful for debugging.  The first error message is triggered if
>> the resource exists but Linux cannot map it for some reason, and the
>> second error message is triggered if the resource does not exist at all
>> (buggy BIOS).
>
> Fair enough.  The two error messages are unique, so grepping will lead
> to the correct check.
>
> Only other nits would be the addition of a copyright (up to Avago I
> guess) and an mpt3sas version (these drivers are 99% the same code).
>
> Acked-by: Joe Lawrence 
>
> -- Joe



-- 

Regards,
Sreekanth
--
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] mpt2sas: Abort initialization if no memory I/O resources, detected

[Joe Lawrence]

On 06/21/2015 02:46 PM, Timothy Pearson wrote:
> On 06/16/2015 01:49 PM, Timothy Pearson wrote:
>> On 06/16/2015 12:42 PM, Joe Lawrence wrote:
>>> On 06/16/2015 12:28 PM, Timothy Pearson wrote:
 On 06/12/2015 05:05 PM, Timothy Pearson wrote:
> The mpt2sas driver crashes if the BIOS does not set up at least one
> memory I/O resource. This failure can happen if the device is too
> slow to respond during POST and is missed by the BIOS, but Linux
> then detects the device later in the boot process.
>
> This patch aborts initialization and prints a warning if no memory I/O
> resources are found.
>
> Signed-off-by: Timothy Pearson
> Tested-by: Timothy Pearson
> ---
> drivers/scsi/mpt2sas/mpt2sas_base.c | 9 +
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c
> b/drivers/scsi/mpt2sas/mpt2sas_base.c
> index 11248de..15c9504 100644
> --- a/drivers/scsi/mpt2sas/mpt2sas_base.c
> +++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
> @@ -6,6 +6,8 @@
> * Copyright (C) 2007-2014 LSI Corporation
> * Copyright (C) 20013-2014 Avago Technologies
> * (mailto: mpt-fusionlinux@avagotech.com)
> + * Copyright (C) 2015 Raptor Engineering
> + * (mailto: supp...@araptorengineeringinc.com)
> *
> * This program is free software; you can redistribute it and/or
> * modify it under the terms of the GNU General Public License
> @@ -1582,6 +1584,13 @@ mpt2sas_base_map_resources(struct
> MPT2SAS_ADAPTER
> *ioc)
> }
> }
>
> + if (ioc->chip == NULL) {
> + printk(MPT2SAS_ERR_FMT "unable to map "
> + "adapter memory (resource not found)!\n", ioc->name);
> + r = -EINVAL;
> + goto out_fail;
> + }
> +
> _base_mask_interrupts(ioc);
>
> r = _base_get_ioc_facts(ioc, CAN_SLEEP);

 Just following up on this patch as I have not yet received any
 response.

 Thanks!
>>>
>>> Hi Tim -- just curious, why was the similar check on ioc->chip just a
>>> few lines above the one added by the patch insufficient?
>>>
>>> That loop block sets memap_sz when it finds an IORESOURCE_MEM so that it
>>> only sets ioc->chip once. I wonder if the fix might be simpler if the
>>> existing ioc->chip check relocated entirely to where you put it (maybe
>>> also pulling the entire error text onto one line for easier grepping).
>>>
>>> Regards,
>>>
>>> -- Joe
>>
>> If there are no IORESOURCE_MEM resources allocated by the BIOS (i.e. if
>> the BIOS does not run resource allocation on the mpt2sas device) then
>> the check you are referring to is not executed, and the driver attempts
>> to perform operations on a null ioc->chip pointer.
>>
>> I can relocate the check if desired.
>>
> 
> On looking more closely at the code I think having the two separate
> checks is useful for debugging.  The first error message is triggered if
> the resource exists but Linux cannot map it for some reason, and the
> second error message is triggered if the resource does not exist at all
> (buggy BIOS).

Fair enough.  The two error messages are unique, so grepping will lead
to the correct check.

Only other nits would be the addition of a copyright (up to Avago I
guess) and an mpt3sas version (these drivers are 99% the same code).

Acked-by: Joe Lawrence 

-- Joe
--
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 0/8] tcm_loop updates

[Hannes Reinecke]

On 06/23/2015 10:29 AM, Nicholas A. Bellinger wrote:
> On Fri, 2015-06-19 at 09:13 +0200, Hannes Reinecke wrote:
>> On 06/19/2015 08:48 AM, Christoph Hellwig wrote:
>>> What's the benefit of the SAS transport class writeout?  I honestly
>>> always saw tcm_loop as a simple loopback driver, with the different
>>> transport IDs in the PR code as a gimmick.  Note that vhost and
>>> xen-blkback copies that style and I did plan to consolidate it
>>> in common code.
>>>
>> The benefit is that tcm_loop will show up in the system as a 'real'
>> SAS hba; long-term goal is to simulate SAS multipathing here.
>> I was even planning on adding simlated FC infrastructure, too;
>> with that we could simulate FC multipathing, too, and our QA would
>> be _so_ happy...
>>
> 
> Sounds like a reasonable use-case to support for loopback testing.
> 
>> Again, these patches are mainly a collection of patches I've done to
>> test various scenarios, in the hope others might find them useful,
>> too. So I can easily hold off these patches until you've posted your
>> rework.
>>
> 
> How different do you expect sas, fc, and iscsi transports to be..?
> 
> Do you think this would this be better served by a simple tcm_loop LLD
> specific API for different multipath transports..?
> 
Actually, I would split off the various transport functions into
separate files (tcm_loop_sas, tcm_loop_fc, etc), but keep a common
tcm_loop module.
We can even make transport classes optional by adding an explicit
'sas.XXX' prefix scanning when creating the device similar to what
we do with the 'fc.XXX' prefix already.
With that we would have a 'sas.XXX', 'fc.XXX', and 'iqn.XXX' WWN
which would attach to the respective transport class, and any other
WWN (which would be the default) would be getting the standard
emulation without any transport class attached.

Cheers,

Hannes
-- 
Dr. Hannes ReineckezSeries & Storage
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

[PATCH] libiscsi: Fix host busy blocking during connection teardown

[John Soni Jose]

 Issue:
 In case of hw iscsi offload, an host can have N-number of active
 connections. There can be IO's running on some connections which
 make host->host_busy always TRUE. Now if logout from a connection
 is tried then the code gets into an infinite loop as host->host_busy
 is always TRUE.

 iscsi_conn_teardown()
 {
   .
/*
 * Block until all in-progress commands for this connection
 * time out or fail.
 */
 for (;;) {
  spin_lock_irqsave(session->host->host_lock, flags);
  if (!atomic_read(&session->host->host_busy)) { /* OK for ERL == 0 */
  spin_unlock_irqrestore(session->host->host_lock, flags);
  break;
  }
 spin_unlock_irqrestore(session->host->host_lock, flags);
 msleep_interruptible(500);
 iscsi_conn_printk(KERN_INFO, conn, "iscsi conn_destroy(): "
 "host_busy %d host_failed %d\n",
  atomic_read(&session->host->host_busy),
  session->host->host_failed);


...
 }
  }
  This is not an issue with software-iscsi/iser as each cxn is a separate
  host.

 Fix:
 Acquiring eh_mutex in iscsi_conn_teardown() before setting
 session->state = ISCSI_STATE_TERMINATE.

Signed-off-by: John Soni Jose 
---
 drivers/scsi/libiscsi.c | 25 ++---
 1 file changed, 2 insertions(+), 23 deletions(-)

diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 8053f24..98d9bb6 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -2941,10 +2941,10 @@ void iscsi_conn_teardown(struct iscsi_cls_conn 
*cls_conn)
 {
struct iscsi_conn *conn = cls_conn->dd_data;
struct iscsi_session *session = conn->session;
-   unsigned long flags;
 
del_timer_sync(&conn->transport_timer);
 
+   mutex_lock(&session->eh_mutex);
spin_lock_bh(&session->frwd_lock);
conn->c_stage = ISCSI_CONN_CLEANUP_WAIT;
if (session->leadconn == conn) {
@@ -2956,28 +2956,6 @@ void iscsi_conn_teardown(struct iscsi_cls_conn *cls_conn)
}
spin_unlock_bh(&session->frwd_lock);
 
-   /*
-* Block until all in-progress commands for this connection
-* time out or fail.
-*/
-   for (;;) {
-   spin_lock_irqsave(session->host->host_lock, flags);
-   if (!atomic_read(&session->host->host_busy)) { /* OK for ERL == 
0 */
-   spin_unlock_irqrestore(session->host->host_lock, flags);
-   break;
-   }
-   spin_unlock_irqrestore(session->host->host_lock, flags);
-   msleep_interruptible(500);
-   iscsi_conn_printk(KERN_INFO, conn, "iscsi conn_destroy(): "
- "host_busy %d host_failed %d\n",
- atomic_read(&session->host->host_busy),
- session->host->host_failed);
-   /*
-* force eh_abort() to unblock
-*/
-   wake_up(&conn->ehwait);
-   }
-
/* flush queued up work because we free the connection below */
iscsi_suspend_tx(conn);
 
@@ -2994,6 +2972,7 @@ void iscsi_conn_teardown(struct iscsi_cls_conn *cls_conn)
if (session->leadconn == conn)
session->leadconn = NULL;
spin_unlock_bh(&session->frwd_lock);
+   mutex_unlock(&session->eh_mutex);
 
iscsi_destroy_conn(cls_conn);
 }
-- 
1.8.3.1

--
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 0/8] tcm_loop updates

[Nicholas A. Bellinger]

On Fri, 2015-06-19 at 09:13 +0200, Hannes Reinecke wrote:
> On 06/19/2015 08:48 AM, Christoph Hellwig wrote:
> > What's the benefit of the SAS transport class writeout?  I honestly
> > always saw tcm_loop as a simple loopback driver, with the different
> > transport IDs in the PR code as a gimmick.  Note that vhost and
> > xen-blkback copies that style and I did plan to consolidate it
> > in common code.
> > 
> The benefit is that tcm_loop will show up in the system as a 'real'
> SAS hba; long-term goal is to simulate SAS multipathing here.
> I was even planning on adding simlated FC infrastructure, too;
> with that we could simulate FC multipathing, too, and our QA would
> be _so_ happy...
> 

Sounds like a reasonable use-case to support for loopback testing.

> Again, these patches are mainly a collection of patches I've done to
> test various scenarios, in the hope others might find them useful,
> too. So I can easily hold off these patches until you've posted your
> rework.
> 

How different do you expect sas, fc, and iscsi transports to be..?

Do you think this would this be better served by a simple tcm_loop LLD
specific API for different multipath transports..?

--
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] st: convert to using driver attr groups for sysfs

[Seymour, Shane M]

This patch changes the st driver to use attribute groups so
driver sysfs files are created automatically. See the
following for reference:

http://kroah.com/log/blog/2013/06/26/how-to-create-a-sysfs-file-correctly/

Signed-off-by: Shane Seymour 
---
--- a/drivers/scsi/st.c 2015-06-22 14:20:40.829612661 -0500
+++ b/drivers/scsi/st.c 2015-06-22 15:49:49.357248393 -0500
@@ -85,6 +85,7 @@ static int debug_flag;
 
 static struct class st_sysfs_class;
 static const struct attribute_group *st_dev_groups[];
+static const struct attribute_group *st_drv_groups[];
 
 MODULE_AUTHOR("Kai Makisara");
 MODULE_DESCRIPTION("SCSI tape (st) driver");
@@ -198,15 +199,13 @@ static int sgl_unmap_user_pages(struct s
 static int st_probe(struct device *);
 static int st_remove(struct device *);
 
-static int do_create_sysfs_files(void);
-static void do_remove_sysfs_files(void);
-
 static struct scsi_driver st_template = {
.gendrv = {
.name   = "st",
.owner  = THIS_MODULE,
.probe  = st_probe,
.remove = st_remove,
+   .groups = st_drv_groups,
},
 };
 
@@ -4404,14 +4403,8 @@ static int __init init_st(void)
if (err)
goto err_chrdev;
 
-   err = do_create_sysfs_files();
-   if (err)
-   goto err_scsidrv;
-
return 0;
 
-err_scsidrv:
-   scsi_unregister_driver(&st_template.gendrv);
 err_chrdev:
unregister_chrdev_region(MKDEV(SCSI_TAPE_MAJOR, 0),
 ST_MAX_TAPE_ENTRIES);
@@ -4422,7 +4415,6 @@ err_class:
 
 static void __exit exit_st(void)
 {
-   do_remove_sysfs_files();
scsi_unregister_driver(&st_template.gendrv);
unregister_chrdev_region(MKDEV(SCSI_TAPE_MAJOR, 0),
 ST_MAX_TAPE_ENTRIES);
@@ -4459,44 +4451,14 @@ static ssize_t st_version_show(struct de
 }
 static DRIVER_ATTR(version, S_IRUGO, st_version_show, NULL);
 
-static int do_create_sysfs_files(void)
-{
-   struct device_driver *sysfs = &st_template.gendrv;
-   int err;
-
-   err = driver_create_file(sysfs, &driver_attr_try_direct_io);
-   if (err)
-   return err;
-   err = driver_create_file(sysfs, &driver_attr_fixed_buffer_size);
-   if (err)
-   goto err_try_direct_io;
-   err = driver_create_file(sysfs, &driver_attr_max_sg_segs);
-   if (err)
-   goto err_attr_fixed_buf;
-   err = driver_create_file(sysfs, &driver_attr_version);
-   if (err)
-   goto err_attr_max_sg;
-
-   return 0;
-
-err_attr_max_sg:
-   driver_remove_file(sysfs, &driver_attr_max_sg_segs);
-err_attr_fixed_buf:
-   driver_remove_file(sysfs, &driver_attr_fixed_buffer_size);
-err_try_direct_io:
-   driver_remove_file(sysfs, &driver_attr_try_direct_io);
-   return err;
-}
-
-static void do_remove_sysfs_files(void)
-{
-   struct device_driver *sysfs = &st_template.gendrv;
-
-   driver_remove_file(sysfs, &driver_attr_version);
-   driver_remove_file(sysfs, &driver_attr_max_sg_segs);
-   driver_remove_file(sysfs, &driver_attr_fixed_buffer_size);
-   driver_remove_file(sysfs, &driver_attr_try_direct_io);
-}
+static struct attribute *st_drv_attrs[] = {
+   &driver_attr_try_direct_io.attr,
+   &driver_attr_fixed_buffer_size.attr,
+   &driver_attr_max_sg_segs.attr,
+   &driver_attr_version.attr,
+   NULL,
+};
+ATTRIBUTE_GROUPS(st_drv);
 
 /* The sysfs simple class interface */
 static ssize_t
--
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 6/6] target: Send UA when changing LUN inventory

[Nicholas A. Bellinger]

On Fri, 2015-06-19 at 15:07 +0200, Christoph Hellwig wrote:
> > +   hlist_for_each_entry_rcu(tmp, &nacl->lun_entry_hlist, link) {
> > +   if (tmp == new)
> > +   continue;
> > +   core_scsi3_ua_allocate(tmp, 0x3F,
> > +   ASCQ_3FH_REPORTED_LUNS_DATA_HAS_CHANGED);
> > +   }
> > +   rcu_read_unlock();
> > +
> 
> > +   rcu_read_lock();
> > +   hlist_for_each_entry_rcu(tmp, &nacl->lun_entry_hlist, link) {
> > +   if (tmp == new)
> > +   continue;
> > +   core_scsi3_ua_allocate(tmp, 0x3F,
> > +   ASCQ_3FH_REPORTED_LUNS_DATA_HAS_CHANGED);
> > +   }
> > +   rcu_read_unlock();
> 
> > +
> > +   rcu_read_lock();
> > +   hlist_for_each_entry_rcu(tmp, &nacl->lun_entry_hlist, link)
> > +   core_scsi3_ua_allocate(tmp, 0x3F,
> > +   ASCQ_3FH_REPORTED_LUNS_DATA_HAS_CHANGED);
> > +   rcu_read_unlock();
> 
> Please add a helper instead of duplicating this three times.



Applying the following squashed commit:

>From 7c0d0d51d26497866d2951a35f1736fc765e4fcf Mon Sep 17 00:00:00 2001
From: Hannes Reinecke 
Date: Thu, 11 Jun 2015 10:01:29 +0200
Subject: [PATCH 68/76] target: Send UA when changing LUN inventory

When changind the LUN inventory via core_enable_device_list_for_node()
or core_disable_device_list_for_node() a REPORTED LUNS DATA HAS CHANGED
UA should be send.

(Convert to target_luns_data_has_changed helper usage - hch)

Signed-off-by: Hannes Reinecke 
Signed-off-by: Nicholas Bellinger 
---
 drivers/target/target_core_device.c | 23 +++
 drivers/target/target_core_ua.h |  1 +
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/target/target_core_device.c 
b/drivers/target/target_core_device.c
index b6df5b9..5244848 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -293,10 +293,22 @@ void target_pr_kref_release(struct kref *kref)
complete(&deve->pr_comp);
 }
 
-/*  core_enable_device_list_for_node():
- *
- *
- */
+static void
+target_luns_data_has_changed(struct se_node_acl *nacl, struct se_dev_entry 
*new,
+bool skip_new)
+{
+   struct se_dev_entry *tmp;
+
+   rcu_read_lock();
+   hlist_for_each_entry_rcu(tmp, &nacl->lun_entry_hlist, link) {
+   if (skip_new && tmp == new)
+   continue;
+   core_scsi3_ua_allocate(tmp, 0x3F,
+  ASCQ_3FH_REPORTED_LUNS_DATA_HAS_CHANGED);
+   }
+   rcu_read_unlock();
+}
+
 int core_enable_device_list_for_node(
struct se_lun *lun,
struct se_lun_acl *lun_acl,
@@ -360,6 +372,7 @@ int core_enable_device_list_for_node(
kref_put(&orig->pr_kref, target_pr_kref_release);
wait_for_completion(&orig->pr_comp);
 
+   target_luns_data_has_changed(nacl, new, true);
kfree_rcu(orig, rcu_head);
return 0;
}
@@ -373,6 +386,7 @@ int core_enable_device_list_for_node(
list_add_tail(&new->lun_link, &lun->lun_deve_list);
spin_unlock(&lun->lun_deve_lock);
 
+   target_luns_data_has_changed(nacl, new, true);
return 0;
 }
 
@@ -428,6 +442,7 @@ void core_disable_device_list_for_node(
kfree_rcu(orig, rcu_head);
 
core_scsi3_free_pr_reg_from_nacl(dev, nacl);
+   target_luns_data_has_changed(nacl, NULL, false);
 }
 
 /*  core_clear_lun_from_tpg():
diff --git a/drivers/target/target_core_ua.h b/drivers/target/target_core_ua.h
index 59278d6..bd6e78b 100644
--- a/drivers/target/target_core_ua.h
+++ b/drivers/target/target_core_ua.h
@@ -26,6 +26,7 @@
 #define ASCQ_2CH_PREVIOUS_RESERVATION_CONFLICT_STATUS  0x09
 
 #define ASCQ_3FH_INQUIRY_DATA_HAS_CHANGED  0x03
+#define ASCQ_3FH_REPORTED_LUNS_DATA_HAS_CHANGED0x0E
 
 extern struct kmem_cache *se_ua_cache;
 
-- 
1.9.1

--
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 5/6] target: Send UA upon LUN RESET tmr completion

[Nicholas A. Bellinger]

On Fri, 2015-06-19 at 15:06 +0200, Christoph Hellwig wrote:
> On Thu, Jun 11, 2015 at 10:01:28AM +0200, Hannes Reinecke wrote:
> > SAM mandates that an BUS DEVICE RESET FUNCTION OCCURRED
> > UA needs to be send after a LUN RESET tmr has completed.
> > 
> > Signed-off-by: Hannes Reinecke 
> > ---
> >  drivers/target/target_core_transport.c | 11 +++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/drivers/target/target_core_transport.c 
> > b/drivers/target/target_core_transport.c
> > index a0e0d3a..bb60c0c4 100644
> > --- a/drivers/target/target_core_transport.c
> > +++ b/drivers/target/target_core_transport.c
> > @@ -3064,6 +3064,17 @@ static void target_tmr_work(struct work_struct *work)
> > ret = core_tmr_lun_reset(dev, tmr, NULL, NULL);
> > tmr->response = (!ret) ? TMR_FUNCTION_COMPLETE :
> >  TMR_FUNCTION_REJECTED;
> > +   if (tmr->response == TMR_FUNCTION_COMPLETE) {
> > +   struct se_dev_entry *deve;
> > +
> > +   rcu_read_lock();
> > +   deve = target_nacl_find_deve(cmd->se_sess->se_node_acl,
> > +cmd->orig_fe_lun);
> > +   if (deve)
> > +   core_scsi3_ua_allocate(deve, 0x29,
> > +   
> > ASCQ_29H_BUS_DEVICE_RESET_FUNCTION_OCCURRED);
> > +   rcu_read_unlock();
> 
> This should use the target_ua_allocate_lun helper.

Fixed.

--
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 4/6] target: Send UA on ALUA target port group change

[Nicholas A. Bellinger]

On Fri, 2015-06-19 at 15:05 +0200, Christoph Hellwig wrote:
> > --- a/drivers/target/target_core_alua.c
> > +++ b/drivers/target/target_core_alua.c
> > @@ -1880,12 +1880,19 @@ static void core_alua_put_tg_pt_gp_from_name(
> >  static void __target_attach_tg_pt_gp(struct se_lun *lun,
> > struct t10_alua_tg_pt_gp *tg_pt_gp)
> >  {
> > +   struct se_dev_entry *se_deve;
> > +
> > assert_spin_locked(&lun->lun_tg_pt_gp_lock);
> >  
> > spin_lock(&tg_pt_gp->tg_pt_gp_lock);
> > lun->lun_tg_pt_gp = tg_pt_gp;
> > list_add_tail(&lun->lun_tg_pt_gp_link, &tg_pt_gp->tg_pt_gp_lun_list);
> > tg_pt_gp->tg_pt_gp_members++;
> > +   spin_lock_bh(&lun->lun_deve_lock);
> > +   list_for_each_entry(se_deve, &lun->lun_deve_list, lun_link)
> > +   core_scsi3_ua_allocate(se_deve, 0x3f,
> > +  ASCQ_3FH_INQUIRY_DATA_HAS_CHANGED);
> > +   spin_unlock_bh(&lun->lun_deve_lock);
> > spin_unlock(&tg_pt_gp->tg_pt_gp_lock);
> 
> Taking a _bh lock inside a regular spinlock is completely broken.
> 

...

> Fortunately I don't think lun_deve_lock needs to disable bottom halves,
> but this needs to be fixed first.



Applying the following + updating this original patch to use normal
spinlock_t access.

>From 1adff1b3a7f75a1c255b7fcab5676edf29d4a5d8 Mon Sep 17 00:00:00 2001
From: Nicholas Bellinger 
Date: Mon, 22 Jun 2015 23:44:05 -0700
Subject: [PATCH 65/76] target: Convert se_lun->lun_deve_lock to normal
 spinlock

This patch converts se_lun->lun_deve_lock acquire/release access
to use a normal, non bottom-half spin_lock_t for protecting
se_lun->lun_deve_list access.

Reported-by: Christoph Hellwig 
Cc: Hannes Reinecke 
Signed-off-by: Nicholas Bellinger 
---
 drivers/target/target_core_alua.c   |  4 ++--
 drivers/target/target_core_device.c | 12 ++--
 drivers/target/target_core_pr.c |  8 
 3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/target/target_core_alua.c 
b/drivers/target/target_core_alua.c
index aa2e4b1..c56ae02 100644
--- a/drivers/target/target_core_alua.c
+++ b/drivers/target/target_core_alua.c
@@ -968,7 +968,7 @@ static void core_alua_queue_state_change_ua(struct 
t10_alua_tg_pt_gp *tg_pt_gp)
continue;
spin_unlock(&tg_pt_gp->tg_pt_gp_lock);
 
-   spin_lock_bh(&lun->lun_deve_lock);
+   spin_lock(&lun->lun_deve_lock);
list_for_each_entry(se_deve, &lun->lun_deve_list, lun_link) {
lacl = rcu_dereference_check(se_deve->se_lun_acl,
lockdep_is_held(&lun->lun_deve_lock));
@@ -1000,7 +1000,7 @@ static void core_alua_queue_state_change_ua(struct 
t10_alua_tg_pt_gp *tg_pt_gp)
core_scsi3_ua_allocate(se_deve, 0x2A,
ASCQ_2AH_ASYMMETRIC_ACCESS_STATE_CHANGED);
}
-   spin_unlock_bh(&lun->lun_deve_lock);
+   spin_unlock(&lun->lun_deve_lock);
 
spin_lock(&tg_pt_gp->tg_pt_gp_lock);
percpu_ref_put(&lun->lun_ref);
diff --git a/drivers/target/target_core_device.c 
b/drivers/target/target_core_device.c
index ed08402..b6df5b9 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -352,10 +352,10 @@ int core_enable_device_list_for_node(
hlist_add_head_rcu(&new->link, &nacl->lun_entry_hlist);
mutex_unlock(&nacl->lun_entry_mutex);
 
-   spin_lock_bh(&lun->lun_deve_lock);
+   spin_lock(&lun->lun_deve_lock);
list_del(&orig->lun_link);
list_add_tail(&new->lun_link, &lun->lun_deve_list);
-   spin_unlock_bh(&lun->lun_deve_lock);
+   spin_unlock(&lun->lun_deve_lock);
 
kref_put(&orig->pr_kref, target_pr_kref_release);
wait_for_completion(&orig->pr_comp);
@@ -369,9 +369,9 @@ int core_enable_device_list_for_node(
hlist_add_head_rcu(&new->link, &nacl->lun_entry_hlist);
mutex_unlock(&nacl->lun_entry_mutex);
 
-   spin_lock_bh(&lun->lun_deve_lock);
+   spin_lock(&lun->lun_deve_lock);
list_add_tail(&new->lun_link, &lun->lun_deve_list);
-   spin_unlock_bh(&lun->lun_deve_lock);
+   spin_unlock(&lun->lun_deve_lock);
 
return 0;
 }
@@ -403,9 +403,9 @@ void core_disable_device_list_for_node(
 * NodeACL context specific PR metadata for demo-mode
 * MappedLUN *deve will be released below..
 */
-   spin_lock_bh(&lun->lun_deve_lock);
+   spin_lock(&lun->lun_deve_lock);
list_del(&orig->lun_link);
-   spin_unlock_bh(&lun->lun_deve_lock);
+   spin_unlock(&lun->lun_deve_lock);
/*
 * Disable struct se_dev_entry LUN ACL mapping
 */
diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c
index 0bb3292..7403b03 100644
--- a/drivers/target/target_core_pr.c
+++ b/drivers/target/target