Re: [PATCH 1/1] include/scsi/osd_protocol.h: remove unnecessary __constant
On 06/01/2014 05:06 PM, Fabian Frederick wrote:
__constant_cpu_to_be16 converted to cpu_to_be16
This patch fixes checkpatch warnings:
WARNING: __constant_cpu_to_be16 should be cpu_to_be16
Cc: Boaz Harrosh bharr...@panasas.com
Cc: Benny Halevy bhal...@primarydata.com
Signed-off-by: Fabian Frederick f...@skynet.be
Ack-by: Boaz Harrosh bharr...@panasas.com
James please apply
Thanks
Boaz
---
include/scsi/osd_protocol.h | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/scsi/osd_protocol.h b/include/scsi/osd_protocol.h
index 25ac628..a2594af 100644
--- a/include/scsi/osd_protocol.h
+++ b/include/scsi/osd_protocol.h
@@ -263,16 +263,16 @@ static inline struct osd_cdb_head *osd_cdb_head(struct
osd_cdb *ocdb)
* Ex name = FORMAT_OSD we have OSD_ACT_FORMAT_OSD OSDv1_ACT_FORMAT_OSD
*/
#define OSD_ACT___(Name, Num) \
- OSD_ACT_##Name = __constant_cpu_to_be16(0x8880 + Num), \
- OSDv1_ACT_##Name = __constant_cpu_to_be16(0x8800 + Num),
+ OSD_ACT_##Name = cpu_to_be16(0x8880 + Num), \
+ OSDv1_ACT_##Name = cpu_to_be16(0x8800 + Num),
/* V2 only actions */
#define OSD_ACT_V2(Name, Num) \
- OSD_ACT_##Name = __constant_cpu_to_be16(0x8880 + Num),
+ OSD_ACT_##Name = cpu_to_be16(0x8880 + Num),
#define OSD_ACT_V1_V2(Name, Num1, Num2) \
- OSD_ACT_##Name = __constant_cpu_to_be16(Num2), \
- OSDv1_ACT_##Name = __constant_cpu_to_be16(Num1),
+ OSD_ACT_##Name = cpu_to_be16(Num2), \
+ OSDv1_ACT_##Name = cpu_to_be16(Num1),
enum osd_service_actions {
OSD_ACT_V2(OBJECT_STRUCTURE_CHECK, 0x00)
--
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] TARGET/sbc,loopback: Adjust command data length in case pi exists on the wire
In various areas of the code, it is assumed that
se_cmd-data_length describes pure data. In case
that protection information exists over the wire
(protect bits is are on) the target core decrease
the protection length from the data length (instead
of each transport peeking in the cdb).
Modify loopback transport to include protection information
in the transferred data length (like other scsi transports).
Signed-off-by: Sagi Grimberg sa...@mellanox.com
---
drivers/target/loopback/tcm_loop.c | 35 +++
drivers/target/target_core_sbc.c | 15 +--
2 files changed, 44 insertions(+), 6 deletions(-)
diff --git a/drivers/target/loopback/tcm_loop.c
b/drivers/target/loopback/tcm_loop.c
index c886ad1..9adde8d 100644
--- a/drivers/target/loopback/tcm_loop.c
+++ b/drivers/target/loopback/tcm_loop.c
@@ -179,7 +179,7 @@ static void tcm_loop_submission_work(struct work_struct
*work)
struct tcm_loop_hba *tl_hba;
struct tcm_loop_tpg *tl_tpg;
struct scatterlist *sgl_bidi = NULL;
- u32 sgl_bidi_count = 0;
+ u32 sgl_bidi_count = 0, data_len;
int rc;
tl_hba = *(struct tcm_loop_hba **)shost_priv(sc-device-host);
@@ -213,12 +213,39 @@ static void tcm_loop_submission_work(struct work_struct
*work)
}
- if (!scsi_prot_sg_count(sc) scsi_get_prot_op(sc) != SCSI_PROT_NORMAL)
- se_cmd-prot_pto = true;
+ data_len = scsi_bufflen(sc);
+ if (scsi_get_prot_op(sc) != SCSI_PROT_NORMAL) {
+ if (!scsi_prot_sg_count(sc))
+ se_cmd-prot_pto = true;
+ else {
+ /**
+* Adjust data_length to include
+* protection information
+**/
+ switch (sc-device-sector_size) {
+ case 512:
+ data_len += (data_len 9) * 8;
+ break;
+ case 1024:
+ data_len += (data_len 10) * 8;
+ break;
+ case 2048:
+ data_len += (data_len 11) * 8;
+ break;
+ case 4096:
+ data_len += (data_len 12) * 8;
+ break;
+ default:
+ data_len +=
+ (data_len ilog2(sc-device-sector_size)) *
8;
+ }
+ }
+ }
+
rc = target_submit_cmd_map_sgls(se_cmd, tl_nexus-se_sess, sc-cmnd,
tl_cmd-tl_sense_buf[0], tl_cmd-sc-device-lun,
- scsi_bufflen(sc), tcm_loop_sam_attr(sc),
+ data_len, tcm_loop_sam_attr(sc),
sc-sc_data_direction, 0,
scsi_sglist(sc), scsi_sg_count(sc),
sgl_bidi, sgl_bidi_count,
diff --git a/drivers/target/target_core_sbc.c b/drivers/target/target_core_sbc.c
index e022959..06f8ecd 100644
--- a/drivers/target/target_core_sbc.c
+++ b/drivers/target/target_core_sbc.c
@@ -665,8 +665,19 @@ sbc_check_prot(struct se_device *dev, struct se_cmd *cmd,
unsigned char *cdb,
cmd-prot_type = dev-dev_attrib.pi_prot_type;
cmd-prot_length = dev-prot_length * sectors;
- pr_debug(%s: prot_type=%d, prot_length=%d prot_op=%d prot_checks=%d\n,
-__func__, cmd-prot_type, cmd-prot_length,
+
+ /**
+* In case protection information exists over the wire
+* we modify command data length to describe pure data.
+* The actual transfer length is data length + protection
+* length
+**/
+ if (protect)
+ cmd-data_length -= cmd-prot_length;
+
+ pr_debug(%s: prot_type=%d, data_length=%d, prot_length=%d
+prot_op=%d prot_checks=%d\n,
+__func__, cmd-prot_type, cmd-data_length, cmd-prot_length,
cmd-prot_op, cmd-prot_checks);
return true;
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] Include protection information in iscsi header
At the SCSI transport level, there is no distinction between user data and protection information. Thus, iscsi header field expected data transfer length should include protection information. This set modifies both the iscsi initiator (patch #1), and target (patch #2) to expect data length which includes protection information. Although these patches involve 3 subsystems with different maintainers (scsi, iser, target) I would prefer seeing these patches included together. Sagi Grimberg (2): libiscsi, iser: Adjust data_length to include protection information TARGET/sbc,loopback: Adjust command data length in case pi exists on the wire drivers/infiniband/ulp/iser/iser_initiator.c | 34 +++- drivers/scsi/libiscsi.c | 35 +- drivers/target/loopback/tcm_loop.c | 35 +++--- drivers/target/target_core_sbc.c | 15 +- include/scsi/libiscsi.h | 19 ++ 5 files changed, 107 insertions(+), 31 deletions(-) -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] libiscsi, iser: Adjust data_length to include protection information
In case protection information exists over the wire
iscsi header data_length field is required to include it.
Also remove iser transfer length checks for each task as
they are not always true and somewhat redundant anyway.
Signed-off-by: Sagi Grimberg sa...@mellanox.com
---
drivers/infiniband/ulp/iser/iser_initiator.c | 34 +++-
drivers/scsi/libiscsi.c | 35 +-
include/scsi/libiscsi.h | 19 ++
3 files changed, 63 insertions(+), 25 deletions(-)
diff --git a/drivers/infiniband/ulp/iser/iser_initiator.c
b/drivers/infiniband/ulp/iser/iser_initiator.c
index 2e2d903..1600e35 100644
--- a/drivers/infiniband/ulp/iser/iser_initiator.c
+++ b/drivers/infiniband/ulp/iser/iser_initiator.c
@@ -41,11 +41,11 @@
#include iscsi_iser.h
/* Register user buffer memory and initialize passive rdma
- * dto descriptor. Total data size is stored in
- * iser_task-data[ISER_DIR_IN].data_len
+ * dto descriptor. Data size is stored in
+ * task-data[ISER_DIR_IN].data_len, Protection size
+ * os stored in task-prot[ISER_DIR_IN].data_len
*/
-static int iser_prepare_read_cmd(struct iscsi_task *task,
-unsigned int edtl)
+static int iser_prepare_read_cmd(struct iscsi_task *task)
{
struct iscsi_iser_task *iser_task = task-dd_data;
@@ -73,14 +73,6 @@ static int iser_prepare_read_cmd(struct iscsi_task *task,
return err;
}
- if (edtl iser_task-data[ISER_DIR_IN].data_len) {
- iser_err(Total data length: %ld, less than EDTL:
-%d, in READ cmd BHS itt: %d, conn: 0x%p\n,
-iser_task-data[ISER_DIR_IN].data_len, edtl,
-task-itt, iser_task-ib_conn);
- return -EINVAL;
- }
-
err = device-iser_reg_rdma_mem(iser_task, ISER_DIR_IN);
if (err) {
iser_err(Failed to set up Data-IN RDMA\n);
@@ -100,8 +92,9 @@ static int iser_prepare_read_cmd(struct iscsi_task *task,
}
/* Register user buffer memory and initialize passive rdma
- * dto descriptor. Total data size is stored in
- * task-data[ISER_DIR_OUT].data_len
+ * dto descriptor. Data size is stored in
+ * task-data[ISER_DIR_OUT].data_len, Protection size
+ * is stored at task-prot[ISER_DIR_OUT].data_len
*/
static int
iser_prepare_write_cmd(struct iscsi_task *task,
@@ -135,14 +128,6 @@ iser_prepare_write_cmd(struct iscsi_task *task,
return err;
}
- if (edtl iser_task-data[ISER_DIR_OUT].data_len) {
- iser_err(Total data length: %ld, less than EDTL: %d,
-in WRITE cmd BHS itt: %d, conn: 0x%p\n,
-iser_task-data[ISER_DIR_OUT].data_len,
-edtl, task-itt, task-conn);
- return -EINVAL;
- }
-
err = device-iser_reg_rdma_mem(iser_task, ISER_DIR_OUT);
if (err != 0) {
iser_err(Failed to register write cmd RDMA mem\n);
@@ -417,11 +402,12 @@ int iser_send_command(struct iscsi_conn *conn,
if (scsi_prot_sg_count(sc)) {
prot_buf-buf = scsi_prot_sglist(sc);
prot_buf-size = scsi_prot_sg_count(sc);
- prot_buf-data_len = sc-prot_sdb-length;
+ prot_buf-data_len = iscsi_prot_len(data_buf-data_len,
+ sc-device-sector_size);
}
if (hdr-flags ISCSI_FLAG_CMD_READ) {
- err = iser_prepare_read_cmd(task, edtl);
+ err = iser_prepare_read_cmd(task);
if (err)
goto send_command_error;
}
diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
index 26dc005..b54d1cc 100644
--- a/drivers/scsi/libiscsi.c
+++ b/drivers/scsi/libiscsi.c
@@ -326,6 +326,31 @@ static int iscsi_check_tmf_restrictions(struct iscsi_task
*task, int opcode)
}
/**
+ * iscsi_adjust_dl - Adjust SCSI data length to include PI
+ * @sc: scsi command.
+ * @data_length: command data length.
+ *
+ * Adjust the data length to account for how much data
+ * is actually on the wire.
+ *
+ * returns the adjusted data length
+ **/
+static unsigned
+iscsi_adjust_dl(struct scsi_cmnd *sc, unsigned data_len)
+{
+ if (sc-sc_data_direction == DMA_FROM_DEVICE) {
+ if (scsi_get_prot_op(sc) == SCSI_PROT_READ_INSERT)
+ return data_len;
+ } else {
+ if (scsi_get_prot_op(sc) == SCSI_PROT_WRITE_STRIP)
+ return data_len;
+ }
+
+ /* Protection information exists on the wire */
+ return data_len + iscsi_prot_len(data_len, sc-device-sector_size);
+}
+
+/**
* iscsi_prep_scsi_cmd_pdu - prep iscsi scsi cmd pdu
* @task: iscsi task
*
@@ -395,6 +420,9 @@ static int iscsi_prep_scsi_cmd_pdu(struct iscsi_task *task)
unsigned out_len =
RE: [PATCH 7/8] be2iscsi: Fix processing cqe for cxn whose endpoint is freed
-Original Message-
From: Mike Christie [mailto:micha...@cs.wisc.edu]
Sent: Thursday, May 08, 2014 3:49 AM
To: Jay Kallickal
Cc: jbottom...@parallels.com; linux-scsi@vger.kernel.org; Jayamohan Kallickal;
Minh Duc Tran; Sony John-N
Subject: Re: [PATCH 7/8] be2iscsi: Fix processing cqe for cxn whose endpoint is
freed
On 05/05/2014 08:41 PM, Jay Kallickal wrote:
From: Jayamohan Kallickal jayamohan.kallic...@emulex.com
During heavy IO in multipath environment with many active sessions
and port-bouncing happening, there is a race condition because of
which beiscsi_prcess_cqe() gets called for a connection whose
endpoint is freed.
Checking endpoint reference for a connection before processing in
beiscsi_process_cq().
Signed-off-by: Minh Tran minhduc.t...@emulex.com
Signed-off-by: John Soni Jose sony.joh...@emulex.com
Signed-off-by: Jayamohan Kallickal jayamohan.kallic...@emulex.com
---
drivers/scsi/be2iscsi/be_main.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/drivers/scsi/be2iscsi/be_main.c
b/drivers/scsi/be2iscsi/be_main.c index dccda6c..5a7022f 100644
--- a/drivers/scsi/be2iscsi/be_main.c
+++ b/drivers/scsi/be2iscsi/be_main.c
@@ -2110,6 +2110,16 @@ static unsigned int beiscsi_process_cq(struct
be_eq_obj *pbe_eq)
cri_index = BE_GET_CRI_FROM_CID(cid);
ep = phba-ep_array[cri_index];
+ if (unlikely(ep == NULL)) {
+ /* connection has already been freed
+ * just move on to next one
+ */
+ beiscsi_log(phba, KERN_WARNING,
+ BEISCSI_LOG_INIT,
+ BM_%d : proc cqe of disconn ep: cid %d\n,
+ cid);
+ goto proc_next_cqe;
+ }
beiscsi_ep = ep-dd_data;
beiscsi_conn = beiscsi_ep-conn;
It looks like if that race is possible then we could also free the ep while
you are accessing right? I think you would need to get a ref to the ep.
We will pull out this patch from this release. This is a very corner case and
requires changes to be done in the IO path of the driver. We will redo the
patch and submit in our next release.
Please pull-in all the other patches in this set expect this particular one
[PATCH 7/8] be2iscsi: Fix processing cqe for cxn whose endpoint is freed.
What command/function tells the card to stop sending the driver
events/notifications/ios for that connection? Is it beiscsi_close_conn or
mgmt_invalidate_connection?
Mgmt_invalidate_connection tell card to stop sending events to the driver for a
particular connection.
--
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
