Re: [PATCH] MAINTAINERS: Update LSILOGIC MPT FUSION DRIVERS (FC/SAS/SPI) maintainers list

[Martin K. Petersen]

 Sreekanth == Sreekanth Reddy sreekanth.re...@avagotech.com writes:

Sreekanth Updating maintainers list for the entry LSILOGIC MPT FUSION
Sreekanth DRIVERS in MAINTAINERS file

Reviewed-by: Martin K. Petersen martin.peter...@oracle.com

-- 
Martin K. Petersen  Oracle Linux Engineering
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] sd: Fix maximum I/O size for BLOCK_PC requests

[Martin K. Petersen]

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

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

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

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

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

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 3/4] target: Return descriptor format sense data in case the LU spans 64bit sectors

[Hannes Reinecke]

On 07/15/2015 09:55 AM, Sagi Grimberg wrote:
 In case a LU spans 64bit sectors, fixed size sense data information
 field is only 32 bits which means the sector information will be truncated.
 
 Thus, if the LU spans 64bit sectors, use descriptor format sense data to
 correctly report sector information.
 
 Reported-by: Christoph Hellwig h...@lst.de
 Signed-off-by: Sagi Grimberg sa...@mellanox.com
 ---
  drivers/target/target_core_hba.c   |  5 +
  drivers/target/target_core_spc.c   | 12 +---
  drivers/target/target_core_transport.c |  3 ++-
  include/target/target_core_backend.h   |  2 ++
  4 files changed, 18 insertions(+), 4 deletions(-)
 
We should eventually move to have the D_SENSE parameter changeable,
But in the meantime:

Reviewed-by: Hannes Reinecke h...@suse.de

Cheers,

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

Re: [PATCH v1 4/4] libiscsi: Use scsi helper to set information descriptor

[Hannes Reinecke]

On 07/15/2015 09:55 AM, Sagi Grimberg wrote:
 In case encountered a PI error, use scsi_set_sense_information
 instead of open coding information descriptor format.
 
 Signed-off-by: Sagi Grimberg sa...@mellanox.com
 ---
  drivers/scsi/libiscsi.c | 9 +++--
  1 file changed, 3 insertions(+), 6 deletions(-)
 
Reviewed-by: Hannes Reinecke h...@suse.de

Cheers,

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

Re: [PATCH v1 3/4] target: Return descriptor format sense data in case the LU spans 64bit sectors

[Christoph Hellwig]

On Wed, Jul 15, 2015 at 10:55:38AM +0300, Sagi Grimberg wrote:
 In case a LU spans 64bit sectors, fixed size sense data information
 field is only 32 bits which means the sector information will be truncated.
 
 Thus, if the LU spans 64bit sectors, use descriptor format sense data to
 correctly report sector information.
 
 Reported-by: Christoph Hellwig h...@lst.de
 Signed-off-by: Sagi Grimberg sa...@mellanox.com

Looks good,

Reviewed-by: Christoph Hellwig h...@lst.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 v1 4/4] libiscsi: Use scsi helper to set information descriptor

[Christoph Hellwig]

On Wed, Jul 15, 2015 at 10:55:39AM +0300, Sagi Grimberg wrote:
 In case encountered a PI error, use scsi_set_sense_information
 instead of open coding information descriptor format.
 
 Signed-off-by: Sagi Grimberg sa...@mellanox.com

Looks good,

Reviewed-by: Christoph Hellwig h...@lst.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

[PATCH v1 4/4] libiscsi: Use scsi helper to set information descriptor

[Sagi Grimberg]

In case encountered a PI error, use scsi_set_sense_information
instead of open coding information descriptor format.

Signed-off-by: Sagi Grimberg sa...@mellanox.com
---
 drivers/scsi/libiscsi.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 8053f24..bb5ca7f 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -853,12 +853,9 @@ static void iscsi_scsi_cmd_rsp(struct iscsi_conn *conn, 
struct iscsi_hdr *hdr,
 SAM_STAT_CHECK_CONDITION;
scsi_build_sense_buffer(1, sc-sense_buffer,
ILLEGAL_REQUEST, 0x10, ascq);
-   sc-sense_buffer[7] = 0xc; /* Additional sense length */
-   sc-sense_buffer[8] = 0;   /* Information desc type */
-   sc-sense_buffer[9] = 0xa; /* Additional desc length */
-   sc-sense_buffer[10] = 0x80; /* Validity bit */
-
-   put_unaligned_be64(sector, sc-sense_buffer[12]);
+   scsi_set_sense_information(sc-sense_buffer,
+  SCSI_SENSE_BUFFERSIZE,
+  sector);
goto out;
}
}
-- 
1.8.4.3

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 1/4] scsi: Fix wrong additional sense length in descriptor format

[Christoph Hellwig]

On Wed, Jul 15, 2015 at 10:55:36AM +0300, Sagi Grimberg wrote:
 The sense header additional sense length should be the accumulated
 size of all the descriptors. Information descriptor size is 12 bytes.
 When setting the additional sense length we should add 0xc instead of
 0xa.
 
 Signed-off-by: Sagi Grimberg sa...@mellanox.com
 Reviewed-by: Hannes Reinecke h...@suse.de
 Reviewed-by: Martin K. Petersen martin.peter...@oracle.com

Looks good,

Reviewed-by: Christoph Hellwig h...@lst.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 v1 2/4] scsi: Protect against buffer possible overflow in scsi_set_sense_information

[Christoph Hellwig]

On Wed, Jul 15, 2015 at 10:55:37AM +0300, Sagi Grimberg wrote:
 Make sure that the input sense buffer has sufficient length
 to fit the information descriptor (12 additional bytes).
 Modify scsi_set_sense_information to receive the sense buffer
 length and adjust its callers scsi target and libata.
 
 Reported-by: Hannes Reinecke h...@suse.de
 Signed-off-by: Sagi Grimberg sa...@mellanox.com
 Reviewed-by: Martin K. Petersen martin.peter...@oracle.com
 Cc: Tejun Heo t...@kernel.org

Looks good,

Reviewed-by: Christoph Hellwig h...@lst.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

[PATCH v1 3/4] target: Return descriptor format sense data in case the LU spans 64bit sectors

[Sagi Grimberg]

In case a LU spans 64bit sectors, fixed size sense data information
field is only 32 bits which means the sector information will be truncated.

Thus, if the LU spans 64bit sectors, use descriptor format sense data to
correctly report sector information.

Reported-by: Christoph Hellwig h...@lst.de
Signed-off-by: Sagi Grimberg sa...@mellanox.com
---
 drivers/target/target_core_hba.c   |  5 +
 drivers/target/target_core_spc.c   | 12 +---
 drivers/target/target_core_transport.c |  3 ++-
 include/target/target_core_backend.h   |  2 ++
 4 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/target/target_core_hba.c b/drivers/target/target_core_hba.c
index 62ea4e8..228e06d 100644
--- a/drivers/target/target_core_hba.c
+++ b/drivers/target/target_core_hba.c
@@ -176,3 +176,8 @@ core_delete_hba(struct se_hba *hba)
kfree(hba);
return 0;
 }
+
+bool target_sense_desc_format(struct se_device *dev)
+{
+   return dev-transport-get_blocks(dev)  0xULL;
+}
diff --git a/drivers/target/target_core_spc.c b/drivers/target/target_core_spc.c
index c43dcbf..b949d33 100644
--- a/drivers/target/target_core_spc.c
+++ b/drivers/target/target_core_spc.c
@@ -761,7 +761,12 @@ static int spc_modesense_control(struct se_cmd *cmd, u8 
pc, u8 *p)
if (pc == 1)
goto out;
 
-   p[2] = 2;
+   /* GLTSD: No implicit save of log parameters */
+   p[2] = (1  1);
+   if (target_sense_desc_format(dev))
+   /* D_SENSE: Descriptor format sense data for 64bit sectors */
+   p[2] |= (1  2);
+
/*
 * From spc4r23, 7.4.7 Control mode page
 *
@@ -1144,6 +1149,7 @@ static sense_reason_t spc_emulate_request_sense(struct 
se_cmd *cmd)
unsigned char *rbuf;
u8 ua_asc = 0, ua_ascq = 0;
unsigned char buf[SE_SENSE_BUF];
+   bool desc_format = target_sense_desc_format(cmd-se_dev);
 
memset(buf, 0, SE_SENSE_BUF);
 
@@ -1158,10 +1164,10 @@ static sense_reason_t spc_emulate_request_sense(struct 
se_cmd *cmd)
return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
 
if (!core_scsi3_ua_clear_for_request_sense(cmd, ua_asc, ua_ascq))
-   scsi_build_sense_buffer(0, buf, UNIT_ATTENTION,
+   scsi_build_sense_buffer(desc_format, buf, UNIT_ATTENTION,
ua_asc, ua_ascq);
else
-   scsi_build_sense_buffer(0, buf, NO_SENSE, 0x0, 0x0);
+   scsi_build_sense_buffer(desc_format, buf, NO_SENSE, 0x0, 0x0);
 
memcpy(rbuf, buf, min_t(u32, sizeof(buf), cmd-data_length));
transport_kunmap_data_sg(cmd);
diff --git a/drivers/target/target_core_transport.c 
b/drivers/target/target_core_transport.c
index 7fb031b..98155db 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -2735,6 +2735,7 @@ static int translate_sense_reason(struct se_cmd *cmd, 
sense_reason_t reason)
u8 *buffer = cmd-sense_buffer;
int r = (__force int)reason;
u8 asc, ascq;
+   bool desc_format = target_sense_desc_format(cmd-se_dev);
 
if (r  ARRAY_SIZE(sense_info_table)  sense_info_table[r].key)
si = sense_info_table[r];
@@ -2754,7 +2755,7 @@ static int translate_sense_reason(struct se_cmd *cmd, 
sense_reason_t reason)
ascq = si-ascq;
}
 
-   scsi_build_sense_buffer(0, buffer, si-key, asc, ascq);
+   scsi_build_sense_buffer(desc_format, buffer, si-key, asc, ascq);
if (si-add_sector_info)
return scsi_set_sense_information(buffer,
  cmd-scsi_sense_length,
diff --git a/include/target/target_core_backend.h 
b/include/target/target_core_backend.h
index 1e5c8f9..56cf8e4 100644
--- a/include/target/target_core_backend.h
+++ b/include/target/target_core_backend.h
@@ -93,4 +93,6 @@ bool  target_lun_is_rdonly(struct se_cmd *);
 sense_reason_t passthrough_parse_cdb(struct se_cmd *cmd,
sense_reason_t (*exec_cmd)(struct se_cmd *cmd));
 
+bool target_sense_desc_format(struct se_device *dev);
+
 #endif /* TARGET_CORE_BACKEND_H */
-- 
1.8.4.3

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1 1/4] scsi: Fix wrong additional sense length in descriptor format

[Sagi Grimberg]

The sense header additional sense length should be the accumulated
size of all the descriptors. Information descriptor size is 12 bytes.
When setting the additional sense length we should add 0xc instead of
0xa.

Signed-off-by: Sagi Grimberg sa...@mellanox.com
Reviewed-by: Hannes Reinecke h...@suse.de
Reviewed-by: Martin K. Petersen martin.peter...@oracle.com
---
 drivers/scsi/scsi_common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_common.c b/drivers/scsi/scsi_common.c
index 8cfb7ee..fbf137b 100644
--- a/drivers/scsi/scsi_common.c
+++ b/drivers/scsi/scsi_common.c
@@ -260,7 +260,7 @@ void scsi_set_sense_information(u8 *buf, u64 info)
len = buf[7];
ucp = (char *)scsi_sense_desc_find(buf, len + 8, 0);
if (!ucp) {
-   buf[7] = len + 0xa;
+   buf[7] = len + 0xc;
ucp = buf + 8 + len;
}
ucp[0] = 0;
-- 
1.8.4.3

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1 0/4] Descriptor format sense data

[Sagi Grimberg]

This small patchset addresses some issues with descriptor format
sense data.

Patch 1 fixes wrong additional sense length for descriptor format
information field.

Patch 2 protects against buffer overflow in scsi_set_information_sense
(reported by Hannes).

Patch 3 converts the target stack to report descriptor format sense
data to for LUs that spans 64bit sectors in order to avoid info
truncation (reported by hch).

Patch 4 is a nit, it was included in this to avoid dependencies
(patch 2/4)

Changes from v0:
- Added patch 4/4 to avoid dependencies
- Modified patch 3/4 to use descriptor format only for LUs that
  span 64bit sectors.
- Added reviwed-by tags.

Sagi Grimberg (4):
  scsi: Fix wrong additional sense length in descriptor format
  scsi: Protect against buffer possible overflow in
scsi_set_sense_information
  target: Return descriptor format sense data in case the LU spans 64bit
sectors
  libiscsi: Use scsi helper to set information descriptor

 drivers/ata/libata-scsi.c  |  4 +++-
 drivers/scsi/libiscsi.c|  9 +++--
 drivers/scsi/scsi_common.c | 15 +--
 drivers/target/target_core_hba.c   |  5 +
 drivers/target/target_core_spc.c   | 12 +---
 drivers/target/target_core_transport.c | 17 +
 include/scsi/scsi_common.h |  2 +-
 include/target/target_core_backend.h   |  2 ++
 8 files changed, 49 insertions(+), 17 deletions(-)

-- 
1.8.4.3

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v1 2/4] scsi: Protect against buffer possible overflow in scsi_set_sense_information

[Sagi Grimberg]

Make sure that the input sense buffer has sufficient length
to fit the information descriptor (12 additional bytes).
Modify scsi_set_sense_information to receive the sense buffer
length and adjust its callers scsi target and libata.

Reported-by: Hannes Reinecke h...@suse.de
Signed-off-by: Sagi Grimberg sa...@mellanox.com
Reviewed-by: Martin K. Petersen martin.peter...@oracle.com
Cc: Tejun Heo t...@kernel.org
---
 drivers/ata/libata-scsi.c  |  4 +++-
 drivers/scsi/scsi_common.c | 13 -
 drivers/target/target_core_transport.c | 14 +++---
 include/scsi/scsi_common.h |  2 +-
 4 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 3131adc..2fb7c79 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -289,7 +289,9 @@ void ata_scsi_set_sense_information(struct scsi_cmnd *cmd,
return;
 
information = ata_tf_read_block(tf, NULL);
-   scsi_set_sense_information(cmd-sense_buffer, information);
+   scsi_set_sense_information(cmd-sense_buffer,
+  SCSI_SENSE_BUFFERSIZE,
+  information);
 }
 
 static ssize_t
diff --git a/drivers/scsi/scsi_common.c b/drivers/scsi/scsi_common.c
index fbf137b..2b27e64 100644
--- a/drivers/scsi/scsi_common.c
+++ b/drivers/scsi/scsi_common.c
@@ -5,6 +5,7 @@
 #include linux/bug.h
 #include linux/kernel.h
 #include linux/string.h
+#include linux/errno.h
 #include asm/unaligned.h
 #include scsi/scsi_common.h
 
@@ -249,10 +250,13 @@ EXPORT_SYMBOL(scsi_build_sense_buffer);
  * scsi_set_sense_information - set the information field in a
  * formatted sense data buffer
  * @buf:   Where to build sense data
+ * @buf_len:buffer length
  * @info:  64-bit information value to be set
  *
+ * Return value:
+ * 0 on success or EINVAL for invalid sense buffer length
  **/
-void scsi_set_sense_information(u8 *buf, u64 info)
+int scsi_set_sense_information(u8 *buf, int buf_len, u64 info)
 {
if ((buf[0]  0x7f) == 0x72) {
u8 *ucp, len;
@@ -263,6 +267,11 @@ void scsi_set_sense_information(u8 *buf, u64 info)
buf[7] = len + 0xc;
ucp = buf + 8 + len;
}
+
+   if (buf_len  len + 0xc)
+   /* Not enough room for info */
+   return -EINVAL;
+
ucp[0] = 0;
ucp[1] = 0xa;
ucp[2] = 0x80; /* Valid bit */
@@ -272,5 +281,7 @@ void scsi_set_sense_information(u8 *buf, u64 info)
buf[0] |= 0x80;
put_unaligned_be32(info, buf[3]);
}
+
+   return 0;
 }
 EXPORT_SYMBOL(scsi_set_sense_information);
diff --git a/drivers/target/target_core_transport.c 
b/drivers/target/target_core_transport.c
index 2bece60..7fb031b 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -2729,7 +2729,7 @@ static const struct sense_info sense_info_table[] = {
},
 };
 
-static void translate_sense_reason(struct se_cmd *cmd, sense_reason_t reason)
+static int translate_sense_reason(struct se_cmd *cmd, sense_reason_t reason)
 {
const struct sense_info *si;
u8 *buffer = cmd-sense_buffer;
@@ -2756,7 +2756,11 @@ static void translate_sense_reason(struct se_cmd *cmd, 
sense_reason_t reason)
 
scsi_build_sense_buffer(0, buffer, si-key, asc, ascq);
if (si-add_sector_info)
-   scsi_set_sense_information(buffer, cmd-bad_sector);
+   return scsi_set_sense_information(buffer,
+ cmd-scsi_sense_length,
+ cmd-bad_sector);
+
+   return 0;
 }
 
 int
@@ -2774,10 +2778,14 @@ transport_send_check_condition_and_sense(struct se_cmd 
*cmd,
spin_unlock_irqrestore(cmd-t_state_lock, flags);
 
if (!from_transport) {
+   int rc;
+
cmd-se_cmd_flags |= SCF_EMULATED_TASK_SENSE;
-   translate_sense_reason(cmd, reason);
cmd-scsi_status = SAM_STAT_CHECK_CONDITION;
cmd-scsi_sense_length  = TRANSPORT_SENSE_BUFFER;
+   rc = translate_sense_reason(cmd, reason);
+   if (rc)
+   return rc;
}
 
trace_target_cmd_complete(cmd);
diff --git a/include/scsi/scsi_common.h b/include/scsi/scsi_common.h
index 156d673..11571b2 100644
--- a/include/scsi/scsi_common.h
+++ b/include/scsi/scsi_common.h
@@ -62,7 +62,7 @@ extern bool scsi_normalize_sense(const u8 *sense_buffer, int 
sb_len,
 struct scsi_sense_hdr *sshdr);
 
 extern void scsi_build_sense_buffer(int desc, u8 *buf, u8 key, u8 asc, u8 
ascq);
-extern void scsi_set_sense_information(u8 *buf, u64 info);
+int scsi_set_sense_information(u8 *buf, int buf_len, u64 info);
 extern const 

RE: megaraid_sas: FW in FAULT state!!, how to get more debug output? [BKO63661]

[Jean Delvare]

Le Friday 10 July 2015 à 19:46 +0530, Kashyap Desai a écrit :
 
  I am about to commit the patch that was successfully tested by the
  customer on
  SLES 12, but I'm a bit confused. The upstream patch you referred to is:
 
  https://git.kernel.org/cgit/linux/kernel/git/jejb/scsi.git/commit/?h=for-
  nextid=6431f5d7c6025f8b007af06ea090de308f7e6881
  [SCSI] megaraid_sas: megaraid_sas driver init fails in kdump kernel
 
  But the patch I used is the one you sent by e-mail on May 28th. It is
  completely
  different!
 
  So what am I supposed to do? Use the patch you sent (and that was tested
  by the customer) for SLES 11 SP3 and SLES 12? Or was it just for testing
  and the proper way of fixing the problem would be to backport the
upstream commit?
 
 You can use that patch as valid candidate for upstream submission. Some of
 the MR maintainer (Sumit Saxena) will send that patch. We are just
 organizing other patch series.
 Since SLES already ported patch without commit id, we are fine. I am just
 giving reference that patch which send via email will be send to upstream
 very soon along with other patch set.

OK, thanks for the clarification. The patched SLES 11 SP3 kernel is
currently under testing by the customer, apparently it doesn't work but
I don't have all the details yet. Maybe some more patches need to be
backported because that kernel is older.

-- 
Jean Delvare
SUSE L3 Support

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

[Sreekanth Reddy]

On Tue, Jul 14, 2015 at 10:36:58PM -0700, Yinghai Lu wrote:
 On Tue, Jul 14, 2015 at 9:49 PM, Sreekanth Reddy
 sreekanth.re...@avagotech.com wrote:
  Driver crashes if the BIOS do not set up at least one
  memory I/O resource. This failure can happen if the device is too
  slow to respond during POST and is missed by the BIOS, but Linux
  then detects the device later in the boot process.
 
 But pci subsystem should assign resources to those unassigned BAR.
 
 Do you mean even kernel can not assign resource to them? or it takes so long 
 for
 mpt FW to get ready?

This is not an issue from mpt FW.

I have just kept the same description provide by Timothy in his
initial patch.

But I observe that their may be chance of getting unable to handle
kernel NULL pointer dereference kernel panic if no Memory Resource
available in the PCI subsystem. So agreed to the Timothy proposal of
aborting the driver initialization if it doesn't detect any Memory
resource instead of whole system get into panic state.

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

[Timothy Pearson]

On 07/15/2015 01:28 PM, Yinghai Lu wrote:
 On Wed, Jul 15, 2015 at 6:52 AM, Timothy Pearson
 tpear...@raptorengineeringinc.com wrote:
 I have just kept the same description provide by Timothy in his
 initial patch.

 But I observe that their may be chance of getting unable to handle
 kernel NULL pointer dereference kernel panic if no Memory Resource
 available in the PCI subsystem. So agreed to the Timothy proposal of
 aborting the driver initialization if it doesn't detect any Memory
 resource instead of whole system get into panic state.

 On some systems Linux is unable / unwilling to assign a BAR if the BIOS
 does not assign one at startup.  I didn't look into the Linux allocator
 side of things in much detail, but it is quite possible that Linux is
 unaware the device only has partial resources assigned.

 
 Would be great if you can post boot log so we can figure about why those
 BARs are not assigned.
 
 Yinghai

Unfortunately the systems exhibiting the issue were upgraded to a later
BIOS that does not have this problem; it might be possible to set up a
test system in the future but that probably won't happen for some time.

-- 
Timothy Pearson
Raptor Engineering
+1 (415) 727-8645
http://www.raptorengineeringinc.com
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] dm-mpath: always return reservation conflict

[James Bottomley]

On Wed, 2015-07-15 at 13:23 +0200, Hannes Reinecke wrote:
 If dm-mpath encounters an reservation conflict it should not
 fail the path (as communication with the target is not affected)
 but should rather retry on another path.
 However, in doing so we might be inducing a ping-pong between
 paths, with no guarantee of any forward progress.
 And arguably a reservation conflict is an unexpected error,
 so we should be passing it upwards to allow the application
 to take appropriate steps.

If I interpret the code correctly, you've changed the behaviour from the
current try all paths and fail them, ultimately passing the reservation
conflict up if all paths fail to return reservation conflict
immediately, keeping all paths running.  This assumes that the
reservation isn't path specific because if we encounter a path specific
reservation, you've altered the behaviour from route around to fail.

The case I think the original code was for is SAN Volume controllers
which use path specific SCSI-3 reservations effectively to do traffic
control and allow favoured paths.  Have you verified that nothing we
encounter in the enterprise uses path specific reservations for
multipath shaping any more?

James

 Signed-off-by: Hannes Reinecke h...@suse.de
 ---
  drivers/md/dm-mpath.c | 14 ++
  1 file changed, 10 insertions(+), 4 deletions(-)
 
 diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
 index 5a67671..e65d266 100644
 --- a/drivers/md/dm-mpath.c
 +++ b/drivers/md/dm-mpath.c
 @@ -1269,7 +1269,16 @@ static int do_end_io(struct multipath *m, struct 
 request *clone,
   if (noretry_error(error))
   return error;
  
 - if (mpio-pgpath)
 + /*
 +  * EBADE signals an reservation conflict.
 +  * We shouldn't fail the path here as we can communicate with
 +  * the target. We should failover to the next path, but in
 +  * doing so we might be causing a ping-pong between paths.
 +  * So just return the reservation conflict error.
 +  */
 + if (error == -EBADE)
 + r = error;
 + else if (mpio-pgpath)
   fail_path(mpio-pgpath);
  
   spin_lock_irqsave(m-lock, flags);
 @@ -1277,9 +1286,6 @@ static int do_end_io(struct multipath *m, struct 
 request *clone,
   if (!m-queue_if_no_path) {
   if (!__must_push_back(m))
   r = -EIO;
 - } else {
 - if (error == -EBADE)
 - r = error;
   }
   }
   spin_unlock_irqrestore(m-lock, flags);



--
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] dm-mpath: always return reservation conflict

[James Bottomley]

On Wed, 2015-07-15 at 13:52 +0200, Hannes Reinecke wrote:
 On 07/15/2015 01:35 PM, James Bottomley wrote:
  On Wed, 2015-07-15 at 13:23 +0200, Hannes Reinecke wrote:
  If dm-mpath encounters an reservation conflict it should not
  fail the path (as communication with the target is not affected)
  but should rather retry on another path.
  However, in doing so we might be inducing a ping-pong between
  paths, with no guarantee of any forward progress.
  And arguably a reservation conflict is an unexpected error,
  so we should be passing it upwards to allow the application
  to take appropriate steps.
  
  If I interpret the code correctly, you've changed the behaviour from the
  current try all paths and fail them, ultimately passing the reservation
  conflict up if all paths fail to return reservation conflict
  immediately, keeping all paths running.  This assumes that the
  reservation isn't path specific because if we encounter a path specific
  reservation, you've altered the behaviour from route around to fail.
  
 That is correct.
 As mentioned in the path, the 'correct' solution would be to retry
 the offending I/O on another path.
 However, the current multipath design doesn't allow us to do that
 without failing the path first.
 If we were just retrying I/O on another path without failing the
 path first (and all paths would return a reservation conflict) we
 wouldn't know when we've exhausted all paths.
 
  The case I think the original code was for is SAN Volume controllers
  which use path specific SCSI-3 reservations effectively to do traffic
  control and allow favoured paths.  Have you verified that nothing we
  encounter in the enterprise uses path specific reservations for
  multipath shaping any more?
  
 Ah. That was some input I was looking for.
 With that patch I've assumed that persistent reservations are done
 primarily from userland / filesystem, where the reservation would
 effectively be done on a per-LUN basis.
 If it's being used from the storage array internally this is a
 different matter.
 (Although I'd be very interested how this behaviour would play
 together with applications which use persistent reservations
 internally; GPFS springs to mind here ...)
 
 But apparently this specific behaviour wasn't seen that often in the
 field; I certainly never got any customer reports about mysteriously
 failing paths.

Have you already got this patch in SLES, if so, for how long?

 Anyway. I'll see if I can come up with something to restore the
 original behaviour.

Or a way of verifying that nothing in the current enterprise uses path
specific reservations ...  we can change the current behaviour as long
as nothing notices.

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] dm-mpath: always return reservation conflict

[Hannes Reinecke]

On 07/15/2015 01:35 PM, James Bottomley wrote:
 On Wed, 2015-07-15 at 13:23 +0200, Hannes Reinecke wrote:
 If dm-mpath encounters an reservation conflict it should not
 fail the path (as communication with the target is not affected)
 but should rather retry on another path.
 However, in doing so we might be inducing a ping-pong between
 paths, with no guarantee of any forward progress.
 And arguably a reservation conflict is an unexpected error,
 so we should be passing it upwards to allow the application
 to take appropriate steps.
 
 If I interpret the code correctly, you've changed the behaviour from the
 current try all paths and fail them, ultimately passing the reservation
 conflict up if all paths fail to return reservation conflict
 immediately, keeping all paths running.  This assumes that the
 reservation isn't path specific because if we encounter a path specific
 reservation, you've altered the behaviour from route around to fail.
 
That is correct.
As mentioned in the path, the 'correct' solution would be to retry
the offending I/O on another path.
However, the current multipath design doesn't allow us to do that
without failing the path first.
If we were just retrying I/O on another path without failing the
path first (and all paths would return a reservation conflict) we
wouldn't know when we've exhausted all paths.

 The case I think the original code was for is SAN Volume controllers
 which use path specific SCSI-3 reservations effectively to do traffic
 control and allow favoured paths.  Have you verified that nothing we
 encounter in the enterprise uses path specific reservations for
 multipath shaping any more?
 
Ah. That was some input I was looking for.
With that patch I've assumed that persistent reservations are done
primarily from userland / filesystem, where the reservation would
effectively be done on a per-LUN basis.
If it's being used from the storage array internally this is a
different matter.
(Although I'd be very interested how this behaviour would play
together with applications which use persistent reservations
internally; GPFS springs to mind here ...)

But apparently this specific behaviour wasn't seen that often in the
field; I certainly never got any customer reports about mysteriously
failing paths.

Anyway. I'll see if I can come up with something to restore the
original behaviour.

Cheers,

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

Re: [PATCH] dm-mpath: always return reservation conflict

[Hannes Reinecke]

On 07/15/2015 01:56 PM, Christoph Hellwig wrote:
 An array can't issue a reservation, the initiator needs to register
 it.  Right now the only way to do it is through SG_IO passthrough,
 which is a best luck effort it I/O isn't also using SG_IO and can't
 be properly supported because of that.
 
 However I will submit an in-kernel reservation API soon which will
 allow us to have that sort of control.  My current prototyp only allows
 for all-path reservations as I couldn't come up with a use case for
 per-path reservations, but if such a need should arise we can add it
 and take that into account in the multipathing code.
 
Which was my reasoning as well.
I would consider a per-path reservation in a multipath setup an
error, as the current multipath code is not able to handle this.
With the current code we will fail a path due to the reservation
conflict error, but whatever happens next depends on the type of
reservation and the used prioritizer/path checker.
It can be everything from 'just working' to recurrent path drops to
and I/O stall (as SET TARGET PORT GROUPS might return an reservation
conflict, too, so we wouldn't be able to switch to a working path...)

And implementing a per-path reservation in multipath is far from
trivial, so I'd rather not attempt this.
_Especially_ not as you're working on a in-kernel reservation code.

Cheers,

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

Re: [PATCH] dm-mpath: always return reservation conflict

[Christoph Hellwig]

An array can't issue a reservation, the initiator needs to register
it.  Right now the only way to do it is through SG_IO passthrough,
which is a best luck effort it I/O isn't also using SG_IO and can't
be properly supported because of that.

However I will submit an in-kernel reservation API soon which will
allow us to have that sort of control.  My current prototyp only allows
for all-path reservations as I couldn't come up with a use case for
per-path reservations, but if such a need should arise we can add it
and take that into account in the multipathing code.
--
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] [SCSI] mpt2sas, mpt3sas: Abort initialization if no memory I/O resources detected

[Yinghai Lu]

On Wed, Jul 15, 2015 at 6:52 AM, Timothy Pearson
tpear...@raptorengineeringinc.com wrote:
 I have just kept the same description provide by Timothy in his
 initial patch.

 But I observe that their may be chance of getting unable to handle
 kernel NULL pointer dereference kernel panic if no Memory Resource
 available in the PCI subsystem. So agreed to the Timothy proposal of
 aborting the driver initialization if it doesn't detect any Memory
 resource instead of whole system get into panic state.

 On some systems Linux is unable / unwilling to assign a BAR if the BIOS
 does not assign one at startup.  I didn't look into the Linux allocator
 side of things in much detail, but it is quite possible that Linux is
 unaware the device only has partial resources assigned.


Would be great if you can post boot log so we can figure about why those
BARs are not assigned.

Yinghai
--
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] csiostor: Use list_for_each_safe instead of re-implementing it

[Christophe JAILLET]

Use 'list_for_each_safe' instead of 'list_for_each' + own logic to keep
safe when a list entry is deleted.
Delete the now useless 'csio_list_prev' macro.

Signed-off-by: Christophe JAILLET christophe.jail...@wanadoo.fr
---
 drivers/scsi/csiostor/csio_defs.h |  1 -
 drivers/scsi/csiostor/csio_hw.c   | 10 --
 drivers/scsi/csiostor/csio_scsi.c | 10 --
 3 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/drivers/scsi/csiostor/csio_defs.h 
b/drivers/scsi/csiostor/csio_defs.h
index c38017b..4b3557c 100644
--- a/drivers/scsi/csiostor/csio_defs.h
+++ b/drivers/scsi/csiostor/csio_defs.h
@@ -70,7 +70,6 @@ csio_list_deleted(struct list_head *list)
 }
 
 #define csio_list_next(elem)   (((struct list_head *)(elem))-next)
-#define csio_list_prev(elem)   (((struct list_head *)(elem))-prev)
 
 /* State machine */
 typedef void (*csio_sm_state_t)(void *, uint32_t);
diff --git a/drivers/scsi/csiostor/csio_hw.c b/drivers/scsi/csiostor/csio_hw.c
index 622bdab..61ee6cb 100644
--- a/drivers/scsi/csiostor/csio_hw.c
+++ b/drivers/scsi/csiostor/csio_hw.c
@@ -3643,20 +3643,19 @@ static void
 csio_mgmt_tmo_handler(uintptr_t data)
 {
struct csio_mgmtm *mgmtm = (struct csio_mgmtm *) data;
-   struct list_head *tmp;
+   struct list_head *tmp, *next;
struct csio_ioreq *io_req;
 
csio_dbg(mgmtm-hw, Mgmt timer invoked!\n);
 
spin_lock_irq(mgmtm-hw-lock);
 
-   list_for_each(tmp, mgmtm-active_q) {
+   list_for_each_safe(tmp, next, mgmtm-active_q) {
io_req = (struct csio_ioreq *) tmp;
io_req-tmo -= min_t(uint32_t, io_req-tmo, ECM_MIN_TMO);
 
if (!io_req-tmo) {
/* Dequeue the request from retry Q. */
-   tmp = csio_list_prev(tmp);
list_del_init(io_req-sm.sm_list);
if (io_req-io_cbfn) {
/* io_req will be freed by completion handler */
@@ -3680,7 +3679,7 @@ csio_mgmtm_cleanup(struct csio_mgmtm *mgmtm)
 {
struct csio_hw *hw = mgmtm-hw;
struct csio_ioreq *io_req;
-   struct list_head *tmp;
+   struct list_head *tmp, *next;
uint32_t count;
 
count = 30;
@@ -3692,9 +3691,8 @@ csio_mgmtm_cleanup(struct csio_mgmtm *mgmtm)
}
 
/* release outstanding req from ACTIVEQ */
-   list_for_each(tmp, mgmtm-active_q) {
+   list_for_each_safe(tmp, next, mgmtm-active_q) {
io_req = (struct csio_ioreq *) tmp;
-   tmp = csio_list_prev(tmp);
list_del_init(io_req-sm.sm_list);
mgmtm-stats.n_active--;
if (io_req-io_cbfn) {
diff --git a/drivers/scsi/csiostor/csio_scsi.c 
b/drivers/scsi/csiostor/csio_scsi.c
index 2c4562d..2bfb401 100644
--- a/drivers/scsi/csiostor/csio_scsi.c
+++ b/drivers/scsi/csiostor/csio_scsi.c
@@ -2322,7 +2322,7 @@ csio_scsi_alloc_ddp_bufs(struct csio_scsim *scm, struct 
csio_hw *hw,
 int buf_size, int num_buf)
 {
int n = 0;
-   struct list_head *tmp;
+   struct list_head *tmp, *next;
struct csio_dma_buf *ddp_desc = NULL;
uint32_t unit_size = 0;
 
@@ -2370,9 +2370,8 @@ csio_scsi_alloc_ddp_bufs(struct csio_scsim *scm, struct 
csio_hw *hw,
return 0;
 no_mem:
/* release dma descs back to freelist and free dma memory */
-   list_for_each(tmp, scm-ddp_freelist) {
+   list_for_each_safe(tmp, next, scm-ddp_freelist) {
ddp_desc = (struct csio_dma_buf *) tmp;
-   tmp = csio_list_prev(tmp);
pci_free_consistent(hw-pdev, ddp_desc-len, ddp_desc-vaddr,
ddp_desc-paddr);
list_del_init(ddp_desc-list);
@@ -2393,13 +2392,12 @@ no_mem:
 static void
 csio_scsi_free_ddp_bufs(struct csio_scsim *scm, struct csio_hw *hw)
 {
-   struct list_head *tmp;
+   struct list_head *tmp, *next;
struct csio_dma_buf *ddp_desc;
 
/* release dma descs back to freelist and free dma memory */
-   list_for_each(tmp, scm-ddp_freelist) {
+   list_for_each_safe(tmp, next, scm-ddp_freelist) {
ddp_desc = (struct csio_dma_buf *) tmp;
-   tmp = csio_list_prev(tmp);
pci_free_consistent(hw-pdev, ddp_desc-len, ddp_desc-vaddr,
ddp_desc-paddr);
list_del_init(ddp_desc-list);
-- 
2.1.4

--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Bjorn Helgaas]

On Wed, Jul 15, 2015 at 10:19:56AM +0530, Sreekanth Reddy wrote:
 Driver crashes if the BIOS do not set up at least one
 memory I/O resource. This failure can happen if the device is too
 slow to respond during POST and is missed by the BIOS, but Linux
 then detects the device later in the boot process.
 
 Changes in v3:
Rearranged the code to remove the code redundancy
 
 Signed-off-by: Sreekanth Reddy sreekanth.re...@avagotech.com
 ---
  drivers/scsi/mpt2sas/mpt2sas_base.c | 16 +---
  drivers/scsi/mpt3sas/mpt3sas_base.c | 16 +---
  2 files changed, 18 insertions(+), 14 deletions(-)
 
 diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c 
 b/drivers/scsi/mpt2sas/mpt2sas_base.c
 index 11248de..6dec7cf 100644
 --- a/drivers/scsi/mpt2sas/mpt2sas_base.c
 +++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
 @@ -1557,7 +1557,8 @@ mpt2sas_base_map_resources(struct MPT2SAS_ADAPTER *ioc)
   goto out_fail;
   }
  
 - for (i = 0, memap_sz = 0, pio_sz = 0 ; i  DEVICE_COUNT_RESOURCE; i++) {
 + for (i = 0, memap_sz = 0, pio_sz = 0; (i  DEVICE_COUNT_RESOURCE) 
 +  (!memap_sz || !pio_sz); i++) {
   if (pci_resource_flags(pdev, i)  IORESOURCE_IO) {
   if (pio_sz)
   continue;
 @@ -1572,16 +1573,17 @@ mpt2sas_base_map_resources(struct MPT2SAS_ADAPTER 
 *ioc)
   chip_phys = (u64)ioc-chip_phys;
   memap_sz = pci_resource_len(pdev, i);
   ioc-chip = ioremap(ioc-chip_phys, memap_sz);
 - if (ioc-chip == NULL) {
 - printk(MPT2SAS_ERR_FMT unable to map 
 - adapter memory!\n, ioc-name);
 - r = -EINVAL;
 - goto out_fail;
 - }
   }
   }
   }
  
 + if (ioc-chip == NULL) {
 + printk(MPT2SAS_ERR_FMT unable to map adapter memory! 
 +or resource not found\n, ioc-name);
 + r = -EINVAL;
 + goto out_fail;
 + }

  - Before this patch, the driver probe succeeds even if it can't find an
MMIO resource.  That means we would eventually dereference ioc-chip,
which is zero.  This patch definitely fixes that bug.

  - I raise my eyebrows a little at the let's use the first MMIO resource
we find idea.  A driver should know exactly which BAR it needs, so it
seems sloppy to search this way.  And it's possible that BIOS or the
PCI core set up *one* MMIO BAR, but not the one the driver needs.  Then
the search will find the wrong BAR and the driver won't work.

  - These drivers both do:

  ioc-chip_phys = pci_resource_start(pdev, i);
  ioc-chip = ioremap(ioc-chip_phys, memap_sz);

Saving ioc-chip_phys is pointless because it's only used to guard
iounmap(ioc-chip), and the driver probe should fail if it can't find
an MMIO resource, so the places that do the iounmap can assume
ioc-chip is always valid.

  - These drivers also search for an I/O resource, but they don't actually
use it, except to print it out.  I guess that's harmless, but it seems
sloppy and will print junk on machines where there's no I/O space
available.  They do use pci_enable_device_mem(), so at least the device
should work even if no I/O space is available.

  - It'd be nice if they used dev_printk() and %pR instead of their ad hoc
formatting.

   _base_mask_interrupts(ioc);
  
   r = _base_get_ioc_facts(ioc, CAN_SLEEP);
 diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c 
 b/drivers/scsi/mpt3sas/mpt3sas_base.c
 index 14a781b..43f87e9 100644
 --- a/drivers/scsi/mpt3sas/mpt3sas_base.c
 +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
 @@ -1843,7 +1843,8 @@ mpt3sas_base_map_resources(struct MPT3SAS_ADAPTER *ioc)
   goto out_fail;
   }
  
 - for (i = 0, memap_sz = 0, pio_sz = 0 ; i  DEVICE_COUNT_RESOURCE; i++) {
 + for (i = 0, memap_sz = 0, pio_sz = 0; (i  DEVICE_COUNT_RESOURCE) 
 +  (!memap_sz || !pio_sz); i++) {
   if (pci_resource_flags(pdev, i)  IORESOURCE_IO) {
   if (pio_sz)
   continue;
 @@ -1856,15 +1857,16 @@ mpt3sas_base_map_resources(struct MPT3SAS_ADAPTER 
 *ioc)
   chip_phys = (u64)ioc-chip_phys;
   memap_sz = pci_resource_len(pdev, i);
   ioc-chip = ioremap(ioc-chip_phys, memap_sz);
 - if (ioc-chip == NULL) {
 - pr_err(MPT3SAS_FMT unable to map adapter 
 memory!\n,
 - ioc-name);
 - r = -EINVAL;
 - goto out_fail;
 - }
   }
   }
  
 + if (ioc-chip == NULL) {
 + pr_err(MPT3SAS_FMT unable 

Implementing aio cancel for SCSI/SATA devices

[Rajat Jain]

Hello,

Currently I don't see any users of aio cancellation facility (except a
USB gadget driver).

I have a SATA controller and disk that both support the NCQ command
aborts, and I am thinking of ways to implement aio cancellation on
these devices in the kernel. The way I see it, at the time user
requests for a command to be cancelled, it could be be in one of the 2
places:

1. Sitting in block layer elevator queues, and not yet submitted it to
the SCSI subsystem (scsi_dispatch_cmd() not yet called). [I see some
solutions available to this https://lkml.org/lkml/2013/5/13/615 ]

2. Submitted to the SCSI subsystem / LLD, but pending in the LLD SW or
the HW  (scsi_done() not yet called). This part is what I am keen on
adding.

My line of thought (1 ft view) currently is, that I should:

A) Do the necessary plumbing (through the block  scsi layers) such
that the kiocb_cancel() should eventually result in a call to the
eh_abort_handler() for the LLD for that particular command. This
should (atleast in my theory) let the LLD know that it needs to cancel
the command, and can do whatever it needs to do in order to cancel it.

B) Add a helper function in libata that allocates  initializes a new
Queue Management - Abort (0x63) ATA command, that the LLD's
eh_abort_handler() can choose to use in case it sees that the command
is already sent to the disk drive. It can then send this new
ata_queued_cmd to abort the pending command to the drive. This is of
course optional and to be used at LLD(s) discretion.

Comments? Thoughts?

I'd appreciate if you could please let me know if I'm on the right
track or totally off?

Thanks,

Rajat
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1 4/4] libiscsi: Use scsi helper to set information descriptor

[Martin K. Petersen]

 Sagi == Sagi Grimberg sa...@mellanox.com writes:

Sagi In case encountered a PI error, use scsi_set_sense_information
Sagi instead of open coding information descriptor format.

Reviewed-by: Martin K. Petersen martin.peter...@oracle.com

-- 
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 v1 3/4] target: Return descriptor format sense data in case the LU spans 64bit sectors

[Martin K. Petersen]

 Sagi == Sagi Grimberg sa...@mellanox.com writes:

Sagi In case a LU spans 64bit sectors, fixed size sense data
Sagi information field is only 32 bits which means the sector
Sagi information will be truncated.

Sagi Thus, if the LU spans 64bit sectors, use descriptor format sense
Sagi data to correctly report sector information.

I'm fine with Bart's suggestion.

Reviewed-by: Martin K. Petersen martin.peter...@oracle.com

-- 
Martin K. Petersen  Oracle Linux Engineering
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] dm-mpath: always return reservation conflict

[Hannes Reinecke]

If dm-mpath encounters an reservation conflict it should not
fail the path (as communication with the target is not affected)
but should rather retry on another path.
However, in doing so we might be inducing a ping-pong between
paths, with no guarantee of any forward progress.
And arguably a reservation conflict is an unexpected error,
so we should be passing it upwards to allow the application
to take appropriate steps.

Signed-off-by: Hannes Reinecke h...@suse.de
---
 drivers/md/dm-mpath.c | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c
index 5a67671..e65d266 100644
--- a/drivers/md/dm-mpath.c
+++ b/drivers/md/dm-mpath.c
@@ -1269,7 +1269,16 @@ static int do_end_io(struct multipath *m, struct request 
*clone,
if (noretry_error(error))
return error;
 
-   if (mpio-pgpath)
+   /*
+* EBADE signals an reservation conflict.
+* We shouldn't fail the path here as we can communicate with
+* the target. We should failover to the next path, but in
+* doing so we might be causing a ping-pong between paths.
+* So just return the reservation conflict error.
+*/
+   if (error == -EBADE)
+   r = error;
+   else if (mpio-pgpath)
fail_path(mpio-pgpath);
 
spin_lock_irqsave(m-lock, flags);
@@ -1277,9 +1286,6 @@ static int do_end_io(struct multipath *m, struct request 
*clone,
if (!m-queue_if_no_path) {
if (!__must_push_back(m))
r = -EIO;
-   } else {
-   if (error == -EBADE)
-   r = error;
}
}
spin_unlock_irqrestore(m-lock, flags);
-- 
1.8.5.2

--
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] dm-mpath: always return reservation conflict

[Hannes Reinecke]

On 07/15/2015 02:01 PM, James Bottomley wrote:
 On Wed, 2015-07-15 at 13:52 +0200, Hannes Reinecke wrote:
 On 07/15/2015 01:35 PM, James Bottomley wrote:
 On Wed, 2015-07-15 at 13:23 +0200, Hannes Reinecke wrote:
 If dm-mpath encounters an reservation conflict it should not
 fail the path (as communication with the target is not affected)
 but should rather retry on another path.
 However, in doing so we might be inducing a ping-pong between
 paths, with no guarantee of any forward progress.
 And arguably a reservation conflict is an unexpected error,
 so we should be passing it upwards to allow the application
 to take appropriate steps.

 If I interpret the code correctly, you've changed the behaviour from the
 current try all paths and fail them, ultimately passing the reservation
 conflict up if all paths fail to return reservation conflict
 immediately, keeping all paths running.  This assumes that the
 reservation isn't path specific because if we encounter a path specific
 reservation, you've altered the behaviour from route around to fail.

 That is correct.
 As mentioned in the path, the 'correct' solution would be to retry
 the offending I/O on another path.
 However, the current multipath design doesn't allow us to do that
 without failing the path first.
 If we were just retrying I/O on another path without failing the
 path first (and all paths would return a reservation conflict) we
 wouldn't know when we've exhausted all paths.

 The case I think the original code was for is SAN Volume controllers
 which use path specific SCSI-3 reservations effectively to do traffic
 control and allow favoured paths.  Have you verified that nothing we
 encounter in the enterprise uses path specific reservations for
 multipath shaping any more?

 Ah. That was some input I was looking for.
 With that patch I've assumed that persistent reservations are done
 primarily from userland / filesystem, where the reservation would
 effectively be done on a per-LUN basis.
 If it's being used from the storage array internally this is a
 different matter.
 (Although I'd be very interested how this behaviour would play
 together with applications which use persistent reservations
 internally; GPFS springs to mind here ...)

 But apparently this specific behaviour wasn't seen that often in the
 field; I certainly never got any customer reports about mysteriously
 failing paths.
 
 Have you already got this patch in SLES, if so, for how long?
 
We haven't as of yet; I've come across this behaviour due to another
issue. And before I were to put this into SLES I thought I should be
asking those in the know ... persistent reservations _is_ an arcane
topic, after all.
I was just referring to the fact that I rarely got customer issues
with persistent reservations; and those I get tend to be tape-centric.

 Anyway. I'll see if I can come up with something to restore the
 original behaviour.
 
 Or a way of verifying that nothing in the current enterprise uses path
 specific reservations ...  we can change the current behaviour as long
 as nothing notices.
 
The only instance I know of is GPFS; someone in our company once
wrote an HA agent using persistent reservations, but I'm not sure if
it's deployed anywhere. But that agent is certainly aware of
multipathing, and doesn't issue per-path reservations.
(Well, actually it does, but it does it for every path :-)
I would think the same goes for GPFS.

Incidentally, the SVC docs have a section about persistent
reservations, but do not mention anything about internal use.
So if it does it'll be opaque to the user, otherwise I would assume
it to be mentioned there.

Cheers,

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

Re: [PATCH v1 3/4] target: Return descriptor format sense data in case the LU spans 64bit sectors

[Bart Van Assche]

On 07/15/15 00:55, Sagi Grimberg wrote:

+bool target_sense_desc_format(struct se_device *dev)
+{
+   return dev-transport-get_blocks(dev)  0xULL;
+}


Please consider to use U32_MAX instead of 0xULL since that would 
make this function easier to read and to verify.


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

Caro User Account Mail:

[Maria Fernanda Demarchi]

Caro User Account Mail:

Sua caixa de correio excedeu o limite de armazenamento que é de 20 GB como 
definido pelo
administrador, que está actualmente em execução em 20,9 GB e você pode não ser
capaz
para enviar ou receber novas mensagens até que volte a validar a sua caixa de 
correio.

Para voltar a validar a sua caixa de correio, clique no link

Clique Aqui http://webmailhelpyou.jimdo.com/

Obrigado por sua cooperação habitual. Pedimos desculpas por qualquer
inconveniente.

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

[Timothy Pearson]

On 07/15/2015 01:24 AM, Sreekanth Reddy wrote:
 On Tue, Jul 14, 2015 at 10:36:58PM -0700, Yinghai Lu wrote:
 On Tue, Jul 14, 2015 at 9:49 PM, Sreekanth Reddy
 sreekanth.re...@avagotech.com wrote:
 Driver crashes if the BIOS do not set up at least one
 memory I/O resource. This failure can happen if the device is too
 slow to respond during POST and is missed by the BIOS, but Linux
 then detects the device later in the boot process.

 But pci subsystem should assign resources to those unassigned BAR.

 Do you mean even kernel can not assign resource to them? or it takes so long 
 for
 mpt FW to get ready?
 
 This is not an issue from mpt FW.
 
 I have just kept the same description provide by Timothy in his
 initial patch.
 
 But I observe that their may be chance of getting unable to handle
 kernel NULL pointer dereference kernel panic if no Memory Resource
 available in the PCI subsystem. So agreed to the Timothy proposal of
 aborting the driver initialization if it doesn't detect any Memory
 resource instead of whole system get into panic state.
 

 Thanks

 Yinghai

On some systems Linux is unable / unwilling to assign a BAR if the BIOS
does not assign one at startup.  I didn't look into the Linux allocator
side of things in much detail, but it is quite possible that Linux is
unaware the device only has partial resources assigned.

-- 
Timothy Pearson
Raptor Engineering
+1 (415) 727-8645
http://www.raptorengineeringinc.com
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] scsi: fix hang in scsi error handling

[Kevin Groeneveld]

With the following setup/steps I can consistently trigger the scsi host to
hang requiring a reboot:
1. iMX6Q processor with built in AHCI compatible SATA host
2. SATA port multiplier in CBS mode connected to iMX6Q
3. HDD connected to port multiplier
4. CDROM connected to port multiplier
5. trigger continuous I/O to HDD
6. repeatedly execute CDROM_DRIVE_STATUS ioctl on CDROM with no disc in
   drive

I don't think this issue is iMX6 specific but that is the only platform
I have duplicated the hang on.

To trigger the issue at least two CPU cores must be enabled and the HDD
access and CDROM ioctls must be happening concurrently. If I only enable
one CPU core the hang does not occur.

The following C program can be used to trigger the CDROM ioctl:

#include stdio.h
#include fcntl.h
#include linux/cdrom.h

int main(int argc, char* argv[])
{
int fd;

fd = open(/dev/cdrom, O_RDONLY | O_NONBLOCK);
if(fd  0)
{
perror(cannot open /dev/cdrom);
return fd;
}

for(;;)
{
ioctl(fd, CDROM_DRIVE_STATUS, 0);
usleep(100 * 1000);
}
}

When the hang occurs shost-host_busy == 2 and shost-host_failed == 1 in
the scsi_eh_wakeup function. However this function only wakes the error
handler if host_busy == host_failed.

The patch changes the condition to test if host_busy = host_failed and
updates the corresponding condition in scsi_error_handler. Without the
patch I can trigger the hang within seconds. With the patch I have not
duplicated the hang after hours of testing.

Signed-off-by: Kevin Groeneveld kgroenev...@lenbrook.com
---
 drivers/scsi/scsi_error.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index 106884a..853964b 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -61,7 +61,7 @@ static int scsi_try_to_abort_cmd(struct scsi_host_template *,
 /* called with shost-host_lock held */
 void scsi_eh_wakeup(struct Scsi_Host *shost)
 {
-   if (atomic_read(shost-host_busy) == shost-host_failed) {
+   if (atomic_read(shost-host_busy) = shost-host_failed) {
trace_scsi_eh_wakeup(shost);
wake_up_process(shost-ehandler);
SCSI_LOG_ERROR_RECOVERY(5, shost_printk(KERN_INFO, shost,
@@ -2173,7 +2173,7 @@ int scsi_error_handler(void *data)
while (!kthread_should_stop()) {
set_current_state(TASK_INTERRUPTIBLE);
if ((shost-host_failed == 0  shost-host_eh_scheduled == 0) 
||
-   shost-host_failed != atomic_read(shost-host_busy)) {
+   shost-host_failed  atomic_read(shost-host_busy)) {
SCSI_LOG_ERROR_RECOVERY(1,
shost_printk(KERN_INFO, shost,
 scsi_eh_%d: sleeping\n,
-- 
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: dm-mpath: always return reservation conflict

[Mike Snitzer]

On Wed, Jul 15 2015 at  8:15am -0400,
Hannes Reinecke h...@suse.de wrote:

 On 07/15/2015 02:01 PM, James Bottomley wrote:
  On Wed, 2015-07-15 at 13:52 +0200, Hannes Reinecke wrote:
  On 07/15/2015 01:35 PM, James Bottomley wrote:
  On Wed, 2015-07-15 at 13:23 +0200, Hannes Reinecke wrote:
  If dm-mpath encounters an reservation conflict it should not
  fail the path (as communication with the target is not affected)
  but should rather retry on another path.
  However, in doing so we might be inducing a ping-pong between
  paths, with no guarantee of any forward progress.
  And arguably a reservation conflict is an unexpected error,
  so we should be passing it upwards to allow the application
  to take appropriate steps.
 
  If I interpret the code correctly, you've changed the behaviour from the
  current try all paths and fail them, ultimately passing the reservation
  conflict up if all paths fail to return reservation conflict
  immediately, keeping all paths running.  This assumes that the
  reservation isn't path specific because if we encounter a path specific
  reservation, you've altered the behaviour from route around to fail.
 
  That is correct.
  As mentioned in the path, the 'correct' solution would be to retry
  the offending I/O on another path.
  However, the current multipath design doesn't allow us to do that
  without failing the path first.
  If we were just retrying I/O on another path without failing the
  path first (and all paths would return a reservation conflict) we
  wouldn't know when we've exhausted all paths.
 
  The case I think the original code was for is SAN Volume controllers
  which use path specific SCSI-3 reservations effectively to do traffic
  control and allow favoured paths.  Have you verified that nothing we
  encounter in the enterprise uses path specific reservations for
  multipath shaping any more?
 
  Ah. That was some input I was looking for.
  With that patch I've assumed that persistent reservations are done
  primarily from userland / filesystem, where the reservation would
  effectively be done on a per-LUN basis.
  If it's being used from the storage array internally this is a
  different matter.
  (Although I'd be very interested how this behaviour would play
  together with applications which use persistent reservations
  internally; GPFS springs to mind here ...)
 
  But apparently this specific behaviour wasn't seen that often in the
  field; I certainly never got any customer reports about mysteriously
  failing paths.
  
  Have you already got this patch in SLES, if so, for how long?
  
 We haven't as of yet; I've come across this behaviour due to another
 issue. And before I were to put this into SLES I thought I should be
 asking those in the know ... persistent reservations _is_ an arcane
 topic, after all.
 I was just referring to the fact that I rarely got customer issues
 with persistent reservations; and those I get tend to be tape-centric.
 
  Anyway. I'll see if I can come up with something to restore the
  original behaviour.
  
  Or a way of verifying that nothing in the current enterprise uses path
  specific reservations ...  we can change the current behaviour as long
  as nothing notices.
  
 The only instance I know of is GPFS; someone in our company once
 wrote an HA agent using persistent reservations, but I'm not sure if
 it's deployed anywhere. But that agent is certainly aware of
 multipathing, and doesn't issue per-path reservations.
 (Well, actually it does, but it does it for every path :-)
 I would think the same goes for GPFS.
 
 Incidentally, the SVC docs have a section about persistent
 reservations, but do not mention anything about internal use.
 So if it does it'll be opaque to the user, otherwise I would assume
 it to be mentioned there.

The main consumer of SCSI PR that I'm aware of is fence_scsi.  I don't
have specifics on whether the Clustering layers that use fence_scsi
(e.g. pacemaker) ever make use of per-path SCSI PR (cc'ing Ryan O'hara
who AFAIK mainatins fence_scsi).

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

[Bjorn Helgaas]

On Wed, Jul 15, 2015 at 08:52:13AM -0500, Timothy Pearson wrote:
 On 07/15/2015 01:24 AM, Sreekanth Reddy wrote:
  On Tue, Jul 14, 2015 at 10:36:58PM -0700, Yinghai Lu wrote:
  On Tue, Jul 14, 2015 at 9:49 PM, Sreekanth Reddy
  sreekanth.re...@avagotech.com wrote:
  Driver crashes if the BIOS do not set up at least one
  memory I/O resource. This failure can happen if the device is too
  slow to respond during POST and is missed by the BIOS, but Linux
  then detects the device later in the boot process.
 
  But pci subsystem should assign resources to those unassigned BAR.
 
  Do you mean even kernel can not assign resource to them? or it takes so 
  long for
  mpt FW to get ready?
  
  This is not an issue from mpt FW.
  
  I have just kept the same description provide by Timothy in his
  initial patch.
  
  But I observe that their may be chance of getting unable to handle
  kernel NULL pointer dereference kernel panic if no Memory Resource
  available in the PCI subsystem. So agreed to the Timothy proposal of
  aborting the driver initialization if it doesn't detect any Memory
  resource instead of whole system get into panic state.
 
 On some systems Linux is unable / unwilling to assign a BAR if the BIOS
 does not assign one at startup.  I didn't look into the Linux allocator
 side of things in much detail, but it is quite possible that Linux is
 unaware the device only has partial resources assigned.

There might be a Linux PCI core bug if we don't assign a BAR, although
resource assignment can always fail, even in the absence of bugs.

But there is definitely a driver bug if it uses ioc-chip when it hasn't
been initialized.  That's what this patch seems to fix.

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