[PATCH] lib: scatterlist: fix ifnullfree.cocci warnings

2016-03-22 Thread kbuild test robot
lib/sg_pool.c:152:3-18: WARNING: NULL check before freeing functions like 
kfree, debugfs_remove, debugfs_remove_recursive or usb_free_urb is not needed. 
Maybe consider reorganizing relevant code to avoid passing NULL values.
lib/sg_pool.c:154:3-21: WARNING: NULL check before freeing functions like 
kfree, debugfs_remove, debugfs_remove_recursive or usb_free_urb is not needed. 
Maybe consider reorganizing relevant code to avoid passing NULL values.

 NULL check before some freeing functions is not needed.

 Based on checkpatch warning
 "kfree(NULL) is safe this check is probably not required"
 and kfreeaddr.cocci by Julia Lawall.

Generated by: scripts/coccinelle/free/ifnullfree.cocci

CC: Ming Lin 
Signed-off-by: Fengguang Wu 
---

 sg_pool.c |6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

--- a/lib/sg_pool.c
+++ b/lib/sg_pool.c
@@ -148,10 +148,8 @@ static __init int sg_pool_init(void)
 cleanup_sdb:
for (i = 0; i < SG_MEMPOOL_NR; i++) {
struct sg_pool *sgp = sg_pools + i;
-   if (sgp->pool)
-   mempool_destroy(sgp->pool);
-   if (sgp->slab)
-   kmem_cache_destroy(sgp->slab);
+   mempool_destroy(sgp->pool);
+   kmem_cache_destroy(sgp->slab);
}
 
return -ENOMEM;
--
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 5/5] lib: scatterlist: move SG pool code from SCSI driver to lib/sg_pool.c

2016-03-22 Thread kbuild test robot
Hi Ming,

[auto build test WARNING on scsi/for-next]
[also build test WARNING on v4.5 next-20160322]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improving the system]

url:
https://github.com/0day-ci/linux/commits/Ming-Lin/mempool-based-chained-scatterlist-alloc-free-api/20160323-060710
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next


coccinelle warnings: (new ones prefixed by >>)

>> lib/sg_pool.c:152:3-18: WARNING: NULL check before freeing functions like 
>> kfree, debugfs_remove, debugfs_remove_recursive or usb_free_urb is not 
>> needed. Maybe consider reorganizing relevant code to avoid passing NULL 
>> values.
   lib/sg_pool.c:154:3-21: WARNING: NULL check before freeing functions like 
kfree, debugfs_remove, debugfs_remove_recursive or usb_free_urb is not needed. 
Maybe consider reorganizing relevant code to avoid passing NULL values.

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation
--
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 v3 00/23] ncr5380: Eliminate macros, reduce code duplication, fix bugs etc

2016-03-22 Thread Finn Thain

On Tue, 22 Mar 2016, Ondrej Zary wrote:

> Tested on HP C2502 (53C400A chip), Canon FG2-5202 (53C400 chip) and 
> DTC-3181L (DTCT-436P chip) ISA cards - everything works fine!
> 
> Thanks.

Thanks for testing, Ondrej.

> 
> Tested-by: Ondrej Zary 

Hannes, would you please take a look at patches 04/23, 14/23, 20/23 and 
23/23 when convenient? These four patches need a "reviewed-by" tag.

-- 
--
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 v3 00/23] ncr5380: Eliminate macros, reduce code duplication, fix bugs etc

2016-03-22 Thread Ondrej Zary
On Monday 21 March 2016 03:31:49 Finn Thain wrote:
> 
> This patch series has more macro elimination and some tweaks to the
> DMA hooks so that all the wrapper drivers can share the same core
> DMA algorithm. This resolves the major discrepancies between the two
> core drivers, which relate to code conditional on the REAL_DMA and
> PSEUDO_DMA macros.
> 
> After all the wrapper drivers agree on the DMA hook api, the core driver
> fork gets resolved. NCR5380.c is adopted by atari_scsi and sun3_scsi and
> atari_NCR5380.c is then deleted.
> 
> Historically, the 5380 drivers suffered from over-use of conditional
> compilation, which caused the compile-time configuration space to explode,
> leading to core driver code that was practically untestable, unmaintainable
> and difficult to reason about. It also prevented driver modules from
> sharing object code.
> 
> Along with REAL_DMA, REAL_DMA_POLL and PSEUDO_DMA, most of the remaining
> macros are also eradicated, such as CONFIG_SCSI_GENERIC_NCR53C400,
> SUPPORT_TAGS, DONT_USE_INTR, AUTOPROBE_IRQ and BIOSPARAM.
> 
> Also in this patch series, some duplicated documentation is removed and
> the PDMA implementation in mac_scsi finally gets fixed.
> 
> This patch series was tested by exercising the dmx3191d and mac_scsi modules
> on suitable hardware. Michael has tested atari_scsi on an Atari Falcon.
> Help with driver testing on ISA cards is sought as I don't have such
> hardware. Likewise RiscPC ecards and Sun 3.

Tested on HP C2502 (53C400A chip), Canon FG2-5202 (53C400 chip) and DTC-3181L
(DTCT-436P chip) ISA cards - everything works fine!

Thanks.

Tested-by: Ondrej Zary 

HP C2502:
scsi host2: Generic NCR5380/NCR53C400 SCSI, io_port 0x280, n_io_port 16, base 
0x0, irq 0, can_queue 16, cmd_per_lun 2, sg_tablesize 128, this_id 7, flags { 
}, options { }

Canon FG2-5202:
scsi host2: Generic NCR5380/NCR53C400 SCSI, io_port 0x0, n_io_port 0, base 
0xd8000, irq 0, can_queue 16, cmd_per_lun 2, sg_tablesize 128, this_id 7, flags 
{ }, options { }

DTC-3181L:
scsi host2: Generic NCR5380/NCR53C400 SCSI, io_port 0x240, n_io_port 16, base 
0x0, irq 0, can_queue 16, cmd_per_lun 2, sg_tablesize 128, this_id 7, flags { 
}, options { }

-- 
Ondrej Zary
--
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 2/5] scsi: replace "mq" with "first_chunk" in SG functions

2016-03-22 Thread Ming Lin
From: Ming Lin 

Parameter "bool mq" is block driver specific.
Change it to "first_chunk" to make it more generic.

Signed-off-by: Ming Lin 
---
 drivers/scsi/scsi_lib.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 5eaddc7..ab3dd09 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -583,33 +583,32 @@ static struct scatterlist *scsi_sg_alloc(unsigned int 
nents, gfp_t gfp_mask)
return mempool_alloc(sgp->pool, gfp_mask);
 }
 
-static void scsi_free_sgtable(struct sg_table *table, bool mq)
+static void scsi_free_sgtable(struct sg_table *table, bool first_chunk)
 {
-   if (mq && table->orig_nents <= SCSI_MAX_SG_SEGMENTS)
+   if (first_chunk && table->orig_nents <= SCSI_MAX_SG_SEGMENTS)
return;
-   __sg_free_table(table, SCSI_MAX_SG_SEGMENTS, mq, scsi_sg_free);
+   __sg_free_table(table, SCSI_MAX_SG_SEGMENTS, first_chunk, scsi_sg_free);
 }
 
-static int scsi_alloc_sgtable(struct sg_table *table, int nents, bool mq)
+static int scsi_alloc_sgtable(struct sg_table *table, int nents,
+   struct scatterlist *first_chunk)
 {
-   struct scatterlist *first_chunk = NULL;
int ret;
 
BUG_ON(!nents);
 
-   if (mq) {
+   if (first_chunk) {
if (nents <= SCSI_MAX_SG_SEGMENTS) {
table->nents = table->orig_nents = nents;
sg_init_table(table->sgl, nents);
return 0;
}
-   first_chunk = table->sgl;
}
 
ret = __sg_alloc_table(table, nents, SCSI_MAX_SG_SEGMENTS,
   first_chunk, GFP_ATOMIC, scsi_sg_alloc);
if (unlikely(ret))
-   scsi_free_sgtable(table, mq);
+   scsi_free_sgtable(table, (bool)first_chunk);
return ret;
 }
 
@@ -1091,7 +1090,7 @@ static int scsi_init_sgtable(struct request *req, struct 
scsi_data_buffer *sdb)
 * If sg table allocation fails, requeue request later.
 */
if (unlikely(scsi_alloc_sgtable(&sdb->table, req->nr_phys_segments,
-   req->mq_ctx != NULL)))
+   sdb->table.sgl)))
return BLKPREP_DEFER;
 
/* 
@@ -1163,7 +1162,8 @@ int scsi_init_io(struct scsi_cmnd *cmd)
 
ivecs = blk_rq_count_integrity_sg(rq->q, rq->bio);
 
-   if (scsi_alloc_sgtable(&prot_sdb->table, ivecs, is_mq)) {
+   if (scsi_alloc_sgtable(&prot_sdb->table, ivecs,
+   prot_sdb->table.sgl)) {
error = BLKPREP_DEFER;
goto err_exit;
}
-- 
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


[PATCH v2 0/5] mempool based chained scatterlist alloc/free api

2016-03-22 Thread Ming Lin
From: Ming Lin 

The fist 4 patches make the SG related definitions/structs/functions
in SCSI code generic and the last patch move it to lib/sg_pool.c.

I still keep the macro "SG_MEMPOOL_NR" since it's used in 3 places.

v2:
  - do modification in scsi code first then move to lib/sg_pool.c
  - address Christoph's comments

Ming Lin (5):
  scsi: replace "scsi_data_buffer" with "sg_table" in SG functions
  scsi: replace "mq" with "first_chunk" in SG functions
  scsi: rename SG related struct and functions
  scsi: rename SCSI_MAX_{SG, SG_CHAIN}_SEGMENTS
  lib: scatterlist: move SG pool code from SCSI driver to lib/sg_pool.c

 drivers/ata/pata_icside.c   |   2 +-
 drivers/infiniband/ulp/srp/ib_srp.c |   4 +-
 drivers/scsi/Kconfig|   1 +
 drivers/scsi/arm/cumana_2.c |   2 +-
 drivers/scsi/arm/eesox.c|   2 +-
 drivers/scsi/arm/powertec.c |   2 +-
 drivers/scsi/esas2r/esas2r_main.c   |   4 +-
 drivers/scsi/hisi_sas/hisi_sas.h|   2 +-
 drivers/scsi/mpt3sas/mpt3sas_base.c |   4 +-
 drivers/scsi/mpt3sas/mpt3sas_base.h |   2 +-
 drivers/scsi/scsi_debug.c   |   2 +-
 drivers/scsi/scsi_lib.c | 172 +---
 drivers/usb/storage/scsiglue.c  |   2 +-
 include/linux/scatterlist.h |  25 ++
 include/scsi/scsi.h |  19 
 include/scsi/scsi_host.h|   2 +-
 lib/Kconfig |   7 ++
 lib/Makefile|   1 +
 lib/sg_pool.c   | 172 
 19 files changed, 241 insertions(+), 186 deletions(-)
 create mode 100644 lib/sg_pool.c

-- 
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


[PATCH v2 1/5] scsi: replace "scsi_data_buffer" with "sg_table" in SG functions

2016-03-22 Thread Ming Lin
From: Ming Lin 

Replace parameter "struct scsi_data_buffer" with "struct sg_table" in
SG alloc/free functions to make them generic.

Signed-off-by: Ming Lin 
---
 drivers/scsi/scsi_lib.c | 41 +++--
 1 file changed, 23 insertions(+), 18 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 8c6e318..5eaddc7 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -583,14 +583,14 @@ static struct scatterlist *scsi_sg_alloc(unsigned int 
nents, gfp_t gfp_mask)
return mempool_alloc(sgp->pool, gfp_mask);
 }
 
-static void scsi_free_sgtable(struct scsi_data_buffer *sdb, bool mq)
+static void scsi_free_sgtable(struct sg_table *table, bool mq)
 {
-   if (mq && sdb->table.orig_nents <= SCSI_MAX_SG_SEGMENTS)
+   if (mq && table->orig_nents <= SCSI_MAX_SG_SEGMENTS)
return;
-   __sg_free_table(&sdb->table, SCSI_MAX_SG_SEGMENTS, mq, scsi_sg_free);
+   __sg_free_table(table, SCSI_MAX_SG_SEGMENTS, mq, scsi_sg_free);
 }
 
-static int scsi_alloc_sgtable(struct scsi_data_buffer *sdb, int nents, bool mq)
+static int scsi_alloc_sgtable(struct sg_table *table, int nents, bool mq)
 {
struct scatterlist *first_chunk = NULL;
int ret;
@@ -599,17 +599,17 @@ static int scsi_alloc_sgtable(struct scsi_data_buffer 
*sdb, int nents, bool mq)
 
if (mq) {
if (nents <= SCSI_MAX_SG_SEGMENTS) {
-   sdb->table.nents = sdb->table.orig_nents = nents;
-   sg_init_table(sdb->table.sgl, nents);
+   table->nents = table->orig_nents = nents;
+   sg_init_table(table->sgl, nents);
return 0;
}
-   first_chunk = sdb->table.sgl;
+   first_chunk = table->sgl;
}
 
-   ret = __sg_alloc_table(&sdb->table, nents, SCSI_MAX_SG_SEGMENTS,
+   ret = __sg_alloc_table(table, nents, SCSI_MAX_SG_SEGMENTS,
   first_chunk, GFP_ATOMIC, scsi_sg_alloc);
if (unlikely(ret))
-   scsi_free_sgtable(sdb, mq);
+   scsi_free_sgtable(table, mq);
return ret;
 }
 
@@ -625,12 +625,17 @@ static void scsi_uninit_cmd(struct scsi_cmnd *cmd)
 
 static void scsi_mq_free_sgtables(struct scsi_cmnd *cmd)
 {
+   struct scsi_data_buffer *sdb;
+
if (cmd->sdb.table.nents)
-   scsi_free_sgtable(&cmd->sdb, true);
-   if (cmd->request->next_rq && cmd->request->next_rq->special)
-   scsi_free_sgtable(cmd->request->next_rq->special, true);
+   scsi_free_sgtable(&cmd->sdb.table, true);
+   if (cmd->request->next_rq) {
+   sdb = cmd->request->next_rq->special;
+   if (sdb)
+   scsi_free_sgtable(&sdb->table, true);
+   }
if (scsi_prot_sg_count(cmd))
-   scsi_free_sgtable(cmd->prot_sdb, true);
+   scsi_free_sgtable(&cmd->prot_sdb->table, true);
 }
 
 static void scsi_mq_uninit_cmd(struct scsi_cmnd *cmd)
@@ -669,19 +674,19 @@ static void scsi_mq_uninit_cmd(struct scsi_cmnd *cmd)
 static void scsi_release_buffers(struct scsi_cmnd *cmd)
 {
if (cmd->sdb.table.nents)
-   scsi_free_sgtable(&cmd->sdb, false);
+   scsi_free_sgtable(&cmd->sdb.table, false);
 
memset(&cmd->sdb, 0, sizeof(cmd->sdb));
 
if (scsi_prot_sg_count(cmd))
-   scsi_free_sgtable(cmd->prot_sdb, false);
+   scsi_free_sgtable(&cmd->prot_sdb->table, false);
 }
 
 static void scsi_release_bidi_buffers(struct scsi_cmnd *cmd)
 {
struct scsi_data_buffer *bidi_sdb = cmd->request->next_rq->special;
 
-   scsi_free_sgtable(bidi_sdb, false);
+   scsi_free_sgtable(&bidi_sdb->table, false);
kmem_cache_free(scsi_sdb_cache, bidi_sdb);
cmd->request->next_rq->special = NULL;
 }
@@ -1085,7 +1090,7 @@ static int scsi_init_sgtable(struct request *req, struct 
scsi_data_buffer *sdb)
/*
 * If sg table allocation fails, requeue request later.
 */
-   if (unlikely(scsi_alloc_sgtable(sdb, req->nr_phys_segments,
+   if (unlikely(scsi_alloc_sgtable(&sdb->table, req->nr_phys_segments,
req->mq_ctx != NULL)))
return BLKPREP_DEFER;
 
@@ -1158,7 +1163,7 @@ int scsi_init_io(struct scsi_cmnd *cmd)
 
ivecs = blk_rq_count_integrity_sg(rq->q, rq->bio);
 
-   if (scsi_alloc_sgtable(prot_sdb, ivecs, is_mq)) {
+   if (scsi_alloc_sgtable(&prot_sdb->table, ivecs, is_mq)) {
error = BLKPREP_DEFER;
goto err_exit;
}
-- 
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


[PATCH v2 4/5] scsi: rename SCSI_MAX_{SG, SG_CHAIN}_SEGMENTS

2016-03-22 Thread Ming Lin
From: Ming Lin 

Rename SCSI_MAX_SG_SEGMENTS to SG_CHUNK_SIZE, which means the amount
we fit into a single scatterlist chunk.

Rename SCSI_MAX_SG_CHAIN_SEGMENTS to SG_MAX_SEGMENTS.

Will move these 2 generic definitions to scatterlist.h later.

Signed-off-by: Ming Lin 
---
 drivers/ata/pata_icside.c   |  2 +-
 drivers/infiniband/ulp/srp/ib_srp.c |  4 ++--
 drivers/scsi/arm/cumana_2.c |  2 +-
 drivers/scsi/arm/eesox.c|  2 +-
 drivers/scsi/arm/powertec.c |  2 +-
 drivers/scsi/esas2r/esas2r_main.c   |  4 ++--
 drivers/scsi/hisi_sas/hisi_sas.h|  2 +-
 drivers/scsi/mpt3sas/mpt3sas_base.c |  4 ++--
 drivers/scsi/mpt3sas/mpt3sas_base.h |  2 +-
 drivers/scsi/scsi_debug.c   |  2 +-
 drivers/scsi/scsi_lib.c | 34 +-
 drivers/usb/storage/scsiglue.c  |  2 +-
 include/scsi/scsi.h |  8 
 include/scsi/scsi_host.h|  2 +-
 14 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/drivers/ata/pata_icside.c b/drivers/ata/pata_icside.c
index d7c7320..188f2f2 100644
--- a/drivers/ata/pata_icside.c
+++ b/drivers/ata/pata_icside.c
@@ -294,7 +294,7 @@ static int icside_dma_init(struct pata_icside_info *info)
 
 static struct scsi_host_template pata_icside_sht = {
ATA_BASE_SHT(DRV_NAME),
-   .sg_tablesize   = SCSI_MAX_SG_CHAIN_SEGMENTS,
+   .sg_tablesize   = SG_MAX_SEGMENTS,
.dma_boundary   = IOMD_DMA_BOUNDARY,
 };
 
diff --git a/drivers/infiniband/ulp/srp/ib_srp.c 
b/drivers/infiniband/ulp/srp/ib_srp.c
index 60b169a..b428b48 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -81,7 +81,7 @@ MODULE_PARM_DESC(cmd_sg_entries,
 
 module_param(indirect_sg_entries, uint, 0444);
 MODULE_PARM_DESC(indirect_sg_entries,
-"Default max number of gather/scatter entries (default is 12, 
max is " __stringify(SCSI_MAX_SG_CHAIN_SEGMENTS) ")");
+"Default max number of gather/scatter entries (default is 12, 
max is " __stringify(SG_MAX_SEGMENTS) ")");
 
 module_param(allow_ext_sg, bool, 0444);
 MODULE_PARM_DESC(allow_ext_sg,
@@ -3129,7 +3129,7 @@ static int srp_parse_options(const char *buf, struct 
srp_target_port *target)
 
case SRP_OPT_SG_TABLESIZE:
if (match_int(args, &token) || token < 1 ||
-   token > SCSI_MAX_SG_CHAIN_SEGMENTS) {
+   token > SG_MAX_SEGMENTS) {
pr_warn("bad max sg_tablesize parameter '%s'\n",
p);
goto out;
diff --git a/drivers/scsi/arm/cumana_2.c b/drivers/scsi/arm/cumana_2.c
index faa1bee..edce5f3 100644
--- a/drivers/scsi/arm/cumana_2.c
+++ b/drivers/scsi/arm/cumana_2.c
@@ -365,7 +365,7 @@ static struct scsi_host_template cumanascsi2_template = {
.eh_abort_handler   = fas216_eh_abort,
.can_queue  = 1,
.this_id= 7,
-   .sg_tablesize   = SCSI_MAX_SG_CHAIN_SEGMENTS,
+   .sg_tablesize   = SG_MAX_SEGMENTS,
.dma_boundary   = IOMD_DMA_BOUNDARY,
.use_clustering = DISABLE_CLUSTERING,
.proc_name  = "cumanascsi2",
diff --git a/drivers/scsi/arm/eesox.c b/drivers/scsi/arm/eesox.c
index a8ad688..e93e047 100644
--- a/drivers/scsi/arm/eesox.c
+++ b/drivers/scsi/arm/eesox.c
@@ -484,7 +484,7 @@ static struct scsi_host_template eesox_template = {
.eh_abort_handler   = fas216_eh_abort,
.can_queue  = 1,
.this_id= 7,
-   .sg_tablesize   = SCSI_MAX_SG_CHAIN_SEGMENTS,
+   .sg_tablesize   = SG_MAX_SEGMENTS,
.dma_boundary   = IOMD_DMA_BOUNDARY,
.use_clustering = DISABLE_CLUSTERING,
.proc_name  = "eesox",
diff --git a/drivers/scsi/arm/powertec.c b/drivers/scsi/arm/powertec.c
index 5e1b73e..79aa889 100644
--- a/drivers/scsi/arm/powertec.c
+++ b/drivers/scsi/arm/powertec.c
@@ -291,7 +291,7 @@ static struct scsi_host_template powertecscsi_template = {
 
.can_queue  = 8,
.this_id= 7,
-   .sg_tablesize   = SCSI_MAX_SG_CHAIN_SEGMENTS,
+   .sg_tablesize   = SG_MAX_SEGMENTS,
.dma_boundary   = IOMD_DMA_BOUNDARY,
.cmd_per_lun= 2,
.use_clustering = ENABLE_CLUSTERING,
diff --git a/drivers/scsi/esas2r/esas2r_main.c 
b/drivers/scsi/esas2r/esas2r_main.c
index 33581ba..2aca4d1 100644
--- a/drivers/scsi/esas2r/esas2r_main.c
+++ b/drivers/scsi/esas2r/esas2r_main.c
@@ -246,7 +246,7 @@ static struct scsi_host_template driver_template = {
   

[PATCH v2 3/5] scsi: rename SG related struct and functions

2016-03-22 Thread Ming Lin
From: Ming Lin 

Rename SCSI specific struct and functions to more genenic names.

Signed-off-by: Ming Lin 
---
 drivers/scsi/scsi_lib.c | 52 -
 1 file changed, 26 insertions(+), 26 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index ab3dd09..5fd9dd7 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -40,10 +40,10 @@
 #include "scsi_logging.h"
 
 
-#define SG_MEMPOOL_NR  ARRAY_SIZE(scsi_sg_pools)
+#define SG_MEMPOOL_NR  ARRAY_SIZE(sg_pools)
 #define SG_MEMPOOL_SIZE2
 
-struct scsi_host_sg_pool {
+struct sg_pool {
size_t  size;
char*name;
struct kmem_cache   *slab;
@@ -54,7 +54,7 @@ struct scsi_host_sg_pool {
 #if (SCSI_MAX_SG_SEGMENTS < 32)
 #error SCSI_MAX_SG_SEGMENTS is too small (must be 32 or greater)
 #endif
-static struct scsi_host_sg_pool scsi_sg_pools[] = {
+static struct sg_pool sg_pools[] = {
SP(8),
SP(16),
 #if (SCSI_MAX_SG_SEGMENTS > 32)
@@ -553,7 +553,7 @@ void scsi_run_host_queues(struct Scsi_Host *shost)
scsi_run_queue(sdev->request_queue);
 }
 
-static inline unsigned int scsi_sgtable_index(unsigned short nents)
+static inline unsigned int sg_pool_index(unsigned short nents)
 {
unsigned int index;
 
@@ -567,30 +567,30 @@ static inline unsigned int scsi_sgtable_index(unsigned 
short nents)
return index;
 }
 
-static void scsi_sg_free(struct scatterlist *sgl, unsigned int nents)
+static void sg_pool_free(struct scatterlist *sgl, unsigned int nents)
 {
-   struct scsi_host_sg_pool *sgp;
+   struct sg_pool *sgp;
 
-   sgp = scsi_sg_pools + scsi_sgtable_index(nents);
+   sgp = sg_pools + sg_pool_index(nents);
mempool_free(sgl, sgp->pool);
 }
 
-static struct scatterlist *scsi_sg_alloc(unsigned int nents, gfp_t gfp_mask)
+static struct scatterlist *sg_pool_alloc(unsigned int nents, gfp_t gfp_mask)
 {
-   struct scsi_host_sg_pool *sgp;
+   struct sg_pool *sgp;
 
-   sgp = scsi_sg_pools + scsi_sgtable_index(nents);
+   sgp = sg_pools + sg_pool_index(nents);
return mempool_alloc(sgp->pool, gfp_mask);
 }
 
-static void scsi_free_sgtable(struct sg_table *table, bool first_chunk)
+static void sg_free_table_chained(struct sg_table *table, bool first_chunk)
 {
if (first_chunk && table->orig_nents <= SCSI_MAX_SG_SEGMENTS)
return;
-   __sg_free_table(table, SCSI_MAX_SG_SEGMENTS, first_chunk, scsi_sg_free);
+   __sg_free_table(table, SCSI_MAX_SG_SEGMENTS, first_chunk, sg_pool_free);
 }
 
-static int scsi_alloc_sgtable(struct sg_table *table, int nents,
+static int sg_alloc_table_chained(struct sg_table *table, int nents,
struct scatterlist *first_chunk)
 {
int ret;
@@ -606,9 +606,9 @@ static int scsi_alloc_sgtable(struct sg_table *table, int 
nents,
}
 
ret = __sg_alloc_table(table, nents, SCSI_MAX_SG_SEGMENTS,
-  first_chunk, GFP_ATOMIC, scsi_sg_alloc);
+  first_chunk, GFP_ATOMIC, sg_pool_alloc);
if (unlikely(ret))
-   scsi_free_sgtable(table, (bool)first_chunk);
+   sg_free_table_chained(table, (bool)first_chunk);
return ret;
 }
 
@@ -627,14 +627,14 @@ static void scsi_mq_free_sgtables(struct scsi_cmnd *cmd)
struct scsi_data_buffer *sdb;
 
if (cmd->sdb.table.nents)
-   scsi_free_sgtable(&cmd->sdb.table, true);
+   sg_free_table_chained(&cmd->sdb.table, true);
if (cmd->request->next_rq) {
sdb = cmd->request->next_rq->special;
if (sdb)
-   scsi_free_sgtable(&sdb->table, true);
+   sg_free_table_chained(&sdb->table, true);
}
if (scsi_prot_sg_count(cmd))
-   scsi_free_sgtable(&cmd->prot_sdb->table, true);
+   sg_free_table_chained(&cmd->prot_sdb->table, true);
 }
 
 static void scsi_mq_uninit_cmd(struct scsi_cmnd *cmd)
@@ -673,19 +673,19 @@ static void scsi_mq_uninit_cmd(struct scsi_cmnd *cmd)
 static void scsi_release_buffers(struct scsi_cmnd *cmd)
 {
if (cmd->sdb.table.nents)
-   scsi_free_sgtable(&cmd->sdb.table, false);
+   sg_free_table_chained(&cmd->sdb.table, false);
 
memset(&cmd->sdb, 0, sizeof(cmd->sdb));
 
if (scsi_prot_sg_count(cmd))
-   scsi_free_sgtable(&cmd->prot_sdb->table, false);
+   sg_free_table_chained(&cmd->prot_sdb->table, false);
 }
 
 static void scsi_release_bidi_buffers(struct scsi_cmnd *cmd)
 {
struct scsi_data_buffer *bidi_sdb = cmd->request->next_rq->special;
 
-   scsi_free_sgtable(&bidi_sdb->table, false);
+   sg_free_table_chained(&bidi_sdb->table, false);
kmem_cache_free(scsi_sdb_cache, bidi_sdb);
cmd->request->next_rq->special = NULL;
 }
@@ -1089,7 +1089,7 @@ static int scsi_init_sg

[PATCH v2 5/5] lib: scatterlist: move SG pool code from SCSI driver to lib/sg_pool.c

2016-03-22 Thread Ming Lin
From: Ming Lin 

Now it's ready to move the mempool based SG chained allocator code from
SCSI driver to lib/sg_pool.c, which will be compiled only based on a Kconfig
symbol CONFIG_SG_POOL.

SCSI selects CONFIG_SG_POOL.

Signed-off-by: Ming Lin 
---
 drivers/scsi/Kconfig|   1 +
 drivers/scsi/scsi_lib.c | 137 ---
 include/linux/scatterlist.h |  25 +++
 include/scsi/scsi.h |  19 -
 lib/Kconfig |   7 ++
 lib/Makefile|   1 +
 lib/sg_pool.c   | 172 
 7 files changed, 206 insertions(+), 156 deletions(-)
 create mode 100644 lib/sg_pool.c

diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index e2f31c9..ee9cf41 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -17,6 +17,7 @@ config SCSI
tristate "SCSI device support"
depends on BLOCK
select SCSI_DMA if HAS_DMA
+   select SG_POOL
---help---
  If you want to use a SCSI hard disk, SCSI tape drive, SCSI CD-ROM or
  any other SCSI device under Linux, say Y and make sure that you know
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 478b0e8..6d818e8 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -14,8 +14,6 @@
 #include 
 #include 
 #include 
-#include 
-#include 
 #include 
 #include 
 #include 
@@ -40,39 +38,6 @@
 #include "scsi_logging.h"
 
 
-#define SG_MEMPOOL_NR  ARRAY_SIZE(sg_pools)
-#define SG_MEMPOOL_SIZE2
-
-struct sg_pool {
-   size_t  size;
-   char*name;
-   struct kmem_cache   *slab;
-   mempool_t   *pool;
-};
-
-#define SP(x) { .size = x, "sgpool-" __stringify(x) }
-#if (SG_CHUNK_SIZE < 32)
-#error SG_CHUNK_SIZE is too small (must be 32 or greater)
-#endif
-static struct sg_pool sg_pools[] = {
-   SP(8),
-   SP(16),
-#if (SG_CHUNK_SIZE > 32)
-   SP(32),
-#if (SG_CHUNK_SIZE > 64)
-   SP(64),
-#if (SG_CHUNK_SIZE > 128)
-   SP(128),
-#if (SG_CHUNK_SIZE > 256)
-#error SG_CHUNK_SIZE is too large (256 MAX)
-#endif
-#endif
-#endif
-#endif
-   SP(SG_CHUNK_SIZE)
-};
-#undef SP
-
 struct kmem_cache *scsi_sdb_cache;
 
 /*
@@ -553,65 +518,6 @@ void scsi_run_host_queues(struct Scsi_Host *shost)
scsi_run_queue(sdev->request_queue);
 }
 
-static inline unsigned int sg_pool_index(unsigned short nents)
-{
-   unsigned int index;
-
-   BUG_ON(nents > SG_CHUNK_SIZE);
-
-   if (nents <= 8)
-   index = 0;
-   else
-   index = get_count_order(nents) - 3;
-
-   return index;
-}
-
-static void sg_pool_free(struct scatterlist *sgl, unsigned int nents)
-{
-   struct sg_pool *sgp;
-
-   sgp = sg_pools + sg_pool_index(nents);
-   mempool_free(sgl, sgp->pool);
-}
-
-static struct scatterlist *sg_pool_alloc(unsigned int nents, gfp_t gfp_mask)
-{
-   struct sg_pool *sgp;
-
-   sgp = sg_pools + sg_pool_index(nents);
-   return mempool_alloc(sgp->pool, gfp_mask);
-}
-
-static void sg_free_table_chained(struct sg_table *table, bool first_chunk)
-{
-   if (first_chunk && table->orig_nents <= SG_CHUNK_SIZE)
-   return;
-   __sg_free_table(table, SG_CHUNK_SIZE, first_chunk, sg_pool_free);
-}
-
-static int sg_alloc_table_chained(struct sg_table *table, int nents,
-   struct scatterlist *first_chunk)
-{
-   int ret;
-
-   BUG_ON(!nents);
-
-   if (first_chunk) {
-   if (nents <= SG_CHUNK_SIZE) {
-   table->nents = table->orig_nents = nents;
-   sg_init_table(table->sgl, nents);
-   return 0;
-   }
-   }
-
-   ret = __sg_alloc_table(table, nents, SG_CHUNK_SIZE,
-  first_chunk, GFP_ATOMIC, sg_pool_alloc);
-   if (unlikely(ret))
-   sg_free_table_chained(table, (bool)first_chunk);
-   return ret;
-}
-
 static void scsi_uninit_cmd(struct scsi_cmnd *cmd)
 {
if (cmd->request->cmd_type == REQ_TYPE_FS) {
@@ -2269,8 +2175,6 @@ EXPORT_SYMBOL(scsi_unblock_requests);
 
 int __init scsi_init_queue(void)
 {
-   int i;
-
scsi_sdb_cache = kmem_cache_create("scsi_data_buffer",
   sizeof(struct scsi_data_buffer),
   0, 0, NULL);
@@ -2279,53 +2183,12 @@ int __init scsi_init_queue(void)
return -ENOMEM;
}
 
-   for (i = 0; i < SG_MEMPOOL_NR; i++) {
-   struct sg_pool *sgp = sg_pools + i;
-   int size = sgp->size * sizeof(struct scatterlist);
-
-   sgp->slab = kmem_cache_create(sgp->name, size, 0,
-   SLAB_HWCACHE_ALIGN, NULL);
-   if (!sgp->slab) {
-   printk(KERN_ERR "SCSI: can't init sg slab %s\n",
-   sgp->name);
-   goto cle

Re: [BUG] mpt2sas: driver init fails on kernel >=4.2 for 9211-8i IT

2016-03-22 Thread Matthias Prager
Hi Sreekanth,

Am 22.03.2016 um 11:52 schrieb Sreekanth Reddy:
> Hi Matthias,
> 
> Thanks for providing detail explanation.
> 
> Currently I am trying to reproduce this issue locally. I have used the
> same HBA card but still I am not able to reproduce this issue on 4.5
> kernel.
> 
> I will try for few more times and I will also try to get the diff
> between 4.1 to 4.5 kernels w.r.t msix vectors support.
I will try to bisect this tomorrow. And I will try to reproduce this
outside the VM environment on bare metal.

> 
> Meanwhile can you please provide me the output of "lspci -vvv" command
> for LSI SAS2 HBA's and also hexdump of config sysfs parameter for SAS2
> pci device. I need this data to be collected before "timeout" print is
> observed, or in the mpt3sas_base.c file and in the
> mpt3sas_base_attach(). return from mpt3sas_base_attach() function if
> return value of mpt3sas_base_map_resources() is non zero instead of
> freeing the resources, as shown below
> 
> ---
> r = mpt3sas_base_map_resources(ioc);
> if (r)
> - goto out_free_resources;
> +return r;
> -
I will get you this data tomorrow too.

> 
> Also are you sure that this issue won't occur on less than 4.1 kernel?
Yes definitely - I never observed these timeouts with any kernel
<=4.1.20, and I started seeing them from kernel 4.2.1 onward (I haven't
actually tested 4.2.0 yet).

> 
> Thanks,
> Sreekanth
> 
> On Mon, Mar 21, 2016 at 10:00 PM, Matthias Prager
>  wrote:
>> Am 21.03.2016 um 16:52 schrieb Matthias Prager:
>>> Hi Sreekanth,
>>>
>>> thanks for digging into this issue. Regarding the 4.5.0 after 4.1.20
>>> boot statement, I will try to express myself better:
>>>
>>> I first started the system with the 4.1.20 kernel. Then I issued an
>>> 'init 6' warm-reboot and chose to boot the 4.5.0 kernel. This procedure
>>> often works (i.e. I'm able to boot the newer kernels (>=4.2) this way
>>> with mpt2sas initializing just fine).
>> One small addition: warm-rebooting kernel 4.5.0 from kernel 4.5.0
>> reliably leads to this issue manifesting (mpt2sas driver not initializing).
>>
>>>
>>> I will try the msix_disable parameter and report back. Maybe kernel
>>> version 4.2 broke interrupt handling somehow?
>> With the msix_disable parameter set kernels 4.5.0, 4.4.5, 4.3.3 and
>> 4.2.1 boot without the mpt2sas init issue. I also tried the pci=nomsi
>> paremeter instead, which also works fine.
>>
>> The igb ethernet-driver issue I mentioned as possibly related has not
>> changed with pci=nomsi. So I'm assuming it is not the same issue, but
>> possibly related, since it also occurs on kernels >=4.2.
>>
>>>
>>> ---
>>> Matthias
>>>
>>>
>>> Am 21.03.2016 um 14:59 schrieb Sreekanth Reddy:
 Hi Matthias,

 Thanks for providing the logs. In these logs, I am not observing any
 such a prints which should be suspected for this issue.

 Can you please try once by setting mpt3sas driver's "msix_disable"
 module parameter to one.

 Also, can you please elaborate below statement for me, I am not able
 understand this statement
  "I managed to boot the same 4.5.0 kernel successfully after warm
 rebooting from 4.1.20"

 Thanks,
 Sreekanth

 On Mon, Mar 21, 2016 at 2:48 PM, Matthias Prager
  wrote:
> Hi Sreekanth,
>
> thank you for replying so quickly. Here are the logs you requested
> (kernel 4.5.0):
>
>> [2.083177] mpt3sas version 09.102.00.00 loaded
>> [2.083757] mpt2sas_cm0: mpt3sas_base_attach
>> [2.083956] mpt2sas_cm0: mpt3sas_base_map_resources
>> [2.084708] mpt2sas_cm0: 32 BIT PCI BUS DMA ADDRESSING SUPPORTED, 
>> total mem (3074748 kB)
>> [2.084964] mpt2sas_cm0: _base_get_ioc_facts
>> [2.085154] mpt2sas_cm0: _base_wait_for_iocstate
>> [2.140893]offset:data
>> [2.141082][0x00]:03100200
>> [2.141257][0x04]:2300
>> [2.141432][0x08]:
>> [2.141606][0x0c]:
>> [2.141780][0x10]:
>> [2.141955][0x14]:00010480
>> [2.142129][0x18]:22130d68
>> [2.142303][0x1c]:0001285c
>> [2.142477][0x20]:14000400
>> [2.142651][0x24]:0020
>> [2.142825][0x28]:010f
>> [2.143000][0x2c]:000c000b
>> [2.143174][0x30]:003c0003
>> [2.143349][0x34]:0020ffe0
>> [2.143544][0x38]:00800122
>> [2.143719][0x3c]:0009
>> [2.143895] mpt2sas_cm0: hba queue depth(3432), max chains per io(128)
>> [2.144106] mpt2sas_cm0: request frame size(128), reply frame 
>> size(128)
>> [2.144397] mpt2sas_cm0: msix is supported, vector_count(1)
>> [2.144600]

RE: [PATCH 1/1] scsi: storvsc: Support manual scan of FC hosts on Hyper-V

2016-03-22 Thread KY Srinivasan


> -Original Message-
> From: KY Srinivasan
> Sent: Sunday, March 20, 2016 11:59 AM
> To: 'James Bottomley' ; Martin
> K. Petersen 
> Cc: Christoph Hellwig ; gre...@linuxfoundation.org;
> linux-ker...@vger.kernel.org; de...@linuxdriverproject.org;
> oher...@suse.com; jbottom...@parallels.com; linux-scsi@vger.kernel.org;
> a...@canonical.com; vkuzn...@redhat.com; jasow...@redhat.com;
> h...@suse.de
> Subject: RE: [PATCH 1/1] scsi: storvsc: Support manual scan of FC hosts on
> Hyper-V
> 
> 
> 
> > -Original Message-
> > From: James Bottomley
> [mailto:james.bottom...@hansenpartnership.com]
> > Sent: Friday, March 18, 2016 3:41 PM
> > To: KY Srinivasan ; Martin K. Petersen
> > 
> > Cc: Christoph Hellwig ; gre...@linuxfoundation.org;
> > linux-ker...@vger.kernel.org; de...@linuxdriverproject.org;
> > oher...@suse.com; jbottom...@parallels.com; linux-scsi@vger.kernel.org;
> > a...@canonical.com; vkuzn...@redhat.com; jasow...@redhat.com;
> > h...@suse.de
> > Subject: Re: [PATCH 1/1] scsi: storvsc: Support manual scan of FC hosts on
> > Hyper-V
> >
> > On Thu, 2016-03-17 at 00:01 +, KY Srinivasan wrote:
> > > The only attributes I would be interested are:
> > > 1) node name
> > > 2) port name
> > >
> > > Ideally, if this can show under /sys/class/fc_host/hostx/port_name
> > > and node_name,
> > > it will be ideal since all user scripts can work.
> >
> > OK, like this?
> 
> Yes; thank you very much James. Looking at the patch though, it may be an
> overkill considering how much of the code is duplicated. The current fc
> transport
> class does give us the flexibility to control the attributes we want to 
> surface
> (fc_function_template). In any case I will test this code and get back to you
> soon.

Today I got a chance to test this patch. Looks like all the state is not getting
properly set in this new transport class. I am hitting this NULL pointer 
reference fault in
get_device_parent(). Looks like the device class is not properly setup for
this transport class. class_dir_create_and_add() is not called for this class
and so the glue_dirs is NULL.   I fixed the issue:

1) You will need to call the transport_class_register() for this new transport 
class in
fc_transport_init()
2) We cannot use fc_host as the name in this new class since the standard fc 
transport already
Uses that name. I changed the name to get this to work. This will create a new 
directory under /sys/class.
So my original goal of  being compatible with existing scripts that expect to 
find the information under
/sys/class/fc_host will not be met.

Regards,

K. Y


> 
> Regards,
> 
> K. Y
> 
> 
> >
> > From 7af7c428e7e04ddcc87fda12d6571e3dff8ae024 Mon Sep 17 00:00:00
> > 2001
> > From: James Bottomley 
> > Date: Fri, 18 Mar 2016 15:35:45 -0700
> > Subject: scsi_transport_fc: introduce lightweight class for virtualization
> >  systems
> >
> > The FC transport class is very heavily tilted towards helping things
> > which operate a fabric (as it should be).  However, there seems to be
> > a need for a lightweight version for use in virtual systems that
> > simply want to show pass through FC information without making any use
> > of the heavyweight functions.  This is an attempt to give them what
> > they want: the lightweight class has no vports or rports and only two
> > host attributes.  Essentially, it's designed for the HV storvsc
> > driver, but if other virtualizataion systems have similar problems, we
> > can add more attributes.
> >
> > Signed-off-by: James Bottomley 
> > ---
> >  drivers/scsi/scsi_transport_fc.c | 94
> > 
> >  include/scsi/scsi_transport_fc.h |  3 ++
> >  2 files changed, 97 insertions(+)
> >
> > diff --git a/drivers/scsi/scsi_transport_fc.c
> b/drivers/scsi/scsi_transport_fc.c
> > index 8a88226..a9fcb4d 100644
> > --- a/drivers/scsi/scsi_transport_fc.c
> > +++ b/drivers/scsi/scsi_transport_fc.c
> > @@ -351,6 +351,27 @@ struct fc_internal {
> >
> >  #define to_fc_internal(tmpl)   container_of(tmpl, struct fc_internal, 
> > t)
> >
> > +#define FC_LW_HOST_NUM_ATTRS   2
> > +struct fc_lw_internal {
> > +   struct scsi_transport_template t;
> > +   struct fc_function_template *f;
> > +
> > +   /*
> > +* For attributes : each object has :
> > +*   An array of the actual attributes structures
> > +*   An array of null-terminated pointers to the attribute
> > +* structures - used for mid-layer interaction.
> > +*
> > +* The attribute containers for the starget and host are are
> > +* part of the midlayer. As the remote port is specific to the
> > +* fc transport, we must provide the attribute container.
> > +*/
> > +   struct device_attribute
> > private_host_attrs[FC_LW_HOST_NUM_ATTRS];
> > +   struct device_attribute *host_attrs[FC_LW_HOST_NUM_ATTRS + 1];
> > +};
> > +
> > +#define to_fc_lw_internal(tmpl)container_of(tmpl, struct
> > fc_lw_internal, t)
> > +
> >  static int fc_target_setup(struct transport_contai

[PATCH v3 1/3] scsi: make some Additional Sense strings more grep'able

2016-03-22 Thread Rasmus Villemoes
There's little point in breaking these strings over multiple lines.

Signed-off-by: Rasmus Villemoes 
Reviewed-by: Hannes Reinecke 
---
 drivers/scsi/constants.c | 27 +--
 1 file changed, 9 insertions(+), 18 deletions(-)

diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index fa09d4be2b53..58d94e3c3713 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -346,11 +346,9 @@ static const struct error_info additional[] =
{0x0407, "Logical unit not ready, operation in progress"},
{0x0408, "Logical unit not ready, long write in progress"},
{0x0409, "Logical unit not ready, self-test in progress"},
-   {0x040A, "Logical unit not accessible, asymmetric access state "
-"transition"},
+   {0x040A, "Logical unit not accessible, asymmetric access state 
transition"},
{0x040B, "Logical unit not accessible, target port in standby state"},
-   {0x040C, "Logical unit not accessible, target port in unavailable "
-"state"},
+   {0x040C, "Logical unit not accessible, target port in unavailable 
state"},
{0x040D, "Logical unit not ready, structure check required"},
{0x040E, "Logical unit not ready, security session in progress"},
{0x0410, "Logical unit not ready, auxiliary memory not accessible"},
@@ -363,11 +361,9 @@ static const struct error_info additional[] =
{0x0417, "Logical unit not ready, calibration required"},
{0x0418, "Logical unit not ready, a door is open"},
{0x0419, "Logical unit not ready, operating in sequential mode"},
-   {0x041A, "Logical unit not ready, start stop unit command in "
-"progress"},
+   {0x041A, "Logical unit not ready, start stop unit command in progress"},
{0x041B, "Logical unit not ready, sanitize in progress"},
-   {0x041C, "Logical unit not ready, additional power use not yet "
-"granted"},
+   {0x041C, "Logical unit not ready, additional power use not yet 
granted"},
{0x041D, "Logical unit not ready, configuration in progress"},
{0x041E, "Logical unit not ready, microcode activation required"},
{0x041F, "Logical unit not ready, microcode download required"},
@@ -559,8 +555,7 @@ static const struct error_info additional[] =
{0x2300, "Invalid token operation, cause not reportable"},
{0x2301, "Invalid token operation, unsupported token type"},
{0x2302, "Invalid token operation, remote token usage not supported"},
-   {0x2303, "Invalid token operation, remote rod token creation not "
-"supported"},
+   {0x2303, "Invalid token operation, remote rod token creation not 
supported"},
{0x2304, "Invalid token operation, token unknown"},
{0x2305, "Invalid token operation, token corrupt"},
{0x2306, "Invalid token operation, token revoked"},
@@ -641,8 +636,7 @@ static const struct error_info additional[] =
{0x2A0D, "Data encryption capabilities changed"},
{0x2A10, "Timestamp changed"},
{0x2A11, "Data encryption parameters changed by another i_t nexus"},
-   {0x2A12, "Data encryption parameters changed by vendor specific "
-"event"},
+   {0x2A12, "Data encryption parameters changed by vendor specific event"},
{0x2A13, "Data encryption key instance counter has changed"},
{0x2A14, "SA creation capabilities data has changed"},
{0x2A15, "Medium removal prevention preempted"},
@@ -759,8 +753,7 @@ static const struct error_info additional[] =
{0x3B19, "Element enabled"},
{0x3B1A, "Data transfer device removed"},
{0x3B1B, "Data transfer device inserted"},
-   {0x3B1C, "Too many logical objects on partition to support "
-"operation"},
+   {0x3B1C, "Too many logical objects on partition to support operation"},
 
{0x3D00, "Invalid bits in identify message"},
 
@@ -957,8 +950,7 @@ static const struct error_info additional[] =
{0x5D39, "Data channel impending failure throughput performance"},
{0x5D3A, "Data channel impending failure seek time performance"},
{0x5D3B, "Data channel impending failure spin-up retry count"},
-   {0x5D3C, "Data channel impending failure drive calibration retry "
-"count"},
+   {0x5D3C, "Data channel impending failure drive calibration retry 
count"},
{0x5D40, "Servo impending failure general hard drive failure"},
{0x5D41, "Servo impending failure drive error rate too high"},
{0x5D42, "Servo impending failure data error rate too high"},
@@ -1070,8 +1062,7 @@ static const struct error_info additional[] =
 
{0x6E00, "Command to logical unit failed"},
 
-   {0x6F00, "Copy protection key exchange failure - authentication "
-"failure"},
+   {0x6F00, "Copy protection key exchange failure - authentication 
failure"},
{0x6F01, "Copy protection key exchange failure - key not present"},

[PATCH v3 3/3] scsi: reduce CONFIG_SCSI_CONSTANTS=y impact by 8k

2016-03-22 Thread Rasmus Villemoes
On 64 bit, struct error_info has 6 bytes of padding, which amounts to
over 4k of wasted space in the additional[] array. We could easily get
rid of that by instead using separate arrays for the codes and the
pointers. However, we can do even better than that and save an
additional 6 bytes per entry: In the table, just store the sizeof()
the corresponding string literal. The cumulative sum of these is then
the appropriate offset into additional_text, which is built from the
concatenation (with '\0's inbetween) of the strings.

$ scripts/bloat-o-meter /tmp/vmlinux vmlinux
add/remove: 0/0 grow/shrink: 1/1 up/down: 24/-8488 (-8464)
function old new   delta
scsi_extd_sense_format   136 160 +24
additional 113122824   -8488

The Kconfig help text used to say that CONFIG_SCSI_CONSTANTS=y costs
around 75 KB, but that was a little exaggerated. The actual number was
closer to 44K, and 36K with this patch.

Signed-off-by: Rasmus Villemoes 
Reviewed-by: Hannes Reinecke 
Tested-by: Douglas Gilbert 
---
 drivers/scsi/Kconfig |  4 ++--
 drivers/scsi/constants.c | 26 +-
 2 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index e80768f8e579..47611bda633f 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -202,12 +202,12 @@ config SCSI_ENCLOSURE
  certain enclosure conditions to be reported and is not required.
 
 config SCSI_CONSTANTS
-   bool "Verbose SCSI error reporting (kernel size +=75K)"
+   bool "Verbose SCSI error reporting (kernel size += 36K)"
depends on SCSI
help
  The error messages regarding your SCSI hardware will be easier to
  understand if you say Y here; it will enlarge your kernel by about
- 75 KB. If in doubt, say Y.
+ 36 KB. If in doubt, say Y.
 
 config SCSI_LOGGING
bool "SCSI logging facility"
diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index 6e813eec4f8d..83458f7a2824 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -292,17 +292,30 @@ bool scsi_opcode_sa_name(int opcode, int service_action,
 
 struct error_info {
unsigned short code12;  /* 0x0302 looks better than 0x03,0x02 */
-   const char * text;
+   unsigned short size;
 };
 
+/*
+ * There are 700+ entries in this table. To save space, we don't store
+ * (code, pointer) pairs, which would make sizeof(struct
+ * error_info)==16 on 64 bits. Rather, the second element just stores
+ * the size (including \0) of the corresponding string, and we use the
+ * sum of these to get the appropriate offset into additional_text
+ * defined below. This approach saves 12 bytes per entry.
+ */
 static const struct error_info additional[] =
 {
-#define SENSE_CODE(c, s) {c, s},
+#define SENSE_CODE(c, s) {c, sizeof(s)},
 #include "sense_codes.h"
 #undef SENSE_CODE
-   {0, NULL}
 };
 
+static const char *additional_text =
+#define SENSE_CODE(c, s) s "\0"
+#include "sense_codes.h"
+#undef SENSE_CODE
+   ;
+
 struct error_info2 {
unsigned char code1, code2_min, code2_max;
const char * str;
@@ -364,11 +377,14 @@ scsi_extd_sense_format(unsigned char asc, unsigned char 
ascq, const char **fmt)
 {
int i;
unsigned short code = ((asc << 8) | ascq);
+   unsigned offset = 0;
 
*fmt = NULL;
-   for (i = 0; additional[i].text; i++)
+   for (i = 0; i < ARRAY_SIZE(additional); i++) {
if (additional[i].code12 == code)
-   return additional[i].text;
+   return additional_text + offset;
+   offset += additional[i].size;
+   }
for (i = 0; additional2[i].fmt; i++) {
if (additional2[i].code1 == asc &&
ascq >= additional2[i].code2_min &&
-- 
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


[PATCH v3 2/3] scsi: move Additional Sense Codes to separate file

2016-03-22 Thread Rasmus Villemoes
This is a purely mechanical move of the list of additional sense codes
to a separate file, in preparation for reducing the impact of choosing
CONFIG_SCSI_CONSTANTS=y by about 8k.

Signed-off-by: Rasmus Villemoes 
Reviewed-by: Hannes Reinecke 
---
 drivers/scsi/constants.c   | 830 +
 drivers/scsi/sense_codes.h | 826 
 2 files changed, 829 insertions(+), 827 deletions(-)
 create mode 100644 drivers/scsi/sense_codes.h

diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index 58d94e3c3713..6e813eec4f8d 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -295,835 +295,11 @@ struct error_info {
const char * text;
 };
 
-/*
- * The canonical list of T10 Additional Sense Codes is available at:
- * http://www.t10.org/lists/asc-num.txt [most recent: 20141221]
- */
-
 static const struct error_info additional[] =
 {
-   {0x, "No additional sense information"},
-   {0x0001, "Filemark detected"},
-   {0x0002, "End-of-partition/medium detected"},
-   {0x0003, "Setmark detected"},
-   {0x0004, "Beginning-of-partition/medium detected"},
-   {0x0005, "End-of-data detected"},
-   {0x0006, "I/O process terminated"},
-   {0x0007, "Programmable early warning detected"},
-   {0x0011, "Audio play operation in progress"},
-   {0x0012, "Audio play operation paused"},
-   {0x0013, "Audio play operation successfully completed"},
-   {0x0014, "Audio play operation stopped due to error"},
-   {0x0015, "No current audio status to return"},
-   {0x0016, "Operation in progress"},
-   {0x0017, "Cleaning requested"},
-   {0x0018, "Erase operation in progress"},
-   {0x0019, "Locate operation in progress"},
-   {0x001A, "Rewind operation in progress"},
-   {0x001B, "Set capacity operation in progress"},
-   {0x001C, "Verify operation in progress"},
-   {0x001D, "ATA pass through information available"},
-   {0x001E, "Conflicting SA creation request"},
-   {0x001F, "Logical unit transitioning to another power condition"},
-   {0x0020, "Extended copy information available"},
-   {0x0021, "Atomic command aborted due to ACA"},
-
-   {0x0100, "No index/sector signal"},
-
-   {0x0200, "No seek complete"},
-
-   {0x0300, "Peripheral device write fault"},
-   {0x0301, "No write current"},
-   {0x0302, "Excessive write errors"},
-
-   {0x0400, "Logical unit not ready, cause not reportable"},
-   {0x0401, "Logical unit is in process of becoming ready"},
-   {0x0402, "Logical unit not ready, initializing command required"},
-   {0x0403, "Logical unit not ready, manual intervention required"},
-   {0x0404, "Logical unit not ready, format in progress"},
-   {0x0405, "Logical unit not ready, rebuild in progress"},
-   {0x0406, "Logical unit not ready, recalculation in progress"},
-   {0x0407, "Logical unit not ready, operation in progress"},
-   {0x0408, "Logical unit not ready, long write in progress"},
-   {0x0409, "Logical unit not ready, self-test in progress"},
-   {0x040A, "Logical unit not accessible, asymmetric access state 
transition"},
-   {0x040B, "Logical unit not accessible, target port in standby state"},
-   {0x040C, "Logical unit not accessible, target port in unavailable 
state"},
-   {0x040D, "Logical unit not ready, structure check required"},
-   {0x040E, "Logical unit not ready, security session in progress"},
-   {0x0410, "Logical unit not ready, auxiliary memory not accessible"},
-   {0x0411, "Logical unit not ready, notify (enable spinup) required"},
-   {0x0412, "Logical unit not ready, offline"},
-   {0x0413, "Logical unit not ready, SA creation in progress"},
-   {0x0414, "Logical unit not ready, space allocation in progress"},
-   {0x0415, "Logical unit not ready, robotics disabled"},
-   {0x0416, "Logical unit not ready, configuration required"},
-   {0x0417, "Logical unit not ready, calibration required"},
-   {0x0418, "Logical unit not ready, a door is open"},
-   {0x0419, "Logical unit not ready, operating in sequential mode"},
-   {0x041A, "Logical unit not ready, start stop unit command in progress"},
-   {0x041B, "Logical unit not ready, sanitize in progress"},
-   {0x041C, "Logical unit not ready, additional power use not yet 
granted"},
-   {0x041D, "Logical unit not ready, configuration in progress"},
-   {0x041E, "Logical unit not ready, microcode activation required"},
-   {0x041F, "Logical unit not ready, microcode download required"},
-   {0x0420, "Logical unit not ready, logical unit reset required"},
-   {0x0421, "Logical unit not ready, hard reset required"},
-   {0x0422, "Logical unit not ready, power cycle required"},
-
-   {0x0500, "Logical unit does not respond to selection"},
-
-   {0x0600, "No reference position 

[PATCH v3 0/3] scsi: reduce CONFIG_SCSI_CONSTANTS=y impact by 8k

2016-03-22 Thread Rasmus Villemoes
This is mostly identical to v2 sent in November, rebased (cleanly) to
current mainline. The only change is that I've this time also updated
the Kconfig help text in 3/3, since the 75 KB was a bit
exaggerated. After these patches are applied, the impact of choosing
CONFIG_SCSI_CONSTANTS=y is about 36 KB, while it was about 44 KB
before. I took the liberty of including Hannes' Reviewed-by and
Douglas' Tested-by despite that addition.

Rasmus Villemoes (3):
  scsi: make some Additional Sense strings more grep'able
  scsi: move Additional Sense Codes to separate file
  scsi: reduce CONFIG_SCSI_CONSTANTS=y impact by 8k

 drivers/scsi/Kconfig   |   4 +-
 drivers/scsi/constants.c   | 859 ++---
 drivers/scsi/sense_codes.h | 826 +++
 3 files changed, 849 insertions(+), 840 deletions(-)
 create mode 100644 drivers/scsi/sense_codes.h

-- 
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


[GIT PULL] target updates for v4.6-rc1

2016-03-22 Thread Nicholas A. Bellinger
Hello Linus,

Here are the target updates for v4.6-rc1 code.

Please go ahead and pull from:

  git://git.kernel.org/pub/scm/linux/kernel/git/nab/target-pending.git for-next

The highlights this round include:

  - Add target_alloc_session() w/ callback helper for doing se_session
allocation + tag + se_node_acl lookup. (HCH + nab)
  - Tree-wide fabric driver conversion to use target_alloc_session()
  - Convert sbp-target to use percpu_ida tag pre-allocation, and
TARGET_SCF_ACK_KREF I/O krefs (Chris Boot + nab)
  - Convert usb-gadget to use percpu_ida tag pre-allocation, and
TARGET_SCF_ACK_KREF I/O krefs (Andrzej Pietrasiewicz + nab)
  - Convert xen-scsiback to use percpu_ida tag pre-allocation, and
TARGET_SCF_ACK_KREF I/O krefs (Juergen Gross + nab)
  - Convert tcm_fc to use TARGET_SCF_ACK_KREF I/O + TMR krefs
  - Convert ib_srpt to use percpu_ida tag pre-allocation
  - Add DebugFS node for qla2xxx target sess list (Quinn)
  - Rework iser-target connection termination (Jenny + Sagi)
  - Convert iser-target to new CQ API (HCH)
  - Add pass-through WRITE_SAME support for IBLOCK (Mike Christie)
  - Introduce data_bitmap for asynchronous access of data area
(Sheng Yang + Andy)
  - Fix target_release_cmd_kref shutdown comp leak (Himanshu Madhani)

Also, there is a separate PULL request coming for cxgb4 NIC driver
prerequisites for supporting hw iscsi segmentation offload (ISO),
that will be the base for a number of v4.7 developments involving
iscsi-target hw offloads.

Thank you,

--nab

Andy Grover (1):
  target: Remove enum transport_lunflags_table

Christoph Hellwig (6):
  target: Convert demo-mode only drivers to target_alloc_session
  iser-target: Remove ISER_RECV_DATA_SEG_LEN
  iser-target: Split and properly type the login buffer
  iser-target: Convert to new CQ API
  iser-target: Kill struct isert_rdma_wr
  iser-target: Kill the ->isert_cmd back pointer in struct iser_tx_desc

Himanshu Madhani (1):
  target: Fix target_release_cmd_kref shutdown comp leak

Jenny Derzhavetz (6):
  iser-target: Fix identification of login rx descriptor type
  iser-target: Add new state ISER_CONN_BOUND to isert_conn
  iser-target: Separate flows for np listeners and connections cma
events
  iser-target: Rework connection termination
  iser-target: Remove redundant wait in release_conn
  iser-target: Remove impossible condition from isert_wait_conn

Mike Christie (1):
  target/iblock: pass WRITE_SAME to device if possible

Nicholas Bellinger (15):
  target: Add target_alloc_session() helper function
  vhost/scsi: Convert to target_alloc_session usage
  tcm_qla2xxx: Convert to target_alloc_session usage
  tcm_fc: Convert to target_alloc_session usage
  ib_srpt: Convert to target_alloc_session usage
  sbp-target: Conversion to percpu_ida tag pre-allocation
  sbp-target: Convert to TARGET_SCF_ACK_KREF I/O krefs
  usb-gadget/tcm: Conversion to percpu_ida tag pre-allocation
  usb-gadget/tcm: Convert to TARGET_SCF_ACK_KREF I/O krefs
  xen-scsiback: Convert to percpu_ida tag allocation
  xen-scsiback: Convert to TARGET_SCF_ACK_KREF I/O krefs
  tcm_fc: Convert to TARGET_SCF_ACK_KREF I/O + TMR krefs
  ib_srpt: Convert to percpu_ida tag allocation
  target/user: Fix size_t format-spec build warning
  target: Avoid DataIN transfers for non-GOOD SAM status

Quinn Tran (1):
  qla2xxx: Add DebugFS node for target sess list.

Sheng Yang (5):
  target/user: Use iovec[] to describe continuous area
  target/user: Free data ring in unified function
  target/user: Introduce data_bitmap, replace
data_length/data_head/data_tail
  target/user: Don't free expired command when time out
  target/user: Report capability of handling out-of-order completions to
userspace

 Documentation/target/tcmu-design.txt |  11 +-
 drivers/infiniband/ulp/isert/ib_isert.c  | 810 ---
 drivers/infiniband/ulp/isert/ib_isert.h  |  72 +--
 drivers/infiniband/ulp/srpt/ib_srpt.c|  76 +--
 drivers/infiniband/ulp/srpt/ib_srpt.h|   2 -
 drivers/scsi/qla2xxx/qla_def.h   |   1 +
 drivers/scsi/qla2xxx/qla_dfs.c   |  55 ++
 drivers/scsi/qla2xxx/qla_target.c|  57 +-
 drivers/scsi/qla2xxx/qla_target.h|   2 +-
 drivers/scsi/qla2xxx/tcm_qla2xxx.c   |  79 +--
 drivers/target/loopback/tcm_loop.c   |  46 +-
 drivers/target/sbp/sbp_target.c  |  95 ++--
 drivers/target/target_core_device.c  |  41 +-
 drivers/target/target_core_fabric_configfs.c |  32 +-
 drivers/target/target_core_iblock.c  |  34 ++
 drivers/target/target_core_internal.h|   8 +-
 drivers/target/target_core_spc.c |   3 +-
 drivers/target/target_core_tpg.c |  21 +-
 drivers/target/target_core_transport.c   |  66 ++-
 drivers/target/target_core_user.c| 267 +
 drivers/target/tcm_fc/tfc_cmd.c  |  20 +-
 drivers/target/tcm_fc/tfc_sess.c |  44 +-
 drivers/usb/gadget/function

Re: [PATCHv3] scsi: disable automatic target scan

2016-03-22 Thread Hannes Reinecke
On 03/21/2016 09:40 PM, Laurence Oberman wrote:
> Hello Hannes
> 
> Please share latest scripts and an example of how you are using them.
> I have some scripts from last November, that you posted but I am sure they 
> have changed.
> If not then I will modify them as appropriate, just let me know.
> 
> I have added the patches and booted the system set to async, so before I boot 
> with
> scsi_mod.scan=manual want to prepare my test system.
> This feature may be a very useful feature we would want to include in RHEL as 
> we
> struggle with large LUN boot configurations all the time.

Oh, really? Hardly a surprise.
In fact, I'm constantly surprised that RHEL ships with systemd
enabled, apparently without any issues.
Especially in large machines.
As we found to our dismay; I spent about a year to get things
running smoothly.
Or, to be precise, running at all.

Anyway.

Attached is the original patch to dracut; please check if it's the
correct version (I've changed the 'disabled' to 'manual' for
upstream submission).

I haven't included the systemd service; that's actually quite
trivial: reset the scan mode (as it's now writeable) and call
rescan-scsi-bus.sh or equivalent.

But please add a 'Tested-by' or equivalent to the upstream patch, to
add a bit more urgency to it.

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
From 1d427bcf3527ba96828a4bd5231409967ff096b4 Mon Sep 17 00:00:00 2001
From: Hannes Reinecke 
Date: Wed, 11 Nov 2015 12:20:33 +0100
Subject: [PATCH] 95lunmask: Add module to handle LUN masking

Using the module option 'scsi_mod.scan=disabled'
this implements LUN masking by selectively enable only those
devices required for booting.

References: bsc#954600

Signed-off-by: Hannes Reinecke 
---
 modules.d/95lunmask/fc_transport_scan_lun.sh  | 26 ++
 modules.d/95lunmask/module-setup.sh   | 70 +++
 modules.d/95lunmask/parse-lunmask.sh  | 40 +++
 modules.d/95lunmask/sas_transport_scan_lun.sh | 26 ++
 4 files changed, 162 insertions(+)
 create mode 100755 modules.d/95lunmask/fc_transport_scan_lun.sh
 create mode 100755 modules.d/95lunmask/module-setup.sh
 create mode 100755 modules.d/95lunmask/parse-lunmask.sh
 create mode 100755 modules.d/95lunmask/sas_transport_scan_lun.sh

diff --git a/modules.d/95lunmask/fc_transport_scan_lun.sh b/modules.d/95lunmask/fc_transport_scan_lun.sh
new file mode 100755
index 000..d9f84a3
--- /dev/null
+++ b/modules.d/95lunmask/fc_transport_scan_lun.sh
@@ -0,0 +1,26 @@
+#!/bin/bash
+#
+# fc_transport_lun_scan
+#
+# Selectively enable individual LUNs behind an FC remote port
+#
+# ACTION=="add", SUBSYSTEM=="fc_transport", ATTR{port_name}=="wwpn", \
+#PROGRAM="fc_transport_lun_scan lun"
+#
+
+[ -z $DEVPATH ] && exit 1
+
+if [ -n "$1" ] ; then
+LUN=$1
+else
+LUN=-
+fi
+ID=${DEVPATH##*/rport-}
+HOST=${ID%%:*}
+CHANNEL=${ID%%-*}
+CHANNEL=${CHANNEL#*:}
+if [ -f /sys$DEVPATH/scsi_target_id ] ; then
+TARGET=$(cat /sys$DEVPATH/scsi_target_id)
+fi
+[ -z "$TARGET" ] && exit 1
+echo $CHANNEL $TARGET $LUN > /sys/class/scsi_host/host$HOST/scan
diff --git a/modules.d/95lunmask/module-setup.sh b/modules.d/95lunmask/module-setup.sh
new file mode 100755
index 000..c88b1c0
--- /dev/null
+++ b/modules.d/95lunmask/module-setup.sh
@@ -0,0 +1,70 @@
+#!/bin/bash
+# -*- mode: shell-script; indent-tabs-mode: nil; sh-basic-offset: 4; -*-
+# ex: ts=8 sw=4 sts=4 et filetype=sh
+
+# called by dracut
+cmdline() {
+get_lunmask() {
+local _dev=$1
+local _devpath=$(cd -P /sys/dev/block/$_dev ; echo $PWD)
+local _sdev _lun _rport _end_device _classdev _wwpn _sas_address
+
+[ "${_devpath#*/sd}" == "$_devpath" ] && return 1
+_sdev="${_devpath%%/block/*}"
+_lun="${_sdev##*:}"
+# Check for FibreChannel
+_rport="${_devpath##*/rport-}"
+if [ "$_rport" != "$_devpath" ] ; then
+_rport="${_rport%%/*}"
+_classdev="/sys/class/fc_remote_ports/rport-${_rport}"
+[ -d "$_classdev" ] || return 1
+_wwpn=$(cat ${_classdev}/port_name)
+echo "rd.lunmask=fc,${_wwpn},${_lun}"
+return 0
+fi
+# Check for SAS
+_end_device="${_devpath##*/end_device-}"
+if [ "$_end_device" != "$_devpath" ] ; then
+_end_device="${_end_device%%/*}"
+_classdev="/sys/class/sas_device/end_device-${_end_device}"
+[ -e "$_classdev" ] || return 1
+_sas_address=$(cat ${_classdev}/sas_address)
+echo "rd.lunmask=sas,${_sas_address},${_lun}"
+return 0
+fi
+return 1
+}
+[[ $hostonly ]] || [[ $mount_needs ]] && {
+for_each_host_dev_and_slaves_all get_lunmask
+

Re: [PATCH] mpt3sas: Don't overreach ioc->reply_post[] during initialization

2016-03-22 Thread Jens Axboe

On 03/18/2016 01:45 PM, Calvin Owens wrote:

In _base_make_ioc_operational(), we walk ioc->reply_queue_list and pull
a pointer out of successive elements of ioc->reply_post[] for each entry
in that list if RDPQ is enabled.

Since the code pulls the pointer for the next iteration at the bottom of
the loop, it triggers the a KASAN dump on the final iteration:

 BUG: KASAN: slab-out-of-bounds in _base_make_ioc_operational+0x47b7/0x47e0 
[mpt3sas] at addr 880754816ab0
 Read of size 8 by task modprobe/305
 
 Call Trace:
  [] dump_stack+0x4d/0x6c
  [] print_trailer+0xf9/0x150
  [] object_err+0x34/0x40
  [] kasan_report_error+0x221/0x530
  [] __asan_report_load8_noabort+0x43/0x50
  [] _base_make_ioc_operational+0x47b7/0x47e0 [mpt3sas]
  [] mpt3sas_base_attach+0x1991/0x2120 [mpt3sas]
  [] _scsih_probe+0xeb3/0x16b0 [mpt3sas]
  [] local_pci_probe+0xc7/0x170
  [] pci_device_probe+0x20f/0x290
  [] really_probe+0x17d/0x600
  [] __driver_attach+0x153/0x190
  [] bus_for_each_dev+0x11c/0x1a0
  [] driver_attach+0x3d/0x50
  [] bus_add_driver+0x44a/0x5f0
  [] driver_register+0x18c/0x3b0
  [] __pci_register_driver+0x156/0x200
  [] _mpt3sas_init+0x135/0x1000 [mpt3sas]
  [] do_one_initcall+0x113/0x2b0
  [] do_init_module+0x1d0/0x4d8
  [] load_module+0x6729/0x8dc0
  [] SYSC_init_module+0x183/0x1a0
  [] SyS_init_module+0xe/0x10
  [] entry_SYSCALL_64_fastpath+0x12/0x6a

Fix this by pulling the value at the beginning of the loop.

Signed-off-by: Calvin Owens 


Looks good to me.

Reviewed-by: Jens Axboe 

--
Jens Axboe

--
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] scsi: ufs: select CONFIG_NLS

2016-03-22 Thread Arnd Bergmann
On Tuesday 22 March 2016 07:43:19 James Bottomley wrote:
> > diff --git a/drivers/scsi/ufs/Kconfig b/drivers/scsi/ufs/Kconfig
> > index 5f4530744e0a..097894a1fab5 100644
> > --- a/drivers/scsi/ufs/Kconfig
> > +++ b/drivers/scsi/ufs/Kconfig
> > @@ -37,6 +37,7 @@ config SCSI_UFSHCD
> >   depends on SCSI && SCSI_DMA
> >   select PM_DEVFREQ
> >   select DEVFREQ_GOV_SIMPLE_ONDEMAND
> > + select NLS
> 
> This looks like a bad solution: CONFIG_NLS is nothing more than a menu
> selector for the NLS subsystem.

It's a bit of both: CONFIG_NLS by itself controls the compilation of
the fs/nls/nls_base.c file, which has the definition of the utf16s_to_utf8s
function called by this driver. The same file also contains the
NLS subsystem with the register_nls/unregister_nls/unload_nls/load_nls
APIs that are used by the other files.

>  The problem is that selecting it will
> allow a kernel to be build with NLS and without NLS_DEFAULT which is
> going to cause all sorts of interesting problems on boot.  I think you
> really mean depends on NLS here.

I agree that 'depends on NLS' makes more sense here, however I was
just following what all other users of the API are doing. Mixing
'select' and 'depends on' can easily lead to circular dependencies,
and I was trying to avoid that.

A cleaner solution is probably to split out the utf16 access functions
from the NLS subsystem, and have all these files that currently
'select NLS' pick that other interface (or both, if necessary) instead:

drivers/acpi/device_sysfs.c
drivers/hid/hid-cp2112.c
drivers/hv/hv_fcopy.c
drivers/hv/hv_kvp.c
drivers/net/fjes/fjes_main.c
drivers/net/hyperv/rndis_filter.c
drivers/pci/pci-label.c
drivers/scsi/ufs/ufshcd.c
drivers/usb/core/message.c
drivers/usb/gadget/configfs.c
drivers/usb/gadget/u_os_desc.h
drivers/usb/gadget/usbstring.c
fs/cifs/cifs_unicode.c
fs/fat/dir.c
fs/fat/namei_vfat.c
fs/isofs/joliet.c

Not sure if that's worth the work though.

Arnd
--
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] sd: fixup capacity calculation for 4k drives

2016-03-22 Thread Ewan D. Milne
On Tue, 2016-03-22 at 08:14 +0100, Hannes Reinecke wrote:
> On 03/22/2016 02:16 AM, Martin K. Petersen wrote:
> >> "Hannes" == Hannes Reinecke  writes:
> > 
> > Hannes> in sd_read_capacity() the sdkp->capacity field changes its
> > Hannes> meaning: after the call to read_capacity_XX() it carries the
> > Hannes> _unscaled_ values, making the comparison between the original
> > Hannes> value and the new value always false for drives with a sector
> > Hannes> size != 512.  So introduce a 'new_capacity' carrying the new,
> > Hannes> scaled, capacity.
> > 
> > I agree with Christoph.
> > 
> > How about something like this instead?
> > 
> I've coded it somewhat different, but this one works as well.
> But please modify the description in sd.h, as with this patch
> 'sdkp->capacity' is the _unscaled_ value.
> Might lead to confusion otherwise.
> 
> diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
> index 5f2a84a..5ed7434 100644
> --- a/drivers/scsi/sd.h
> +++ b/drivers/scsi/sd.h
> @@ -65,7 +65,7 @@ struct scsi_disk {
> struct device   dev;
> struct gendisk  *disk;
> atomic_topeners;
> -   sector_tcapacity;   /* size in 512-byte sectors */
> +   sector_tcapacity;   /* size in logical sectors */
> u32 max_xfer_blocks;
> u32 opt_xfer_blocks;
> u32 max_ws_blocks;
> 
> (Apologies for the mangled patch)
> 
> Cheers,
> 
> Hannes

If we change the meaning of the scsi_disk->capacity field to be logical
sectors, we also have to change sd_getgeo() in sd.c, do we not?

The ->capacity field is also accessed by last_sector_hacks() in
drivers/usb/storage/transport.c, although it kind of looks like
that code might not have been correct for 4K sectors with the
scaled value.

-Ewan






--
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] scsi: ufs: select CONFIG_NLS

2016-03-22 Thread James Bottomley
On Thu, 2016-03-17 at 13:29 +0100, Arnd Bergmann wrote:
> A recent change to ufshcd introduced a call to utf16s_to_utf8s,
> a function that is provided by the NLS module, so we get a link
> error when that is not present:
> 
> drivers/scsi/built-in.o: In function `ufshcd_read_string_desc':
> :(.text+0x124d0): undefined reference to `utf16s_to_utf8s'
> 
> This adds a Kconfig 'select' statement to avoid the build error.
> 
> Signed-off-by: Arnd Bergmann 
> Fixes: b573d484e4ff ("scsi: ufs: add support to read device and
> string descriptors")
> ---
>  drivers/scsi/ufs/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/scsi/ufs/Kconfig b/drivers/scsi/ufs/Kconfig
> index 5f4530744e0a..097894a1fab5 100644
> --- a/drivers/scsi/ufs/Kconfig
> +++ b/drivers/scsi/ufs/Kconfig
> @@ -37,6 +37,7 @@ config SCSI_UFSHCD
>   depends on SCSI && SCSI_DMA
>   select PM_DEVFREQ
>   select DEVFREQ_GOV_SIMPLE_ONDEMAND
> + select NLS

This looks like a bad solution: CONFIG_NLS is nothing more than a menu
selector for the NLS subsystem.  The problem is that selecting it will
allow a kernel to be build with NLS and without NLS_DEFAULT which is
going to cause all sorts of interesting problems on boot.  I think you
really mean depends on NLS here.

James


--
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_sas: fix missing { }, nullify kbuff_arr[i] only when necessary

2016-03-22 Thread Sumit Saxena
> -Original Message-
> From: Colin King [mailto:colin.k...@canonical.com]
> Sent: Sunday, March 20, 2016 10:34 PM
> To: Kashyap Desai; Sumit Saxena; James Bottomley; Martin K . Petersen;
> megaraidlinux@avagotech.com; linux-scsi@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> Subject: [PATCH] megaraid_sas: fix missing { }, nullify kbuff_arr[i]
only when
> necessary
>
> From: Colin Ian King 
>
> Fix missing { } on if statement, this change will nullify kbuff_arr[i]
only where
> necessary as the code intended.
>
> detected using static analysis with smatch
>
> Signed-off-by: Colin Ian King 
> ---
>  drivers/scsi/megaraid/megaraid_sas_base.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c
> b/drivers/scsi/megaraid/megaraid_sas_base.c
> index 69d375b..e6ebc7a 100644
> --- a/drivers/scsi/megaraid/megaraid_sas_base.c
> +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
> @@ -6656,12 +6656,13 @@ out:
>   }
>
>   for (i = 0; i < ioc->sge_count; i++) {
> - if (kbuff_arr[i])
> + if (kbuff_arr[i]) {
>   dma_free_coherent(&instance->pdev->dev,
>
le32_to_cpu(kern_sge32[i].length),
> kbuff_arr[i],
>
> le32_to_cpu(kern_sge32[i].phys_addr));
>   kbuff_arr[i] = NULL;
> + }
>   }
>
>   megasas_return_cmd(instance, cmd);

Acked-by: Sumit Saxena 

> --
> 2.7.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: [BUG] mpt2sas: driver init fails on kernel >=4.2 for 9211-8i IT

2016-03-22 Thread Sreekanth Reddy
Hi Matthias,

Thanks for providing detail explanation.

Currently I am trying to reproduce this issue locally. I have used the
same HBA card but still I am not able to reproduce this issue on 4.5
kernel.

I will try for few more times and I will also try to get the diff
between 4.1 to 4.5 kernels w.r.t msix vectors support.

Meanwhile can you please provide me the output of "lspci -vvv" command
for LSI SAS2 HBA's and also hexdump of config sysfs parameter for SAS2
pci device. I need this data to be collected before "timeout" print is
observed, or in the mpt3sas_base.c file and in the
mpt3sas_base_attach(). return from mpt3sas_base_attach() function if
return value of mpt3sas_base_map_resources() is non zero instead of
freeing the resources, as shown below

---
r = mpt3sas_base_map_resources(ioc);
if (r)
- goto out_free_resources;
+return r;
-

Also are you sure that this issue won't occur on less than 4.1 kernel?

Thanks,
Sreekanth

On Mon, Mar 21, 2016 at 10:00 PM, Matthias Prager
 wrote:
> Am 21.03.2016 um 16:52 schrieb Matthias Prager:
>> Hi Sreekanth,
>>
>> thanks for digging into this issue. Regarding the 4.5.0 after 4.1.20
>> boot statement, I will try to express myself better:
>>
>> I first started the system with the 4.1.20 kernel. Then I issued an
>> 'init 6' warm-reboot and chose to boot the 4.5.0 kernel. This procedure
>> often works (i.e. I'm able to boot the newer kernels (>=4.2) this way
>> with mpt2sas initializing just fine).
> One small addition: warm-rebooting kernel 4.5.0 from kernel 4.5.0
> reliably leads to this issue manifesting (mpt2sas driver not initializing).
>
>>
>> I will try the msix_disable parameter and report back. Maybe kernel
>> version 4.2 broke interrupt handling somehow?
> With the msix_disable parameter set kernels 4.5.0, 4.4.5, 4.3.3 and
> 4.2.1 boot without the mpt2sas init issue. I also tried the pci=nomsi
> paremeter instead, which also works fine.
>
> The igb ethernet-driver issue I mentioned as possibly related has not
> changed with pci=nomsi. So I'm assuming it is not the same issue, but
> possibly related, since it also occurs on kernels >=4.2.
>
>>
>> ---
>> Matthias
>>
>>
>> Am 21.03.2016 um 14:59 schrieb Sreekanth Reddy:
>>> Hi Matthias,
>>>
>>> Thanks for providing the logs. In these logs, I am not observing any
>>> such a prints which should be suspected for this issue.
>>>
>>> Can you please try once by setting mpt3sas driver's "msix_disable"
>>> module parameter to one.
>>>
>>> Also, can you please elaborate below statement for me, I am not able
>>> understand this statement
>>>  "I managed to boot the same 4.5.0 kernel successfully after warm
>>> rebooting from 4.1.20"
>>>
>>> Thanks,
>>> Sreekanth
>>>
>>> On Mon, Mar 21, 2016 at 2:48 PM, Matthias Prager
>>>  wrote:
 Hi Sreekanth,

 thank you for replying so quickly. Here are the logs you requested
 (kernel 4.5.0):

> [2.083177] mpt3sas version 09.102.00.00 loaded
> [2.083757] mpt2sas_cm0: mpt3sas_base_attach
> [2.083956] mpt2sas_cm0: mpt3sas_base_map_resources
> [2.084708] mpt2sas_cm0: 32 BIT PCI BUS DMA ADDRESSING SUPPORTED, 
> total mem (3074748 kB)
> [2.084964] mpt2sas_cm0: _base_get_ioc_facts
> [2.085154] mpt2sas_cm0: _base_wait_for_iocstate
> [2.140893]offset:data
> [2.141082][0x00]:03100200
> [2.141257][0x04]:2300
> [2.141432][0x08]:
> [2.141606][0x0c]:
> [2.141780][0x10]:
> [2.141955][0x14]:00010480
> [2.142129][0x18]:22130d68
> [2.142303][0x1c]:0001285c
> [2.142477][0x20]:14000400
> [2.142651][0x24]:0020
> [2.142825][0x28]:010f
> [2.143000][0x2c]:000c000b
> [2.143174][0x30]:003c0003
> [2.143349][0x34]:0020ffe0
> [2.143544][0x38]:00800122
> [2.143719][0x3c]:0009
> [2.143895] mpt2sas_cm0: hba queue depth(3432), max chains per io(128)
> [2.144106] mpt2sas_cm0: request frame size(128), reply frame size(128)
> [2.144397] mpt2sas_cm0: msix is supported, vector_count(1)
> [2.144600] mpt2sas_cm0: MSI-X vectors supported: 1, no of cores: 4, 
> max_msix_vectors: -1
> [2.145161] mpt2sas0-msix0: PCI-MSI-X enabled: IRQ 58
> [2.145361] mpt2sas_cm0: iomem(0xfd4fc000), 
> mapped(0xc90d), size(16384)
> [2.145591] mpt2sas_cm0: ioport(0x4000), size(256)
> [2.146206] mpt2sas_cm0: _base_get_ioc_facts
> [2.146397] mpt2sas_cm0: _base_wait_for_iocstate
> [2.202087]offset:data
> [2.202281][0x00]:03100200
> [

RE: [PATCH] mpt3sas: Don't overreach ioc->reply_post[] during initialization

2016-03-22 Thread Chaitra Basappa
Martin,
 This patch is being reviewed , we shall get back with reviews by
tomorrow.

Thanks,
 Chaitra

-Original Message-
From: Martin K. Petersen [mailto:martin.peter...@oracle.com]
Sent: Tuesday, March 22, 2016 6:00 AM
To: Calvin Owens
Cc: Sathya Prakash; Chaitra P B; Suganath Prabu Subramani; James E.J.
Bottomley; Martin K. Petersen; mpt-fusionlinux@broadcom.com;
linux-scsi@vger.kernel.org; linux-ker...@vger.kernel.org;
kernel-t...@fb.com; Sreekanth Reddy
Subject: Re: [PATCH] mpt3sas: Don't overreach ioc->reply_post[] during
initialization

> "Calvin" == Calvin Owens  writes:

Calvin> In _base_make_ioc_operational(), we walk ioc->reply_queue_list
Calvin> and pull a pointer out of successive elements of
Calvin> ioc->reply_post[] for each entry in that list if RDPQ is
Calvin> enabled.

Calvin> Since the code pulls the pointer for the next iteration at the
Calvin> bottom of the loop, it triggers the a KASAN dump on the final
Calvin> iteration:

Broadcom folks, please review.

Thanks!

-- 
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 0/3] scsi: reduce CONFIG_SCSI_CONSTANTS=y impact by 8k

2016-03-22 Thread Christoph Hellwig
On Tue, Mar 08, 2016 at 08:52:32PM +0100, Rasmus Villemoes wrote:
> On Tue, Nov 24 2015, Rasmus Villemoes  wrote:
> 
> > This reduces the impact of choosing CONFIG_SCSI_CONSTANTS by about 8KB.
> >
> 
> ping? does anyone feel like picking these up?

Please resend them against the current tree.  I'm pretty sure I acked
your appraoch, and if I didn't give you an actualy review I'll happily
do that after a repost.
--
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] sd: fixup capacity calculation for 4k drives

2016-03-22 Thread Hannes Reinecke
On 03/22/2016 02:16 AM, Martin K. Petersen wrote:
>> "Hannes" == Hannes Reinecke  writes:
> 
> Hannes> in sd_read_capacity() the sdkp->capacity field changes its
> Hannes> meaning: after the call to read_capacity_XX() it carries the
> Hannes> _unscaled_ values, making the comparison between the original
> Hannes> value and the new value always false for drives with a sector
> Hannes> size != 512.  So introduce a 'new_capacity' carrying the new,
> Hannes> scaled, capacity.
> 
> I agree with Christoph.
> 
> How about something like this instead?
> 
I've coded it somewhat different, but this one works as well.
But please modify the description in sd.h, as with this patch
'sdkp->capacity' is the _unscaled_ value.
Might lead to confusion otherwise.

diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
index 5f2a84a..5ed7434 100644
--- a/drivers/scsi/sd.h
+++ b/drivers/scsi/sd.h
@@ -65,7 +65,7 @@ struct scsi_disk {
struct device   dev;
struct gendisk  *disk;
atomic_topeners;
-   sector_tcapacity;   /* size in 512-byte sectors */
+   sector_tcapacity;   /* size in logical sectors */
u32 max_xfer_blocks;
u32 opt_xfer_blocks;
u32 max_ws_blocks;

(Apologies for the mangled patch)

Cheers,

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