[PATCH] scsi/aacraid: user upper/lower_address() macro to avoid "right shift count >= width of type" warning.

2012-11-21 Thread Sebastian Andrzej Siewior
Avoids the following warning:
|drivers/scsi/aacraid/src.c: In function 'aac_src_deliver_message':
|drivers/scsi/aacraid/src.c:410:3: warning: right shift count >= width of type 
[enabled by default]
|BUG_ON((u32)(address >> 32) != 0L);
|^
|drivers/scsi/aacraid/src.c:434:2: warning: right shift count >= width of type 
[enabled by default]
|   src_writel(dev, MUnit.IQ_H, (address >> 32) & 0x);

Signed-off-by: Sebastian Andrzej Siewior 
---
 drivers/scsi/aacraid/src.c |6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/aacraid/src.c b/drivers/scsi/aacraid/src.c
index 3b021ec..4f6388e 100644
--- a/drivers/scsi/aacraid/src.c
+++ b/drivers/scsi/aacraid/src.c
@@ -407,7 +407,7 @@ static int aac_src_deliver_message(struct fib *fib)
fib->hw_fib_va->header.StructType = FIB_MAGIC2;
fib->hw_fib_va->header.SenderFibAddress = (u32)address;
fib->hw_fib_va->header.u.TimeStamp = 0;
-   BUG_ON((u32)(address >> 32) != 0L);
+   BUG_ON((upper_32_bits(address) != 0L));
address |= fibsize;
} else {
/* Calculate the amount to the fibsize bits */
@@ -431,8 +431,8 @@ static int aac_src_deliver_message(struct fib *fib)
address |= fibsize;
}
 
-   src_writel(dev, MUnit.IQ_H, (address >> 32) & 0x);
-   src_writel(dev, MUnit.IQ_L, address & 0x);
+   src_writel(dev, MUnit.IQ_H, upper_32_bits(address));
+   src_writel(dev, MUnit.IQ_L, lower_32_bits(address));
 
return 0;
 }
-- 
1.7.10.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 0/2] Change type-2 dif to use rq embedded 32 byte cdb

2012-11-21 Thread Rob Evers
These patches replace the original t10 type 2 dif mempool implementation
by increasing the block and scsi cdb maximum sizes from 16 to 32 bytes.
The cdb embedded in the request structure can then be used for type 2
dif commands, or other 32 byte cdbs as required.

Motivation for this is that type-2 dif commands should be treated as
any other read/writes generally, without any performance penalty.

This patch set conflicts with a patch previously posted which addresses
a race in UA induced retries with type 2 dif commands.  These patches also
address that problem:

http://marc.info/?l=linux-scsi&m=135186352200668&w=2

Testing:

Readily reproduced the race condition panic by simultaneously
inducing UAs while load testing 8 scsi_debug devices.  With patch
applied, same testing ran for 15 hours without a panic.

Sanity tested performance using scsi_debug, with and without
type 2 dif enabled.  Order 80% performance increase noticed
with patches when scsi_debug was running in dif type 2 mode.
Running scsi_debug without type 2 dif, performance was approximately
equal.

Used 3.7.0-rc5 for testing and patch generation.

root (2):
  Change the cdb size limits in block and scsi to 32 bytes
  Change dif type 2 commands to use embedded 32 byte cdb

 drivers/scsi/sd.c| 45 +
 drivers/scsi/sd.h|  5 -
 include/linux/blkdev.h   |  2 +-
 include/scsi/scsi_cmnd.h | 13 ++---
 4 files changed, 4 insertions(+), 61 deletions(-)

-- 
1.7.11.7
--
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] Change dif type 2 commands to use embedded 32 byte cdb

2012-11-21 Thread Rob Evers
The original implementation of type 2 dif in sd.c used
a mempool to allocate scsi cdbs that were 32 bytes in
length.

With the previous change in this set, 32 byte cdbs are
available in the request structure, removing the need
for the mempool.

Also fixed up a comment regarding MAX_COMMAND_SIZE

Signed-off-by: Rob Evers 
---
 drivers/scsi/sd.c| 45 +
 drivers/scsi/sd.h|  5 -
 include/scsi/scsi_cmnd.h | 11 +--
 3 files changed, 2 insertions(+), 59 deletions(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 12f6fdf..f72ae4a 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -122,9 +122,6 @@ static DEFINE_IDA(sd_index_ida);
  * object after last put) */
 static DEFINE_MUTEX(sd_ref_mutex);
 
-static struct kmem_cache *sd_cdb_cache;
-static mempool_t *sd_cdb_pool;
-
 static const char *sd_cache_types[] = {
"write through", "none", "write back",
"write back, no read (daft)"
@@ -852,14 +849,8 @@ static int sd_prep_fn(struct request_queue *q, struct 
request *rq)
protect = 0;
 
if (host_dif == SD_DIF_TYPE2_PROTECTION) {
-   SCpnt->cmnd = mempool_alloc(sd_cdb_pool, GFP_ATOMIC);
-
-   if (unlikely(SCpnt->cmnd == NULL)) {
-   ret = BLKPREP_DEFER;
-   goto out;
-   }
 
-   SCpnt->cmd_len = SD_EXT_CDB_SIZE;
+   SCpnt->cmd_len = MAX_COMMAND_SIZE;
memset(SCpnt->cmnd, 0, SCpnt->cmd_len);
SCpnt->cmnd[0] = VARIABLE_LENGTH_CMD;
SCpnt->cmnd[7] = 0x18;
@@ -1547,21 +1538,6 @@ static int sd_done(struct scsi_cmnd *SCpnt)
if (rq_data_dir(SCpnt->request) == READ && scsi_prot_sg_count(SCpnt))
sd_dif_complete(SCpnt, good_bytes);
 
-   if (scsi_host_dif_capable(sdkp->device->host, sdkp->protection_type)
-   == SD_DIF_TYPE2_PROTECTION && SCpnt->cmnd != SCpnt->request->cmd) {
-
-   /* We have to print a failed command here as the
-* extended CDB gets freed before scsi_io_completion()
-* is called.
-*/
-   if (result)
-   scsi_print_command(SCpnt);
-
-   mempool_free(SCpnt->cmnd, sd_cdb_pool);
-   SCpnt->cmnd = NULL;
-   SCpnt->cmd_len = 0;
-   }
-
return good_bytes;
 }
 
@@ -2964,24 +2940,8 @@ static int __init init_sd(void)
if (err)
goto err_out_class;
 
-   sd_cdb_cache = kmem_cache_create("sd_ext_cdb", SD_EXT_CDB_SIZE,
-0, 0, NULL);
-   if (!sd_cdb_cache) {
-   printk(KERN_ERR "sd: can't init extended cdb cache\n");
-   goto err_out_class;
-   }
-
-   sd_cdb_pool = mempool_create_slab_pool(SD_MEMPOOL_SIZE, sd_cdb_cache);
-   if (!sd_cdb_pool) {
-   printk(KERN_ERR "sd: can't init extended cdb pool\n");
-   goto err_out_cache;
-   }
-
return 0;
 
-err_out_cache:
-   kmem_cache_destroy(sd_cdb_cache);
-
 err_out_class:
class_unregister(&sd_disk_class);
 err_out:
@@ -3001,9 +2961,6 @@ static void __exit exit_sd(void)
 
SCSI_LOG_HLQUEUE(3, printk("exit_sd: exiting sd driver\n"));
 
-   mempool_destroy(sd_cdb_pool);
-   kmem_cache_destroy(sd_cdb_cache);
-
scsi_unregister_driver(&sd_template.gendrv);
class_unregister(&sd_disk_class);
 
diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
index 47c52a6..eec4154 100644
--- a/drivers/scsi/sd.h
+++ b/drivers/scsi/sd.h
@@ -34,11 +34,6 @@
 #define SD_LAST_BUGGY_SECTORS  8
 
 enum {
-   SD_EXT_CDB_SIZE = 32,   /* Extended CDB size */
-   SD_MEMPOOL_SIZE = 2,/* CDB pool size */
-};
-
-enum {
SD_LBP_FULL = 0,/* Full logical block provisioning */
SD_LBP_UNMAP,   /* Use UNMAP command */
SD_LBP_WS16,/* Use WRITE SAME(16) with UNMAP bit */
diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index 9890d29..8f55189 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -13,16 +13,7 @@ struct scsi_device;
 struct scsi_driver;
 
 /*
- * MAX_COMMAND_SIZE is:
- * The longest fixed-length SCSI CDB as per the SCSI standard.
- * fixed-length means: commands that their size can be determined
- * by their opcode and the CDB does not carry a length specifier, (unlike
- * the VARIABLE_LENGTH_CMD(0x7f) command). This is actually not exactly
- * true and the SCSI standard also defines extended commands and
- * vendor specific commands that can be bigger than 16 bytes. The kernel
- * will support these using the same infrastructure used for VARLEN CDB's.
- * So in effect MAX_COMMAND_SIZE means the maximum size command scsi-ml
- * supports without specifying a cmd_len by ULD's
+ * MAX_COMMAND_SIZE is the max size in bytes, a cdb can be.
  */
 #define MAX_COMMAND_SIZE 32
 #if (MAX_COMMAND_SIZE > BLK_MA

[PATCH 1/2] Change the cdb size limits in block and scsi to 32 bytes

2012-11-21 Thread Rob Evers

Signed-off-by: Rob Evers 
---
 include/linux/blkdev.h   | 2 +-
 include/scsi/scsi_cmnd.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 1756001..cafce6e 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -85,7 +85,7 @@ enum rq_cmd_type_bits {
REQ_TYPE_ATA_PC,
 };
 
-#define BLK_MAX_CDB16
+#define BLK_MAX_CDB32
 
 /*
  * try to put the fields that are referenced together in the same cacheline.
diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index de5f5d8..9890d29 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -24,7 +24,7 @@ struct scsi_driver;
  * So in effect MAX_COMMAND_SIZE means the maximum size command scsi-ml
  * supports without specifying a cmd_len by ULD's
  */
-#define MAX_COMMAND_SIZE 16
+#define MAX_COMMAND_SIZE 32
 #if (MAX_COMMAND_SIZE > BLK_MAX_CDB)
 # error MAX_COMMAND_SIZE can not be bigger than BLK_MAX_CDB
 #endif
-- 
1.7.11.7

--
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] UAPI: (Scripted) Disintegrate include/rdma

2012-11-21 Thread David Howells
Signed-off-by: David Howells 
Acked-by: Arnd Bergmann 
Acked-by: Thomas Gleixner 
Acked-by: Michael Kerrisk 
Acked-by: Paul E. McKenney 
Acked-by: Dave Jones 
---

 include/rdma/Kbuild   |6 --
 include/rdma/rdma_netlink.h   |   36 +---
 include/uapi/rdma/Kbuild  |6 ++
 include/uapi/rdma/ib_user_cm.h|0 
 include/uapi/rdma/ib_user_mad.h   |0 
 include/uapi/rdma/ib_user_sa.h|0 
 include/uapi/rdma/ib_user_verbs.h |0 
 include/uapi/rdma/rdma_netlink.h  |   37 +
 include/uapi/rdma/rdma_user_cm.h  |0 
 9 files changed, 44 insertions(+), 41 deletions(-)
 rename include/{rdma/ib_user_cm.h => uapi/rdma/ib_user_cm.h} (100%)
 rename include/{rdma/ib_user_mad.h => uapi/rdma/ib_user_mad.h} (100%)
 rename include/{rdma/ib_user_sa.h => uapi/rdma/ib_user_sa.h} (100%)
 rename include/{rdma/ib_user_verbs.h => uapi/rdma/ib_user_verbs.h} (100%)
 create mode 100644 include/uapi/rdma/rdma_netlink.h
 rename include/{rdma/rdma_user_cm.h => uapi/rdma/rdma_user_cm.h} (100%)

diff --git a/include/rdma/Kbuild b/include/rdma/Kbuild
index ea56f76..e69de29 100644
--- a/include/rdma/Kbuild
+++ b/include/rdma/Kbuild
@@ -1,6 +0,0 @@
-header-y += ib_user_cm.h
-header-y += ib_user_mad.h
-header-y += ib_user_sa.h
-header-y += ib_user_verbs.h
-header-y += rdma_netlink.h
-header-y += rdma_user_cm.h
diff --git a/include/rdma/rdma_netlink.h b/include/rdma/rdma_netlink.h
index bd3d8b2..e38de79 100644
--- a/include/rdma/rdma_netlink.h
+++ b/include/rdma/rdma_netlink.h
@@ -1,41 +1,9 @@
 #ifndef _RDMA_NETLINK_H
 #define _RDMA_NETLINK_H
 
-#include 
-
-enum {
-   RDMA_NL_RDMA_CM = 1
-};
-
-#define RDMA_NL_GET_CLIENT(type) ((type & (((1 << 6) - 1) << 10)) >> 10)
-#define RDMA_NL_GET_OP(type) (type & ((1 << 10) - 1))
-#define RDMA_NL_GET_TYPE(client, op) ((client << 10) + op)
-
-enum {
-   RDMA_NL_RDMA_CM_ID_STATS = 0,
-   RDMA_NL_RDMA_CM_NUM_OPS
-};
-
-enum {
-   RDMA_NL_RDMA_CM_ATTR_SRC_ADDR = 1,
-   RDMA_NL_RDMA_CM_ATTR_DST_ADDR,
-   RDMA_NL_RDMA_CM_NUM_ATTR,
-};
-
-struct rdma_cm_id_stats {
-   __u32   qp_num;
-   __u32   bound_dev_if;
-   __u32   port_space;
-   __s32   pid;
-   __u8cm_state;
-   __u8node_type;
-   __u8port_num;
-   __u8qp_type;
-};
-
-#ifdef __KERNEL__
 
 #include 
+#include 
 
 struct ibnl_client_cbs {
int (*dump)(struct sk_buff *skb, struct netlink_callback *nlcb);
@@ -88,6 +56,4 @@ void *ibnl_put_msg(struct sk_buff *skb, struct nlmsghdr 
**nlh, int seq,
 int ibnl_put_attr(struct sk_buff *skb, struct nlmsghdr *nlh,
  int len, void *data, int type);
 
-#endif /* __KERNEL__ */
-
 #endif /* _RDMA_NETLINK_H */
diff --git a/include/uapi/rdma/Kbuild b/include/uapi/rdma/Kbuild
index aafaa5a..687ae33 100644
--- a/include/uapi/rdma/Kbuild
+++ b/include/uapi/rdma/Kbuild
@@ -1 +1,7 @@
 # UAPI Header export list
+header-y += ib_user_cm.h
+header-y += ib_user_mad.h
+header-y += ib_user_sa.h
+header-y += ib_user_verbs.h
+header-y += rdma_netlink.h
+header-y += rdma_user_cm.h
diff --git a/include/rdma/ib_user_cm.h b/include/uapi/rdma/ib_user_cm.h
similarity index 100%
rename from include/rdma/ib_user_cm.h
rename to include/uapi/rdma/ib_user_cm.h
diff --git a/include/rdma/ib_user_mad.h b/include/uapi/rdma/ib_user_mad.h
similarity index 100%
rename from include/rdma/ib_user_mad.h
rename to include/uapi/rdma/ib_user_mad.h
diff --git a/include/rdma/ib_user_sa.h b/include/uapi/rdma/ib_user_sa.h
similarity index 100%
rename from include/rdma/ib_user_sa.h
rename to include/uapi/rdma/ib_user_sa.h
diff --git a/include/rdma/ib_user_verbs.h b/include/uapi/rdma/ib_user_verbs.h
similarity index 100%
rename from include/rdma/ib_user_verbs.h
rename to include/uapi/rdma/ib_user_verbs.h
diff --git a/include/uapi/rdma/rdma_netlink.h b/include/uapi/rdma/rdma_netlink.h
new file mode 100644
index 000..8297285
--- /dev/null
+++ b/include/uapi/rdma/rdma_netlink.h
@@ -0,0 +1,37 @@
+#ifndef _UAPI_RDMA_NETLINK_H
+#define _UAPI_RDMA_NETLINK_H
+
+#include 
+
+enum {
+   RDMA_NL_RDMA_CM = 1
+};
+
+#define RDMA_NL_GET_CLIENT(type) ((type & (((1 << 6) - 1) << 10)) >> 10)
+#define RDMA_NL_GET_OP(type) (type & ((1 << 10) - 1))
+#define RDMA_NL_GET_TYPE(client, op) ((client << 10) + op)
+
+enum {
+   RDMA_NL_RDMA_CM_ID_STATS = 0,
+   RDMA_NL_RDMA_CM_NUM_OPS
+};
+
+enum {
+   RDMA_NL_RDMA_CM_ATTR_SRC_ADDR = 1,
+   RDMA_NL_RDMA_CM_ATTR_DST_ADDR,
+   RDMA_NL_RDMA_CM_NUM_ATTR,
+};
+
+struct rdma_cm_id_stats {
+   __u32   qp_num;
+   __u32   bound_dev_if;
+   __u32   port_space;
+   __s32   pid;
+   __u8cm_state;
+   __u8node_type;
+   __u8port_num;
+   __u8qp_type;
+};
+
+
+#endif /* _UAPI_RDMA_NETLINK_H */
diff --git a/include/rdma/rdma_user_cm.h b/include/uapi/rdma/rdma_user_cm.h
similarity index 100%
rename from include/rdma/rdma_user_cm.h
rename to include/uapi/rdma/rdma_user

[PATCH] UAPI: (Scripted) Disintegrate include/scsi/fc

2012-11-21 Thread David Howells
Signed-off-by: David Howells 
Acked-by: Arnd Bergmann 
Acked-by: Thomas Gleixner 
Acked-by: Michael Kerrisk 
Acked-by: Paul E. McKenney 
Acked-by: Dave Jones 
---

 include/scsi/fc/Kbuild|4 
 include/uapi/scsi/fc/Kbuild   |4 
 include/uapi/scsi/fc/fc_els.h |0 
 include/uapi/scsi/fc/fc_fs.h  |0 
 include/uapi/scsi/fc/fc_gs.h  |0 
 include/uapi/scsi/fc/fc_ns.h  |0 
 6 files changed, 4 insertions(+), 4 deletions(-)
 rename include/{scsi/fc/fc_els.h => uapi/scsi/fc/fc_els.h} (100%)
 rename include/{scsi/fc/fc_fs.h => uapi/scsi/fc/fc_fs.h} (100%)
 rename include/{scsi/fc/fc_gs.h => uapi/scsi/fc/fc_gs.h} (100%)
 rename include/{scsi/fc/fc_ns.h => uapi/scsi/fc/fc_ns.h} (100%)

diff --git a/include/scsi/fc/Kbuild b/include/scsi/fc/Kbuild
index 5660381..e69de29 100644
--- a/include/scsi/fc/Kbuild
+++ b/include/scsi/fc/Kbuild
@@ -1,4 +0,0 @@
-header-y += fc_els.h
-header-y += fc_fs.h
-header-y += fc_gs.h
-header-y += fc_ns.h
diff --git a/include/uapi/scsi/fc/Kbuild b/include/uapi/scsi/fc/Kbuild
index aafaa5a..5ead9fa 100644
--- a/include/uapi/scsi/fc/Kbuild
+++ b/include/uapi/scsi/fc/Kbuild
@@ -1 +1,5 @@
 # UAPI Header export list
+header-y += fc_els.h
+header-y += fc_fs.h
+header-y += fc_gs.h
+header-y += fc_ns.h
diff --git a/include/scsi/fc/fc_els.h b/include/uapi/scsi/fc/fc_els.h
similarity index 100%
rename from include/scsi/fc/fc_els.h
rename to include/uapi/scsi/fc/fc_els.h
diff --git a/include/scsi/fc/fc_fs.h b/include/uapi/scsi/fc/fc_fs.h
similarity index 100%
rename from include/scsi/fc/fc_fs.h
rename to include/uapi/scsi/fc/fc_fs.h
diff --git a/include/scsi/fc/fc_gs.h b/include/uapi/scsi/fc/fc_gs.h
similarity index 100%
rename from include/scsi/fc/fc_gs.h
rename to include/uapi/scsi/fc/fc_gs.h
diff --git a/include/scsi/fc/fc_ns.h b/include/uapi/scsi/fc/fc_ns.h
similarity index 100%
rename from include/scsi/fc/fc_ns.h
rename to include/uapi/scsi/fc/fc_ns.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


[PATCH] UAPI: (Scripted) Disintegrate include/scsi

2012-11-21 Thread David Howells
Signed-off-by: David Howells 
Acked-by: Arnd Bergmann 
Acked-by: Thomas Gleixner 
Acked-by: Michael Kerrisk 
Acked-by: Paul E. McKenney 
Acked-by: Dave Jones 
---

 include/scsi/Kbuild |3 ---
 include/uapi/scsi/Kbuild|3 +++
 include/uapi/scsi/scsi_bsg_fc.h |0 
 include/uapi/scsi/scsi_netlink.h|0 
 include/uapi/scsi/scsi_netlink_fc.h |0 
 5 files changed, 3 insertions(+), 3 deletions(-)
 rename include/{scsi/scsi_bsg_fc.h => uapi/scsi/scsi_bsg_fc.h} (100%)
 rename include/{scsi/scsi_netlink.h => uapi/scsi/scsi_netlink.h} (100%)
 rename include/{scsi/scsi_netlink_fc.h => uapi/scsi/scsi_netlink_fc.h} (100%)

diff --git a/include/scsi/Kbuild b/include/scsi/Kbuild
index f2b9491..562ff9d 100644
--- a/include/scsi/Kbuild
+++ b/include/scsi/Kbuild
@@ -1,4 +1 @@
-header-y += scsi_netlink.h
-header-y += scsi_netlink_fc.h
-header-y += scsi_bsg_fc.h
 header-y += fc/
diff --git a/include/uapi/scsi/Kbuild b/include/uapi/scsi/Kbuild
index 29a87dd..75746d5 100644
--- a/include/uapi/scsi/Kbuild
+++ b/include/uapi/scsi/Kbuild
@@ -1,2 +1,5 @@
 # UAPI Header export list
 header-y += fc/
+header-y += scsi_bsg_fc.h
+header-y += scsi_netlink.h
+header-y += scsi_netlink_fc.h
diff --git a/include/scsi/scsi_bsg_fc.h b/include/uapi/scsi/scsi_bsg_fc.h
similarity index 100%
rename from include/scsi/scsi_bsg_fc.h
rename to include/uapi/scsi/scsi_bsg_fc.h
diff --git a/include/scsi/scsi_netlink.h b/include/uapi/scsi/scsi_netlink.h
similarity index 100%
rename from include/scsi/scsi_netlink.h
rename to include/uapi/scsi/scsi_netlink.h
diff --git a/include/scsi/scsi_netlink_fc.h 
b/include/uapi/scsi/scsi_netlink_fc.h
similarity index 100%
rename from include/scsi/scsi_netlink_fc.h
rename to include/uapi/scsi/scsi_netlink_fc.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: SCSI LLDs, the SCSI error handler and host resource lifetime

2012-11-21 Thread Bart Van Assche

On 11/21/12 08:19, Hannes Reinecke wrote:

On 11/20/2012 03:24 PM, Bart Van Assche wrote:

If I interpret the SCSI error handler source code correctly then
scsi_unjam_host() may proceed concurrently with scsi_remove_host().
This means that the LLD eh_abort_handler callback may get invoked after
scsi_remove_host() finished. At least the SRP initiator (ib_srp) cleans
up resources necessary for aborting commands as soon as
scsi_remove_host() returns. That looks like a race condition to me. As
far as I can see it is only safe to clean up such resources after the
EH thread has been stopped. Any opinions about adding an additional
callback for this purpose in struct scsi_host_template ?

Note: it doesn't look like a good idea to me to let scsi_remove_host()
wait until error recovery has finished since scsi_remove_host() may get
invoked from the context of a workqueue. If any work gets queued on the
same workqueue related to SCSI error handling letting scsi_remove_host()
wait for the error handler to finish might result in a deadlock.

The patch below is a request for comments patch that does not only add a
callback to struct scsi_host_template but also fixes a (hard to trigger)
race condition in ib_srp: avoid that ib_destroy_cm_id() frees the IB RC
connection while srp_send_tsk_mgmt() is using it.


Hmm.
This would still mean that the eh thread will run until finished.
Which can take _A LOT_ of time (we're speaking hours here).
I would rather have an additional return code in the various
scsi_try_XXX functions to terminate the loop quickly.


How about combining both approaches ? I think the additional callback is 
needed anyway to prevent the race condition explained above. Making the 
SCSI EH stop quicker after scsi_remove_host() has been invoked looks 
like a good idea to me but I'm not sure that change alone is sufficient.


Bart.

--
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 192/493] scsi: remove use of __devinit

2012-11-21 Thread Guennadi Liakhovetski
On Mon, 19 Nov 2012, Bill Pemberton wrote:

>  drivers/scsi/tmscsim.c| 16 +++---

Acked-by: Guennadi Liakhovetski 

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
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/7] Fix race between starved list processing and device removal

2012-11-21 Thread Bart Van Assche
On 11/02/12 07:32, Chanho Min wrote:
>> Yes. Here's the warning.
>> For the trace below, I used scsi_device_get/scsi_device_put() in 
>> scsi_run_queue(). (A little different
>>from your patch). But I think it's the same.
> 
> I think it's correct. cancel_work_sync can sleep. It is caught under 
> CONFIG_DEBUG_ATOMIC_SLEEP.
> What if we only enable irq at cancel_work_sync as the patch bellows?
> 
> diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
> index bb7c482..6e17db9 100644
> --- a/drivers/scsi/scsi_sysfs.c
> +++ b/drivers/scsi/scsi_sysfs.c
> @@ -350,7 +350,9 @@ static void scsi_device_dev_release_usercontext(struct 
> work_struct *work)
>  list_del(&sdev->starved_entry);
>  spin_unlock_irqrestore(sdev->host->host_lock, flags);
>   
> +   local_irq_enable();
>  cancel_work_sync(&sdev->event_work);
> +   local_irq_restore(flags);
>   
>  list_for_each_safe(this, tmp, &sdev->event_list) {
>  struct scsi_event *evt;
> 

As far as I can see this should work but unfortunately this change
creates a nontrivial dependency between scsi_run_queue() and
scsi_device_dev_release_usercontext(). Personally I would prefer
something like this follow-up patch:

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 71bddec..20ea2e9 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -453,15 +453,12 @@ static void scsi_run_queue(struct request_queue *q)
}
 
get_device(&sdev->sdev_gendev);
-   spin_unlock(shost->host_lock);
-
-   spin_lock(sdev->request_queue->queue_lock);
-   __blk_run_queue(sdev->request_queue);
-   spin_unlock(sdev->request_queue->queue_lock);
+   spin_unlock_irqrestore(shost->host_lock, flags);
 
+   blk_run_queue(sdev->request_queue);
put_device(&sdev->sdev_gendev);
 
-   spin_lock(shost->host_lock);
+   spin_lock_irqsave(shost->host_lock, flags);
}
/* put any unprocessed entries back */
list_splice(&starved_list, &shost->starved_list);

Bart.
--
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 V4 1/4] [SCSI] drivers/scsi/ufs: Seggregate PCI Specific Code

2012-11-21 Thread vinayak holikatti
On Wed, Nov 21, 2012 at 4:26 PM, James Bottomley
 wrote:
> On Wed, 2012-11-21 at 16:02 +0530, vinayak holikatti wrote:
>> On Wed, Nov 14, 2012 at 2:56 AM, James Bottomley
>>  wrote:
>> > On Thu, 2012-10-18 at 17:37 +0530, vinayak holikatti wrote:
>> >> I am Vacation will look into it when i am back to work.
>> >> > This doesn't apply on 3.7-rc1. Am I missing any patches in between ?
>> >
>> > OK, so it doesn't apply for me either:
>> >
>> > patching file drivers/scsi/ufs/ufshcd.c
>> > Hunk #11 FAILED at 902.
>> > Hunk #15 succeeded at 1157 with fuzz 1 (offset 2 lines).
>> > Hunk #16 succeeded at 1271 (offset 2 lines).
>> > Hunk #17 succeeded at 1300 (offset 2 lines).
>> > Hunk #18 succeeded at 1364 (offset 2 lines).
>> > Hunk #19 succeeded at 1499 (offset 2 lines).
>> > Hunk #20 succeeded at 1542 (offset 2 lines).
>> > Hunk #21 succeeded at 1678 (offset 2 lines).
>> > Hunk #22 succeeded at 1706 (offset 2 lines).
>> > Hunk #23 succeeded at 1727 (offset 2 lines).
>> > Hunk #24 succeeded at 1775 (offset 2 lines).
>> > Hunk #25 succeeded at 1819 (offset 2 lines).
>> > Hunk #26 succeeded at 1872 (offset 2 lines).
>> > Hunk #27 succeeded at 1898 (offset 2 lines).
>> > Hunk #28 FAILED at 2011.
>> > 2 out of 28 hunks FAILED -- saving rejects to file
>> > drivers/scsi/ufs/ufshcd.c.rej
>> >
>> I have generated patches based on linux/kernel/git/jejb/scsi.git this
>> tree, Also i have
>> confirmed that patches get applied successfully on this git tree.
>>
>> James,
>> Which tree are you refering to. Let me know, which tree i should use to
>> generate the patches.
>
> All patches for the merge window should be against the misc branch of
> the scsi.git tree.
>
> James
>
Ok, I will create the patches against misc branch and resend these patches.
Thanks
Vinayak Holikatti
--
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 192/493] scsi: remove use of __devinit

2012-11-21 Thread Russell King - ARM Linux
On Mon, Nov 19, 2012 at 01:22:21PM -0500, Bill Pemberton wrote:
>  drivers/scsi/arm/acornscsi.c  |  2 +-
>  drivers/scsi/arm/arxescsi.c   |  2 +-
>  drivers/scsi/arm/cumana_1.c   |  2 +-
>  drivers/scsi/arm/cumana_2.c   |  2 +-
>  drivers/scsi/arm/eesox.c  |  2 +-
>  drivers/scsi/arm/oak.c|  2 +-
>  drivers/scsi/arm/powertec.c   |  2 +-

For these,

Acked-by: Russell King 
--
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 097/493] scsi: remove use of __devexit_p

2012-11-21 Thread Russell King - ARM Linux
On Mon, Nov 19, 2012 at 01:20:46PM -0500, Bill Pemberton wrote:
>  drivers/scsi/arm/acornscsi.c  | 2 +-
>  drivers/scsi/arm/arxescsi.c   | 2 +-
>  drivers/scsi/arm/cumana_1.c   | 2 +-
>  drivers/scsi/arm/cumana_2.c   | 2 +-
>  drivers/scsi/arm/eesox.c  | 2 +-
>  drivers/scsi/arm/oak.c| 2 +-
>  drivers/scsi/arm/powertec.c   | 2 +-

For these drivers,

Acked-by: Russell King 
--
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/7] Fix race between starved list processing and device removal

2012-11-21 Thread Bart Van Assche

On 11/02/12 11:48, Bart Van Assche wrote:

[PATCH] Fix race between starved list processing and device removal
[ ... ]
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index ce5224c..2f0f31e 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -348,7 +348,6 @@ static void scsi_device_dev_release_usercontext(struct 
work_struct *work)
starget->reap_ref++;
list_del(&sdev->siblings);
list_del(&sdev->same_target_siblings);
-   list_del(&sdev->starved_entry);
spin_unlock_irqrestore(sdev->host->host_lock, flags);

cancel_work_sync(&sdev->event_work);
@@ -956,6 +955,8 @@ int scsi_sysfs_add_sdev(struct scsi_device *sdev)
  void __scsi_remove_device(struct scsi_device *sdev)
  {
struct device *dev = &sdev->sdev_gendev;
+   struct Scsi_Host *shost = sdev->host;
+   unsigned long flags;

if (sdev->is_visible) {
if (scsi_device_set_state(sdev, SDEV_CANCEL) != 0)
@@ -973,7 +974,13 @@ void __scsi_remove_device(struct scsi_device *sdev)
 * scsi_run_queue() invocations have finished before tearing down the
 * device.
 */
+
scsi_device_set_state(sdev, SDEV_DEL);
+
+   spin_lock_irqsave(shost->host_lock, flags);
+   list_del(&sdev->starved_entry);
+   spin_unlock_irqrestore(shost->host_lock, flags);
+
blk_cleanup_queue(sdev->request_queue);
cancel_work_sync(&sdev->requeue_work);



Please ignore this patch. Even with this patch applied there is still a 
race condition present, namely that the __blk_run_queue() call in 
scsi_run_queue() can get invoked after __scsi_remove_device() invoked 
put_device().


Bart.

--
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 V4 1/4] [SCSI] drivers/scsi/ufs: Seggregate PCI Specific Code

2012-11-21 Thread James Bottomley
On Wed, 2012-11-21 at 16:02 +0530, vinayak holikatti wrote:
> On Wed, Nov 14, 2012 at 2:56 AM, James Bottomley
>  wrote:
> > On Thu, 2012-10-18 at 17:37 +0530, vinayak holikatti wrote:
> >> I am Vacation will look into it when i am back to work.
> >> > This doesn't apply on 3.7-rc1. Am I missing any patches in between ?
> >
> > OK, so it doesn't apply for me either:
> >
> > patching file drivers/scsi/ufs/ufshcd.c
> > Hunk #11 FAILED at 902.
> > Hunk #15 succeeded at 1157 with fuzz 1 (offset 2 lines).
> > Hunk #16 succeeded at 1271 (offset 2 lines).
> > Hunk #17 succeeded at 1300 (offset 2 lines).
> > Hunk #18 succeeded at 1364 (offset 2 lines).
> > Hunk #19 succeeded at 1499 (offset 2 lines).
> > Hunk #20 succeeded at 1542 (offset 2 lines).
> > Hunk #21 succeeded at 1678 (offset 2 lines).
> > Hunk #22 succeeded at 1706 (offset 2 lines).
> > Hunk #23 succeeded at 1727 (offset 2 lines).
> > Hunk #24 succeeded at 1775 (offset 2 lines).
> > Hunk #25 succeeded at 1819 (offset 2 lines).
> > Hunk #26 succeeded at 1872 (offset 2 lines).
> > Hunk #27 succeeded at 1898 (offset 2 lines).
> > Hunk #28 FAILED at 2011.
> > 2 out of 28 hunks FAILED -- saving rejects to file
> > drivers/scsi/ufs/ufshcd.c.rej
> >
> I have generated patches based on linux/kernel/git/jejb/scsi.git this
> tree, Also i have
> confirmed that patches get applied successfully on this git tree.
> 
> James,
> Which tree are you refering to. Let me know, which tree i should use to
> generate the patches.

All patches for the merge window should be against the misc branch of
the scsi.git tree.

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


[PATCH 1/1] iscsi_target: Remove redundant null check before kfree

2012-11-21 Thread Sachin Kamat
kfree on null pointer is a no-op.

Signed-off-by: Sachin Kamat 
---
 drivers/target/iscsi/iscsi_target_login.c |6 ++
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target_login.c 
b/drivers/target/iscsi/iscsi_target_login.c
index 7bf9ccc..fdb632f 100644
--- a/drivers/target/iscsi/iscsi_target_login.c
+++ b/drivers/target/iscsi/iscsi_target_login.c
@@ -1118,10 +1118,8 @@ new_sess_out:
idr_remove(&sess_idr, conn->sess->session_index);
spin_unlock_bh(&sess_idr_lock);
}
-   if (conn->sess->sess_ops)
-   kfree(conn->sess->sess_ops);
-   if (conn->sess)
-   kfree(conn->sess);
+   kfree(conn->sess->sess_ops);
+   kfree(conn->sess);
 old_sess_out:
iscsi_stop_login_thread_timer(np);
/*
-- 
1.7.4.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 V4 1/4] [SCSI] drivers/scsi/ufs: Seggregate PCI Specific Code

2012-11-21 Thread vinayak holikatti
On Wed, Nov 14, 2012 at 2:56 AM, James Bottomley
 wrote:
> On Thu, 2012-10-18 at 17:37 +0530, vinayak holikatti wrote:
>> I am Vacation will look into it when i am back to work.
>> > This doesn't apply on 3.7-rc1. Am I missing any patches in between ?
>
> OK, so it doesn't apply for me either:
>
> patching file drivers/scsi/ufs/ufshcd.c
> Hunk #11 FAILED at 902.
> Hunk #15 succeeded at 1157 with fuzz 1 (offset 2 lines).
> Hunk #16 succeeded at 1271 (offset 2 lines).
> Hunk #17 succeeded at 1300 (offset 2 lines).
> Hunk #18 succeeded at 1364 (offset 2 lines).
> Hunk #19 succeeded at 1499 (offset 2 lines).
> Hunk #20 succeeded at 1542 (offset 2 lines).
> Hunk #21 succeeded at 1678 (offset 2 lines).
> Hunk #22 succeeded at 1706 (offset 2 lines).
> Hunk #23 succeeded at 1727 (offset 2 lines).
> Hunk #24 succeeded at 1775 (offset 2 lines).
> Hunk #25 succeeded at 1819 (offset 2 lines).
> Hunk #26 succeeded at 1872 (offset 2 lines).
> Hunk #27 succeeded at 1898 (offset 2 lines).
> Hunk #28 FAILED at 2011.
> 2 out of 28 hunks FAILED -- saving rejects to file
> drivers/scsi/ufs/ufshcd.c.rej
>
I have generated patches based on linux/kernel/git/jejb/scsi.git this
tree, Also i have
confirmed that patches get applied successfully on this git tree.

James,
Which tree are you refering to. Let me know, which tree i should use to
generate the patches.

> 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


[PATCH] megaraid_sas: fix memory leak if SGL has zero length entries

2012-11-21 Thread Bjørn Mork
commit 98cb7e44 ([SCSI] megaraid_sas: Sanity check user
supplied length before passing it to dma_alloc_coherent())
introduced a memory leak.  Memory allocated for entries
following zero length SGL entries will not be freed.

Reference: http://bugs.debian.org/688198
Cc: 
Signed-off-by: Bjørn Mork 
---
 drivers/scsi/megaraid/megaraid_sas_base.c |   10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c 
b/drivers/scsi/megaraid/megaraid_sas_base.c
index d2c5366..12b6be4 100644
--- a/drivers/scsi/megaraid/megaraid_sas_base.c
+++ b/drivers/scsi/megaraid/megaraid_sas_base.c
@@ -4854,10 +4854,12 @@ megasas_mgmt_fw_ioctl(struct megasas_instance *instance,
sense, sense_handle);
}
 
-   for (i = 0; i < ioc->sge_count && kbuff_arr[i]; i++) {
-   dma_free_coherent(&instance->pdev->dev,
-   kern_sge32[i].length,
-   kbuff_arr[i], kern_sge32[i].phys_addr);
+   for (i = 0; i < ioc->sge_count; i++) {
+   if (kbuff_arr[i])
+   dma_free_coherent(&instance->pdev->dev,
+ kern_sge32[i].length,
+ kbuff_arr[i],
+ kern_sge32[i].phys_addr);
}
 
megasas_return_cmd(instance, cmd);
-- 
1.7.10.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 2/5] qla2xxx: Test and clear FCPORT_UPDATE_NEEDED atomically.

2012-11-21 Thread Saurav Kashyap
Hi,
This needs to be copied to stable, added in cc.

Thanks,
~Saurav



>From: David Jeffery 
>
>When the qla2xxx driver loses access to multiple, remote ports, there is
>a race
>condition which can occur which will keep the request stuck on a scsi
>request
>queue indefinitely.
>
>This bad state occurred do to a race condition with how the
>FCPORT_UPDATE_NEEDED
>bit is set in qla2x00_schedule_rport_del(), and how it is cleared in
>qla2x00_do_dpc().  The problem port has its drport pointer set, but it
>has never
>been processed by the driver to inform the fc transport that the port has
>been
>lost.  qla2x00_schedule_rport_del() sets drport, and then sets the
>FCPORT_UPDATE_NEEDED bit.  In qla2x00_do_dpc(), the port lists are walked
>and
>any drport pointer is handled and the fc transport informed of the port
>loss,
>then the FCPORT_UPDATE_NEEDED bit is cleared.  This leaves a race where
>the
>dpc thread is processing one port removal, another port removal is marked
>with a call to qla2x00_schedule_rport_del(), and the dpc thread clears the
>bit for both removals, even though only the first removal was actually
>handled.  Until another event occurs to set FCPORT_UPDATE_NEEDED, the
>later
>port removal is never finished and qla2xxx stays in a bad state which
>causes
>requests to become stuck on request queues.
>
>This patch updates the driver to test and clear FCPORT_UPDATE_NEEDED
>atomically.  This ensures the port state changes are processed and not
>lost.
>
>Signed-off-by: David Jeffery 
>Signed-off-by: Chad Dupuis 
>Cc: sta...@vger.kernel.org
>Signed-off-by: Saurav Kashyap 
>---
> drivers/scsi/qla2xxx/qla_os.c |4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
>index d501bf5..a603967 100644
>--- a/drivers/scsi/qla2xxx/qla_os.c
>+++ b/drivers/scsi/qla2xxx/qla_os.c
>@@ -4505,9 +4505,9 @@ qla2x00_do_dpc(void *data)
>   "ISP abort end.\n");
>   }
>
>-  if (test_bit(FCPORT_UPDATE_NEEDED, &base_vha->dpc_flags)) {
>+  if (test_and_clear_bit(FCPORT_UPDATE_NEEDED,
>+  &base_vha->dpc_flags)) {
>   qla2x00_update_fcports(base_vha);
>-  clear_bit(FCPORT_UPDATE_NEEDED, &base_vha->dpc_flags);
>   }
>
>   if (test_bit(SCR_PENDING, &base_vha->dpc_flags)) {
>--
>1.7.7
>




This message and any attached documents contain information from QLogic 
Corporation or its wholly-owned subsidiaries that may be confidential. If you 
are not the intended recipient, you may not read, copy, distribute, or use this 
information. If you have received this transmission in error, please notify the 
sender immediately by reply e-mail and then delete this message.

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