Re: [PATCH 3/3] Make blk_cleanup_queue() wait until request_fn finished
On 10/01/12 19:41, Dan Williams wrote: On Thu, Sep 27, 2012 at 9:39 AM, Bart Van Assche bvanass...@acm.org wrote: [ ... ] diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index 593fc71..03571a3 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -1517,10 +1517,6 @@ static void scsi_request_fn(struct request_queue *q) struct scsi_cmnd *cmd; struct request *req; - if(!get_device(sdev-sdev_gendev)) - /* We must be tearing the block queue down already */ - return; - /* * To start with, we keep looping until the queue is empty, or until * the host is no longer able to accept any more requests. @@ -1629,11 +1625,7 @@ out_delay: if (sdev-device_busy == 0) blk_delay_queue(q, SCSI_QUEUE_DELAY); out: - /* must be careful here...if we trigger the -remove() function -* we cannot be holding the q lock */ - spin_unlock_irq(q-queue_lock); - put_device(sdev-sdev_gendev); - spin_lock_irq(q-queue_lock); + ; Any reason to keep this out: label now that it has no effect? Some people prefer a single-exit style for kernel code since that style makes it easy to add cleanup code for resources allocated inside the function itself. I don't have a strong opinion about this though. Note: after I posted this patch series I noticed that patch 2/3 leaves a (small) race window. I'm currently testing this follow-up patch: diff --git a/block/blk-core.c b/block/blk-core.c index f0efe32..3991f8e 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -425,6 +425,9 @@ void blk_drain_queue(struct request_queue *q, bool drain_all) } } + if (!drain blk_queue_dying(q)) + queue_flag_set(QUEUE_FLAG_DEAD, q); + spin_unlock_irq(q-queue_lock); if (!drain) @@ -525,10 +528,6 @@ void blk_cleanup_queue(struct request_queue *q) /* drain all requests queued before DEAD marking */ blk_drain_queue(q, true); - spin_lock_irq(lock); - queue_flag_set(QUEUE_FLAG_DEAD, q); - spin_unlock_irq(lock); - /* @q won't process any more request, flush async actions */ del_timer_sync(q-backing_dev_info.laptop_mode_wb_timer); blk_sync_queue(q); -- 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/4] target: Add target_submit_cmd_map_mem and convert tcm_loop+tcm_vhost
From: Nicholas Bellinger n...@linux-iscsi.org Hi hch Co, This series adds a new target_submit_cmd_map_mem() caller to accept pre-allocated SGL memory within the core generic I/O submission path. Patch #1 contains the core I/O changes, patch #2 + #4 includes the conversion of tcm_loop+tcm_vhost to use this new caller - drop their internal open-coded equivalents using transport_generic_map_mem_to_cmd(). Patch #3 carries forward a work-around for tcm_loop w/ scsi-generic with user-space code that does not zero out it's READ payload buffer + ends up passing a payload filled with random data into target core's control CDB emulation. Since we're not using bounce buffers any more for control CDB emulation in modern v3.x code, AFAICT tcm_loop still requires this extra bit to function properly with some legacy user-space code. Regardless, the main I/O path changes end up being very mechnical in nature for existing core and fabric code, and has been running as expected with fio small-block workloads last hours. Please review. Thanks Christoph! --nab Nicholas Bellinger (4): target: Add target_submit_cmd_map_mem for SGL fabric memory passthrough tcm_loop: Convert I/O path to use target_submit_cmd_map_mem target: Add TARGET_SCF_MAP_CLEAR_MEM work-around for tcm_loop tcm_vhost: Convert I/O path to use target_submit_cmd_map_mem drivers/target/loopback/tcm_loop.c | 62 ++- drivers/target/target_core_transport.c | 129 ++-- drivers/vhost/tcm_vhost.c | 68 - drivers/vhost/tcm_vhost.h |8 ++ include/target/target_core_base.h |2 + include/target/target_core_fabric.h|3 + 6 files changed, 141 insertions(+), 131 deletions(-) -- 1.7.2.5 -- 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/4] target: Add target_submit_cmd_map_mem for SGL fabric memory passthrough
From: Nicholas Bellinger n...@linux-iscsi.org
This patch adds a new TARGET_SCF_MAP_MEM flag within __target_submit_cmd()
+ a target_submit_cmd_map_mem() wrapper to pass pre-allocated SGL memory
using existing transport_generic_map_mem_to_cmd() logic into the generic
target submit I/O codepath.
Reported-by: Christoph Hellwig h...@lst.de
Cc: Christoph Hellwig h...@lst.de
Signed-off-by: Nicholas Bellinger n...@linux-iscsi.org
---
drivers/target/target_core_transport.c | 108 +---
include/target/target_core_base.h |1 +
include/target/target_core_fabric.h|3 +
3 files changed, 89 insertions(+), 23 deletions(-)
diff --git a/drivers/target/target_core_transport.c
b/drivers/target/target_core_transport.c
index 221f67f..ad2097e 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -1455,29 +1455,11 @@ int transport_handle_cdb_direct(
}
EXPORT_SYMBOL(transport_handle_cdb_direct);
-/**
- * target_submit_cmd - lookup unpacked lun and submit uninitialized se_cmd
- *
- * @se_cmd: command descriptor to submit
- * @se_sess: associated se_sess for endpoint
- * @cdb: pointer to SCSI CDB
- * @sense: pointer to SCSI sense buffer
- * @unpacked_lun: unpacked LUN to reference for struct se_lun
- * @data_length: fabric expected data transfer length
- * @task_addr: SAM task attribute
- * @data_dir: DMA data direction
- * @flags: flags for command submission from target_sc_flags_tables
- *
- * Returns non zero to signal active I/O shutdown failure. All other
- * setup exceptions will be returned as a SCSI CHECK_CONDITION response,
- * but still return zero here.
- *
- * This may only be called from process context, and also currently
- * assumes internal allocation of fabric payload buffer by target-core.
- **/
-int target_submit_cmd(struct se_cmd *se_cmd, struct se_session *se_sess,
+static int __target_submit_cmd(struct se_cmd *se_cmd, struct se_session
*se_sess,
unsigned char *cdb, unsigned char *sense, u32 unpacked_lun,
- u32 data_length, int task_attr, int data_dir, int flags)
+ u32 data_length, int task_attr, int data_dir, int flags,
+ struct scatterlist *sgl, u32 sgl_count,
+ struct scatterlist *sgl_bidi, u32 sgl_bidi_count)
{
struct se_portal_group *se_tpg;
int rc;
@@ -1524,7 +1506,19 @@ int target_submit_cmd(struct se_cmd *se_cmd, struct
se_session *se_sess,
transport_generic_request_failure(se_cmd);
return 0;
}
-
+ /*
+* When TARGET_SCF_MAP_MEM has been set perform a SGL passthrough
+* mapping for pre-allocated fabric memory, instead of having target
+* core perform an internal SGL allocation.
+*/
+ if (flags TARGET_SCF_MAP_MEM) {
+ rc = transport_generic_map_mem_to_cmd(se_cmd, sgl, sgl_count,
+ sgl_bidi, sgl_bidi_count);
+ if (rc != 0) {
+ transport_generic_request_failure(se_cmd);
+ return 0;
+ }
+ }
/*
* Check if we need to delay processing because of ALUA
* Active/NonOptimized primary access state..
@@ -1534,8 +1528,76 @@ int target_submit_cmd(struct se_cmd *se_cmd, struct
se_session *se_sess,
transport_handle_cdb_direct(se_cmd);
return 0;
}
+
+/*
+ * target_submit_cmd - lookup unpacked lun and submit uninitialized se_cmd
+ *
+ * @se_cmd: command descriptor to submit
+ * @se_sess: associated se_sess for endpoint
+ * @cdb: pointer to SCSI CDB
+ * @sense: pointer to SCSI sense buffer
+ * @unpacked_lun: unpacked LUN to reference for struct se_lun
+ * @data_length: fabric expected data transfer length
+ * @task_addr: SAM task attribute
+ * @data_dir: DMA data direction
+ * @flags: flags for command submission from target_sc_flags_tables
+ *
+ * Returns non zero to signal active I/O shutdown failure. All other
+ * setup exceptions will be returned as a SCSI CHECK_CONDITION response,
+ * but still return zero here.
+ *
+ * This may only be called from process context, and also currently
+ * assumes internal allocation of fabric payload buffer by target-core.
+ */
+int target_submit_cmd(struct se_cmd *se_cmd, struct se_session *se_sess,
+ unsigned char *cdb, unsigned char *sense, u32 unpacked_lun,
+ u32 data_length, int task_attr, int data_dir, int flags)
+{
+ return __target_submit_cmd(se_cmd, se_sess, cdb, sense, unpacked_lun,
+ data_length, task_attr, data_dir, flags,
+ NULL, 0, NULL, 0);
+}
EXPORT_SYMBOL(target_submit_cmd);
+/*
+ * target_submit_cmd_map_mem - lookup unpacked lun and submit uninitialized
+ *se_cmd using pre-allocated memory.
+ *
+ * @se_cmd: command descriptor to submit
+ * @se_sess: associated se_sess for endpoint
+ * @cdb: pointer to SCSI CDB
+ * @sense:
[PATCH 3/4] target: Add TARGET_SCF_MAP_CLEAR_MEM work-around for tcm_loop
From: Nicholas Bellinger n...@linux-iscsi.org
This patch carries forward a work-around from tcm_loop to target
core code to explicitly clear control CDB READ paylods in order to
avoid bugs in scsi-generic user-space code for INQUIRY that do not
explicitly zero CDB payload memory.
Cc: Christoph Hellwig h...@lst.de
Signed-off-by: Nicholas Bellinger n...@linux-iscsi.org
---
drivers/target/loopback/tcm_loop.c |2 +-
drivers/target/target_core_transport.c | 21 +
include/target/target_core_base.h |1 +
3 files changed, 23 insertions(+), 1 deletions(-)
diff --git a/drivers/target/loopback/tcm_loop.c
b/drivers/target/loopback/tcm_loop.c
index e20b809..911381f 100644
--- a/drivers/target/loopback/tcm_loop.c
+++ b/drivers/target/loopback/tcm_loop.c
@@ -198,7 +198,7 @@ static void tcm_loop_submission_work(struct work_struct
*work)
rc = target_submit_cmd_map_mem(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),
- sc-sc_data_direction, 0,
+ sc-sc_data_direction, TARGET_SCF_MAP_CLEAR_MEM,
scsi_sglist(sc), scsi_sg_count(sc),
sgl_bidi, sgl_bidi_count);
if (rc 0) {
diff --git a/drivers/target/target_core_transport.c
b/drivers/target/target_core_transport.c
index ad2097e..665ace5 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -1512,6 +1512,27 @@ static int __target_submit_cmd(struct se_cmd *se_cmd,
struct se_session *se_sess
* core perform an internal SGL allocation.
*/
if (flags TARGET_SCF_MAP_MEM) {
+ /*
+* A work-around for tcm_loop as some userspace code via
+* scsi-generic do not memset their associated read buffers,
+* so go ahead and do that here for type non-data CDBs. Also
+* note that this is currently guaranteed to be a single SGL
+* for this case by target core in target_setup_cmd_from_cdb()
+* - transport_generic_cmd_sequencer().
+*/
+ if (flags TARGET_SCF_MAP_CLEAR_MEM
+ !(se_cmd-se_cmd_flags SCF_SCSI_DATA_CDB)
+ se_cmd-data_direction == DMA_FROM_DEVICE) {
+ unsigned char *buf = NULL;
+
+ if (sgl)
+ buf = kmap(sg_page(sgl)) + sgl-offset;
+
+ if (buf) {
+ memset(buf, 0, sgl-length);
+ kunmap(sg_page(sgl));
+ }
+ }
rc = transport_generic_map_mem_to_cmd(se_cmd, sgl, sgl_count,
sgl_bidi, sgl_bidi_count);
if (rc != 0) {
diff --git a/include/target/target_core_base.h
b/include/target/target_core_base.h
index 6309298..f660ad2 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -221,6 +221,7 @@ enum target_sc_flags_table {
TARGET_SCF_ACK_KREF = 0x02,
TARGET_SCF_UNKNOWN_SIZE = 0x04,
TARGET_SCF_MAP_MEM = 0x08,
+ TARGET_SCF_MAP_CLEAR_MEM= 0x10,
};
/* fabric independent task management function values */
--
1.7.2.5
--
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/4] tcm_loop: Convert I/O path to use target_submit_cmd_map_mem
From: Nicholas Bellinger n...@linux-iscsi.org
This patch converts tcm_loop to use target_submit_cmd_map_mem() for
I/O submission and mapping of pre-allocated SGL memory from incoming
scsi_cmnd - se_cmd descriptors.
This includes removing the original open-coded fabric uses of target
core callers to support transport_generic_map_mem_to_cmd() between
target_setup_cmd_from_cdb() and transport_handle_cdb_direct() logic.
Reported-by: Christoph Hellwig h...@lst.de
Cc: Christoph Hellwig h...@lst.de
Signed-off-by: Nicholas Bellinger n...@linux-iscsi.org
---
drivers/target/loopback/tcm_loop.c | 62 +---
1 files changed, 8 insertions(+), 54 deletions(-)
diff --git a/drivers/target/loopback/tcm_loop.c
b/drivers/target/loopback/tcm_loop.c
index 7a0da1a..e20b809 100644
--- a/drivers/target/loopback/tcm_loop.c
+++ b/drivers/target/loopback/tcm_loop.c
@@ -166,7 +166,7 @@ static void tcm_loop_submission_work(struct work_struct
*work)
struct tcm_loop_tpg *tl_tpg;
struct scatterlist *sgl_bidi = NULL;
u32 sgl_bidi_count = 0;
- int ret;
+ int rc;
tl_hba = *(struct tcm_loop_hba **)shost_priv(sc-device-host);
tl_tpg = tl_hba-tl_hba_tpgs[sc-device-id];
@@ -187,12 +187,6 @@ static void tcm_loop_submission_work(struct work_struct
*work)
set_host_byte(sc, DID_ERROR);
goto out_done;
}
-
- transport_init_se_cmd(se_cmd, tl_tpg-tl_se_tpg.se_tpg_tfo,
- tl_nexus-se_sess,
- scsi_bufflen(sc), sc-sc_data_direction,
- tcm_loop_sam_attr(sc), tl_cmd-tl_sense_buf[0]);
-
if (scsi_bidi_cmnd(sc)) {
struct scsi_data_buffer *sdb = scsi_in(sc);
@@ -201,56 +195,16 @@ static void tcm_loop_submission_work(struct work_struct
*work)
se_cmd-se_cmd_flags |= SCF_BIDI;
}
-
- if (transport_lookup_cmd_lun(se_cmd, tl_cmd-sc-device-lun) 0) {
- kmem_cache_free(tcm_loop_cmd_cache, tl_cmd);
+ rc = target_submit_cmd_map_mem(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),
+ sc-sc_data_direction, 0,
+ scsi_sglist(sc), scsi_sg_count(sc),
+ sgl_bidi, sgl_bidi_count);
+ if (rc 0) {
set_host_byte(sc, DID_NO_CONNECT);
goto out_done;
}
-
- /*
-* Because some userspace code via scsi-generic do not memset their
-* associated read buffers, go ahead and do that here for type
-* non-data CDBs. Also note that this is currently guaranteed to be a
-* single SGL for this case by target core in
-* target_setup_cmd_from_cdb() - transport_generic_cmd_sequencer().
-*/
- if (!(se_cmd-se_cmd_flags SCF_SCSI_DATA_CDB)
- se_cmd-data_direction == DMA_FROM_DEVICE) {
- struct scatterlist *sg = scsi_sglist(sc);
- unsigned char *buf = kmap(sg_page(sg)) + sg-offset;
-
- if (buf != NULL) {
- memset(buf, 0, sg-length);
- kunmap(sg_page(sg));
- }
- }
-
- ret = target_setup_cmd_from_cdb(se_cmd, sc-cmnd);
- if (ret == -ENOMEM) {
- transport_send_check_condition_and_sense(se_cmd,
- TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE, 0);
- transport_generic_free_cmd(se_cmd, 0);
- return;
- } else if (ret 0) {
- if (se_cmd-se_cmd_flags SCF_SCSI_RESERVATION_CONFLICT)
- tcm_loop_queue_status(se_cmd);
- else
- transport_send_check_condition_and_sense(se_cmd,
- se_cmd-scsi_sense_reason, 0);
- transport_generic_free_cmd(se_cmd, 0);
- return;
- }
-
- ret = transport_generic_map_mem_to_cmd(se_cmd, scsi_sglist(sc),
- scsi_sg_count(sc), sgl_bidi, sgl_bidi_count);
- if (ret) {
- transport_send_check_condition_and_sense(se_cmd,
- se_cmd-scsi_sense_reason, 0);
- transport_generic_free_cmd(se_cmd, 0);
- return;
- }
- transport_handle_cdb_direct(se_cmd);
return;
out_done:
--
1.7.2.5
--
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 4/4] tcm_vhost: Convert I/O path to use target_submit_cmd_map_mem
From: Nicholas Bellinger n...@linux-iscsi.org
This patch converts tcm_vhost to use target_submit_cmd_map_mem() for
I/O submission and mapping of pre-allocated SGL memory from incoming
virtio-scsi SGL memory - se_cmd descriptors.
This includes removing the original open-coded fabric uses of target
core callers to support transport_generic_map_mem_to_cmd() between
target_setup_cmd_from_cdb() and transport_handle_cdb_direct() logic.
It also includes adding a handful of new tcm_vhost_cmnd member +
assignments in vhost_scsi_allocate_cmd() used from cmwq process
context I/O submission within tcm_vhost_submission_work()
Reported-by: Christoph Hellwig h...@lst.de
Cc: Christoph Hellwig h...@lst.de
Cc: Michael S. Tsirkin m...@redhat.com
Cc: Paolo Bonzini pbonz...@redhat.com
Cc: Stefan Hajnoczi stefa...@gmail.com
Signed-off-by: Nicholas Bellinger n...@linux-iscsi.org
---
drivers/vhost/tcm_vhost.c | 68 +---
drivers/vhost/tcm_vhost.h |8 +
2 files changed, 22 insertions(+), 54 deletions(-)
diff --git a/drivers/vhost/tcm_vhost.c b/drivers/vhost/tcm_vhost.c
index 89dc99b..2ca5f02 100644
--- a/drivers/vhost/tcm_vhost.c
+++ b/drivers/vhost/tcm_vhost.c
@@ -415,10 +415,7 @@ static struct tcm_vhost_cmd *vhost_scsi_allocate_cmd(
{
struct tcm_vhost_cmd *tv_cmd;
struct tcm_vhost_nexus *tv_nexus;
- struct se_portal_group *se_tpg = tv_tpg-se_tpg;
struct se_session *se_sess;
- struct se_cmd *se_cmd;
- int sam_task_attr;
tv_nexus = tv_tpg-tpg_nexus;
if (!tv_nexus) {
@@ -434,23 +431,11 @@ static struct tcm_vhost_cmd *vhost_scsi_allocate_cmd(
}
INIT_LIST_HEAD(tv_cmd-tvc_completion_list);
tv_cmd-tvc_tag = v_req-tag;
+ tv_cmd-tvc_task_attr = v_req-task_attr;
+ tv_cmd-tvc_exp_data_len = exp_data_len;
+ tv_cmd-tvc_data_direction = data_direction;
+ tv_cmd-tvc_nexus = tv_nexus;
- se_cmd = tv_cmd-tvc_se_cmd;
- /*
-* Locate the SAM Task Attr from virtio_scsi_cmd_req
-*/
- sam_task_attr = v_req-task_attr;
- /*
-* Initialize struct se_cmd descriptor from TCM infrastructure
-*/
- transport_init_se_cmd(se_cmd, se_tpg-se_tpg_tfo, se_sess, exp_data_len,
- data_direction, sam_task_attr,
- tv_cmd-tvc_sense_buf[0]);
-
-#if 0 /* FIXME: vhost_scsi_allocate_cmd() BIDI operation */
- if (bidi)
- se_cmd-se_cmd_flags |= SCF_BIDI;
-#endif
return tv_cmd;
}
@@ -549,37 +534,10 @@ static void tcm_vhost_submission_work(struct work_struct
*work)
{
struct tcm_vhost_cmd *tv_cmd =
container_of(work, struct tcm_vhost_cmd, work);
+ struct tcm_vhost_nexus *tv_nexus;
struct se_cmd *se_cmd = tv_cmd-tvc_se_cmd;
struct scatterlist *sg_ptr, *sg_bidi_ptr = NULL;
int rc, sg_no_bidi = 0;
- /*
-* Locate the struct se_lun pointer based on v_req-lun, and
-* attach it to struct se_cmd
-*/
- rc = transport_lookup_cmd_lun(tv_cmd-tvc_se_cmd, tv_cmd-tvc_lun);
- if (rc 0) {
- pr_err(Failed to look up lun: %d\n, tv_cmd-tvc_lun);
- transport_send_check_condition_and_sense(tv_cmd-tvc_se_cmd,
- tv_cmd-tvc_se_cmd.scsi_sense_reason, 0);
- transport_generic_free_cmd(se_cmd, 0);
- return;
- }
-
- rc = target_setup_cmd_from_cdb(se_cmd, tv_cmd-tvc_cdb);
- if (rc == -ENOMEM) {
- transport_send_check_condition_and_sense(se_cmd,
- TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE, 0);
- transport_generic_free_cmd(se_cmd, 0);
- return;
- } else if (rc 0) {
- if (se_cmd-se_cmd_flags SCF_SCSI_RESERVATION_CONFLICT)
- tcm_vhost_queue_status(se_cmd);
- else
- transport_send_check_condition_and_sense(se_cmd,
- se_cmd-scsi_sense_reason, 0);
- transport_generic_free_cmd(se_cmd, 0);
- return;
- }
if (tv_cmd-tvc_sgl_count) {
sg_ptr = tv_cmd-tvc_sgl;
@@ -597,17 +555,19 @@ static void tcm_vhost_submission_work(struct work_struct
*work)
} else {
sg_ptr = NULL;
}
-
- rc = transport_generic_map_mem_to_cmd(se_cmd, sg_ptr,
- tv_cmd-tvc_sgl_count, sg_bidi_ptr,
- sg_no_bidi);
+ tv_nexus = tv_cmd-tvc_nexus;
+
+ rc = target_submit_cmd_map_mem(se_cmd, tv_nexus-tvn_se_sess,
+ tv_cmd-tvc_cdb, tv_cmd-tvc_sense_buf[0],
+ tv_cmd-tvc_lun, tv_cmd-tvc_exp_data_len,
+ tv_cmd-tvc_task_attr, tv_cmd-tvc_data_direction,
+ 0, sg_ptr, tv_cmd-tvc_sgl_count,
+ sg_bidi_ptr,
[PATCH v2] qla2xxx: silence two GCC warnings
Compiling qla_gs.o (part of the qla2xxx module) triggers two GCC warnings: drivers/scsi/qla2xxx/qla_gs.c: In function ‘qla2x00_fdmi_rhba’: drivers/scsi/qla2xxx/qla_gs.c:1339:7: warning: array subscript is above array bounds [-Warray-bounds] drivers/scsi/qla2xxx/qla_gs.c: In function ‘qla2x00_fdmi_register’: drivers/scsi/qla2xxx/qla_gs.c:1663:15: warning: array subscript is above array bounds [-Warray-bounds] It seems that the sequence of a strcpy followed by a strlen confuses GCC when it is keeping track of array bounds here. (It is not clear to me which array triggers this warning and by how much GCC thinks the subscript is above its bounds. Neither is it clear to me why comparable code in these two functions doesn't trigger this warning.) An easy way to silence these warnings is to use preprocessor macros here, as that apparently gives GCC enough information to keep track of array bounds. Signed-off-by: Paul Bolle pebo...@tiscali.nl --- 0) Rolf suggested to not use magic constants, to make sure things keep working when these strings change in the future. A trivial solution is to use preprocessor macros. I needed to add one for the manufacturer string. 1) Still only compile tested. drivers/scsi/qla2xxx/qla_def.h | 1 + drivers/scsi/qla2xxx/qla_gs.c | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/scsi/qla2xxx/qla_def.h b/drivers/scsi/qla2xxx/qla_def.h index 39007f5..8895038 100644 --- a/drivers/scsi/qla2xxx/qla_def.h +++ b/drivers/scsi/qla2xxx/qla_def.h @@ -37,6 +37,7 @@ #include qla_nx.h #define QLA2XXX_DRIVER_NAMEqla2xxx #define QLA2XXX_APIDEV ql2xapidev +#define QLA2XXX_MANUFACTURER QLogic Corporation /* * We have MAILBOX_REGISTER_COUNT sized arrays in a few places, diff --git a/drivers/scsi/qla2xxx/qla_gs.c b/drivers/scsi/qla2xxx/qla_gs.c index 05260d2..1714035 100644 --- a/drivers/scsi/qla2xxx/qla_gs.c +++ b/drivers/scsi/qla2xxx/qla_gs.c @@ -1325,8 +1325,8 @@ qla2x00_fdmi_rhba(scsi_qla_host_t *vha) /* Manufacturer. */ eiter = (struct ct_fdmi_hba_attr *) (entries + size); eiter-type = __constant_cpu_to_be16(FDMI_HBA_MANUFACTURER); - strcpy(eiter-a.manufacturer, QLogic Corporation); - alen = strlen(eiter-a.manufacturer); + strcpy(eiter-a.manufacturer, QLA2XXX_MANUFACTURER); + alen = strlen(QLA2XXX_MANUFACTURER); alen += (alen 3) ? (4 - (alen 3)) : 4; eiter-len = cpu_to_be16(4 + alen); size += 4 + alen; @@ -1647,7 +1647,7 @@ qla2x00_fdmi_rpa(scsi_qla_host_t *vha) eiter = (struct ct_fdmi_port_attr *) (entries + size); eiter-type = __constant_cpu_to_be16(FDMI_PORT_OS_DEVICE_NAME); strcpy(eiter-a.os_dev_name, QLA2XXX_DRIVER_NAME); - alen = strlen(eiter-a.os_dev_name); + alen = strlen(QLA2XXX_DRIVER_NAME); alen += (alen 3) ? (4 - (alen 3)) : 4; eiter-len = cpu_to_be16(4 + alen); size += 4 + alen; -- 1.7.11.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 0/2] virtio-scsi fixes for 3.6
On Mon, 2012-10-01 at 15:11 +0200, Paolo Bonzini wrote: Il 26/07/2012 15:28, Paolo Bonzini ha scritto: James, patch 1 fixes scanning of LUNs whose number is greater than 255. QEMU passes a max_lun of 16383 (because it uses SAM numbering) but in Linux it must become 32768 (because LUNs above 255 are relocated to 16640). Patch 2 is a resubmission of the patch for online resizing of virtio-scsi LUNs, which needs to be rebased. LUNs above 255 now work for all of scanning, hotplug, hotunplug and resize. Thanks, Paolo Paolo Bonzini (2): virtio-scsi: fix LUNs greater than 255 virtio-scsi: support online resizing of disks drivers/scsi/virtio_scsi.c | 37 +++-- include/linux/virtio_scsi.h |2 ++ 2 files changed, 37 insertions(+), 2 deletions(-) Ping, are these patches going into 3.7? They're 3.7 candidates yes (enhancements certainly aren't 3.6). I seem to have become lost with the virtio-scsi updates since what I have marked for inclusion is a patch series that's a partial intersection with this. I'll flush my queue for virto-scsi, please resend all the missing patches you want in 3.7. Thanks, 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] target/iscsi: precedence bug in iscsit_set_dataout_sequence_values()
Clang warns about this bug:
drivers/target/iscsi/iscsi_target_erl0.c:52:45: warning: operator '?:'
has lower precedence than '+'; '+' will be evaluated first
[-Wparentheses]
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
---
Please review this very carefully because I haven't tested it. It could
be that the check should be:
(data_done + data_length FirstBurstLength) ? FirstBurstLength : data_length);
Instead of what I have which is:
data_done + (data_length FirstBurstLength ? FirstBurstLength : data_length);
diff --git a/drivers/target/iscsi/iscsi_target_erl0.c
b/drivers/target/iscsi/iscsi_target_erl0.c
index 1a02016..2067efd 100644
--- a/drivers/target/iscsi/iscsi_target_erl0.c
+++ b/drivers/target/iscsi/iscsi_target_erl0.c
@@ -48,9 +48,9 @@ void iscsit_set_dataout_sequence_values(
if (cmd-unsolicited_data) {
cmd-seq_start_offset = cmd-write_data_done;
cmd-seq_end_offset = (cmd-write_data_done +
- (cmd-se_cmd.data_length
-conn-sess-sess_ops-FirstBurstLength) ?
- conn-sess-sess_ops-FirstBurstLength :
cmd-se_cmd.data_length);
+ ((cmd-se_cmd.data_length
+ conn-sess-sess_ops-FirstBurstLength) ?
+conn-sess-sess_ops-FirstBurstLength :
cmd-se_cmd.data_length));
return;
}
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] virtio-scsi fixes for 3.6
Il 02/10/2012 10:18, James Bottomley ha scritto: On Mon, 2012-10-01 at 15:11 +0200, Paolo Bonzini wrote: Il 26/07/2012 15:28, Paolo Bonzini ha scritto: James, patch 1 fixes scanning of LUNs whose number is greater than 255. QEMU passes a max_lun of 16383 (because it uses SAM numbering) but in Linux it must become 32768 (because LUNs above 255 are relocated to 16640). Patch 2 is a resubmission of the patch for online resizing of virtio-scsi LUNs, which needs to be rebased. LUNs above 255 now work for all of scanning, hotplug, hotunplug and resize. Thanks, Paolo Paolo Bonzini (2): virtio-scsi: fix LUNs greater than 255 virtio-scsi: support online resizing of disks drivers/scsi/virtio_scsi.c | 37 +++-- include/linux/virtio_scsi.h |2 ++ 2 files changed, 37 insertions(+), 2 deletions(-) Ping, are these patches going into 3.7? They're 3.7 candidates yes (enhancements certainly aren't 3.6). I seem to have become lost with the virtio-scsi updates since what I have marked for inclusion is a patch series that's a partial intersection with this. I'll flush my queue for virto-scsi, please resend all the missing patches you want in 3.7. Ok, will do so soon and tag the appropriate ones for stable. Paolo -- 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] target/iscsi: precedence bug in iscsit_set_dataout_sequence_values()
Am 02.10.2012 10:22, schrieb Dan Carpenter:
Clang warns about this bug:
drivers/target/iscsi/iscsi_target_erl0.c:52:45: warning: operator '?:'
has lower precedence than '+'; '+' will be evaluated first
[-Wparentheses]
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
---
Please review this very carefully because I haven't tested it. It could
be that the check should be:
(data_done + data_length FirstBurstLength) ? FirstBurstLength :
data_length);
Instead of what I have which is:
data_done + (data_length FirstBurstLength ? FirstBurstLength : data_length);
diff --git a/drivers/target/iscsi/iscsi_target_erl0.c
b/drivers/target/iscsi/iscsi_target_erl0.c
index 1a02016..2067efd 100644
--- a/drivers/target/iscsi/iscsi_target_erl0.c
+++ b/drivers/target/iscsi/iscsi_target_erl0.c
@@ -48,9 +48,9 @@ void iscsit_set_dataout_sequence_values(
if (cmd-unsolicited_data) {
cmd-seq_start_offset = cmd-write_data_done;
cmd-seq_end_offset = (cmd-write_data_done +
- (cmd-se_cmd.data_length
- conn-sess-sess_ops-FirstBurstLength) ?
- conn-sess-sess_ops-FirstBurstLength :
cmd-se_cmd.data_length);
+ ((cmd-se_cmd.data_length
+ conn-sess-sess_ops-FirstBurstLength) ?
+ conn-sess-sess_ops-FirstBurstLength :
cmd-se_cmd.data_length));
return;
}
the ?: operator is nice but at a certain length is starts to become unreadable,
the end is normally calculated by end= start+len; Therefor i suggest:
if ( cmd-se_cmd.data_length conn-sess-sess_ops-FirstBurstLength )
cmd-seq_end_offset = cmd-seq_start_offset +
conn-sess-sess_ops-FirstBurstLength;
else
cmd-seq_end_offset = cmd-seq_start_offset + cmd-se_cmd.data_length;
just my 2 cents,
re,
wh
--
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] target/iscsi: precedence bug in iscsit_set_dataout_sequence_values()
On Tue, Oct 02, 2012 at 01:48:57PM +0200, walter harms wrote:
Am 02.10.2012 10:22, schrieb Dan Carpenter:
Clang warns about this bug:
drivers/target/iscsi/iscsi_target_erl0.c:52:45: warning: operator '?:'
has lower precedence than '+'; '+' will be evaluated first
[-Wparentheses]
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
---
Please review this very carefully because I haven't tested it. It could
be that the check should be:
(data_done + data_length FirstBurstLength) ? FirstBurstLength :
data_length);
Instead of what I have which is:
data_done + (data_length FirstBurstLength ? FirstBurstLength :
data_length);
diff --git a/drivers/target/iscsi/iscsi_target_erl0.c
b/drivers/target/iscsi/iscsi_target_erl0.c
index 1a02016..2067efd 100644
--- a/drivers/target/iscsi/iscsi_target_erl0.c
+++ b/drivers/target/iscsi/iscsi_target_erl0.c
@@ -48,9 +48,9 @@ void iscsit_set_dataout_sequence_values(
if (cmd-unsolicited_data) {
cmd-seq_start_offset = cmd-write_data_done;
cmd-seq_end_offset = (cmd-write_data_done +
- (cmd-se_cmd.data_length
-conn-sess-sess_ops-FirstBurstLength) ?
- conn-sess-sess_ops-FirstBurstLength :
cmd-se_cmd.data_length);
+ ((cmd-se_cmd.data_length
+ conn-sess-sess_ops-FirstBurstLength) ?
+conn-sess-sess_ops-FirstBurstLength :
cmd-se_cmd.data_length));
return;
}
the ?: operator is nice but at a certain length is starts to become
unreadable,
the end is normally calculated by end= start+len; Therefor i suggest:
if ( cmd-se_cmd.data_length conn-sess-sess_ops-FirstBurstLength )
cmd-seq_end_offset = cmd-seq_start_offset +
conn-sess-sess_ops-FirstBurstLength;
else
cmd-seq_end_offset = cmd-seq_start_offset + cmd-se_cmd.data_length;
Yeah. It's not beautiful.
Doing a struct iscsi_sess_ops *ops = conn-sess-sess_ops; at the
start of the function would help as well.
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] tcm_vhost: Convert I/O path to use target_submit_cmd_map_mem
On Tue, Oct 02, 2012 at 07:15:47AM +, Nicholas A. Bellinger wrote:
From: Nicholas Bellinger n...@linux-iscsi.org
This patch converts tcm_vhost to use target_submit_cmd_map_mem() for
I/O submission and mapping of pre-allocated SGL memory from incoming
virtio-scsi SGL memory - se_cmd descriptors.
This includes removing the original open-coded fabric uses of target
core callers to support transport_generic_map_mem_to_cmd() between
target_setup_cmd_from_cdb() and transport_handle_cdb_direct() logic.
It also includes adding a handful of new tcm_vhost_cmnd member +
assignments in vhost_scsi_allocate_cmd() used from cmwq process
context I/O submission within tcm_vhost_submission_work()
Reported-by: Christoph Hellwig h...@lst.de
Cc: Christoph Hellwig h...@lst.de
Cc: Michael S. Tsirkin m...@redhat.com
Cc: Paolo Bonzini pbonz...@redhat.com
Cc: Stefan Hajnoczi stefa...@gmail.com
Signed-off-by: Nicholas Bellinger n...@linux-iscsi.org
---
drivers/vhost/tcm_vhost.c | 68 +---
drivers/vhost/tcm_vhost.h |8 +
2 files changed, 22 insertions(+), 54 deletions(-)
diff --git a/drivers/vhost/tcm_vhost.c b/drivers/vhost/tcm_vhost.c
index 89dc99b..2ca5f02 100644
--- a/drivers/vhost/tcm_vhost.c
+++ b/drivers/vhost/tcm_vhost.c
@@ -415,10 +415,7 @@ static struct tcm_vhost_cmd *vhost_scsi_allocate_cmd(
{
struct tcm_vhost_cmd *tv_cmd;
struct tcm_vhost_nexus *tv_nexus;
- struct se_portal_group *se_tpg = tv_tpg-se_tpg;
struct se_session *se_sess;
- struct se_cmd *se_cmd;
- int sam_task_attr;
tv_nexus = tv_tpg-tpg_nexus;
if (!tv_nexus) {
@@ -434,23 +431,11 @@ static struct tcm_vhost_cmd *vhost_scsi_allocate_cmd(
}
INIT_LIST_HEAD(tv_cmd-tvc_completion_list);
tv_cmd-tvc_tag = v_req-tag;
+ tv_cmd-tvc_task_attr = v_req-task_attr;
+ tv_cmd-tvc_exp_data_len = exp_data_len;
+ tv_cmd-tvc_data_direction = data_direction;
+ tv_cmd-tvc_nexus = tv_nexus;
- se_cmd = tv_cmd-tvc_se_cmd;
- /*
- * Locate the SAM Task Attr from virtio_scsi_cmd_req
- */
- sam_task_attr = v_req-task_attr;
- /*
- * Initialize struct se_cmd descriptor from TCM infrastructure
- */
- transport_init_se_cmd(se_cmd, se_tpg-se_tpg_tfo, se_sess, exp_data_len,
- data_direction, sam_task_attr,
- tv_cmd-tvc_sense_buf[0]);
-
-#if 0/* FIXME: vhost_scsi_allocate_cmd() BIDI operation */
- if (bidi)
- se_cmd-se_cmd_flags |= SCF_BIDI;
-#endif
return tv_cmd;
}
@@ -549,37 +534,10 @@ static void tcm_vhost_submission_work(struct
work_struct *work)
{
struct tcm_vhost_cmd *tv_cmd =
container_of(work, struct tcm_vhost_cmd, work);
+ struct tcm_vhost_nexus *tv_nexus;
struct se_cmd *se_cmd = tv_cmd-tvc_se_cmd;
struct scatterlist *sg_ptr, *sg_bidi_ptr = NULL;
int rc, sg_no_bidi = 0;
- /*
- * Locate the struct se_lun pointer based on v_req-lun, and
- * attach it to struct se_cmd
- */
- rc = transport_lookup_cmd_lun(tv_cmd-tvc_se_cmd, tv_cmd-tvc_lun);
- if (rc 0) {
- pr_err(Failed to look up lun: %d\n, tv_cmd-tvc_lun);
- transport_send_check_condition_and_sense(tv_cmd-tvc_se_cmd,
- tv_cmd-tvc_se_cmd.scsi_sense_reason, 0);
- transport_generic_free_cmd(se_cmd, 0);
- return;
- }
-
IIUC tv_cmd can come from userspace so calling pr_error here
was a bug, it's good that it's fixed now.
And looking at target_submit_cmd_map_mem, it looks that
at least lun lookup error no longer triggers pr_error, right?
If yes it's good.
- rc = target_setup_cmd_from_cdb(se_cmd, tv_cmd-tvc_cdb);
- if (rc == -ENOMEM) {
- transport_send_check_condition_and_sense(se_cmd,
- TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE, 0);
- transport_generic_free_cmd(se_cmd, 0);
- return;
- } else if (rc 0) {
- if (se_cmd-se_cmd_flags SCF_SCSI_RESERVATION_CONFLICT)
- tcm_vhost_queue_status(se_cmd);
- else
- transport_send_check_condition_and_sense(se_cmd,
- se_cmd-scsi_sense_reason, 0);
- transport_generic_free_cmd(se_cmd, 0);
- return;
- }
if (tv_cmd-tvc_sgl_count) {
sg_ptr = tv_cmd-tvc_sgl;
@@ -597,17 +555,19 @@ static void tcm_vhost_submission_work(struct
work_struct *work)
} else {
sg_ptr = NULL;
}
-
- rc = transport_generic_map_mem_to_cmd(se_cmd, sg_ptr,
- tv_cmd-tvc_sgl_count, sg_bidi_ptr,
- sg_no_bidi);
+ tv_nexus = tv_cmd-tvc_nexus;
+
+ rc = target_submit_cmd_map_mem(se_cmd, tv_nexus-tvn_se_sess,
+
Re: [PATCH 0/4] target: Add target_submit_cmd_map_mem and convert tcm_loop+tcm_vhost
On Tue, Oct 02, 2012 at 07:15:43AM +, Nicholas A. Bellinger wrote: From: Nicholas Bellinger n...@linux-iscsi.org Hi hch Co, This series adds a new target_submit_cmd_map_mem() caller to accept pre-allocated SGL memory within the core generic I/O submission path. Patch #1 contains the core I/O changes, patch #2 + #4 includes the conversion of tcm_loop+tcm_vhost to use this new caller - drop their internal open-coded equivalents using transport_generic_map_mem_to_cmd(). Patch #3 carries forward a work-around for tcm_loop w/ scsi-generic with user-space code that does not zero out it's READ payload buffer + ends up passing a payload filled with random data into target core's control CDB emulation. Since we're not using bounce buffers any more for control CDB emulation in modern v3.x code, AFAICT tcm_loop still requires this extra bit to function properly with some legacy user-space code. Regardless, the main I/O path changes end up being very mechnical in nature for existing core and fabric code, and has been running as expected with fio small-block workloads last hours. Please review. Thanks Christoph! --nab For the vhost part: ACK series. Nicholas Bellinger (4): target: Add target_submit_cmd_map_mem for SGL fabric memory passthrough tcm_loop: Convert I/O path to use target_submit_cmd_map_mem target: Add TARGET_SCF_MAP_CLEAR_MEM work-around for tcm_loop tcm_vhost: Convert I/O path to use target_submit_cmd_map_mem drivers/target/loopback/tcm_loop.c | 62 ++- drivers/target/target_core_transport.c | 129 ++-- drivers/vhost/tcm_vhost.c | 68 - drivers/vhost/tcm_vhost.h |8 ++ include/target/target_core_base.h |2 + include/target/target_core_fabric.h|3 + 6 files changed, 141 insertions(+), 131 deletions(-) -- 1.7.2.5 -- 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 1/4] target: Add target_submit_cmd_map_mem for SGL fabric memory passthrough
On Tue, Oct 02, 2012 at 07:15:44AM +, Nicholas A. Bellinger wrote: From: Nicholas Bellinger n...@linux-iscsi.org This patch adds a new TARGET_SCF_MAP_MEM flag within __target_submit_cmd() + a target_submit_cmd_map_mem() wrapper to pass pre-allocated SGL memory using existing transport_generic_map_mem_to_cmd() logic into the generic target submit I/O codepath. Instead of requiring a flag we could simply check for the sglist_count to be non-zero instead of requiring multiple pieces of information to be in sync. I'd also be tempted to remove the wrappers and just expose what is __target_submit_cmd now to the drivers, but that can be done independently as a cleanup. -- 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/4] tcm_loop: Convert I/O path to use target_submit_cmd_map_mem
On Tue, Oct 02, 2012 at 07:15:45AM +, Nicholas A. Bellinger wrote: From: Nicholas Bellinger n...@linux-iscsi.org This patch converts tcm_loop to use target_submit_cmd_map_mem() for I/O submission and mapping of pre-allocated SGL memory from incoming scsi_cmnd - se_cmd descriptors. This includes removing the original open-coded fabric uses of target core callers to support transport_generic_map_mem_to_cmd() between target_setup_cmd_from_cdb() and transport_handle_cdb_direct() logic. 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][SCSI] mpt3sas: Paer 1 of MPI API headers
On Sat, Sep 29, 2012 at 10:52:50PM +0200, Bj??rn Mork wrote: sreekanth.re...@lsi.com writes: This patch contains MPI API headers Why can't this and the other headers be shared between the mpt2sas and mpt3sas drivers? Looks like you are duplicating a lot of code already present in drivers/scsi/mpt2sas. Why? That's a recipe for maintenance hell... I would go further and ask why this needs a new driver; why can't mpt2sas be enhanced to drive the 12Gbit cards? A deliberate choice was made to keep the mpt3sas separate from mpt2sas due to our 6 GIG product being truly mature with a noticeable ramping down of enhancements/releases. mpt3sas is at it's initial baseline with a healthy roadmap ahead. Architecturally there will be changes to the headers that will be only mpt3sas specific. Another reason to keep the source separate. The mpt3sas release schedules is on a faster cadence than mpt2sas so keeping the source aligned is problematic. From a maintenance and test perspective having the source base separate actually reduces the so called maintenance hell. -- Regards, Nagalakshmi -- 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 3/4] target: Add TARGET_SCF_MAP_CLEAR_MEM work-around for tcm_loop
On Tue, Oct 02, 2012 at 07:15:46AM +, Nicholas A. Bellinger wrote: From: Nicholas Bellinger n...@linux-iscsi.org This patch carries forward a work-around from tcm_loop to target core code to explicitly clear control CDB READ paylods in order to avoid bugs in scsi-generic user-space code for INQUIRY that do not explicitly zero CDB payload memory. I never understood why we'd limit this to loop. As far as I can see we can get the same issue using sg-passthrough ontop of vhost, and for the other drivers we'd simply rely on the drivers zeroing the pages. I'd be much happier if we'd just unconditionally do this for the control plane commands, as the performance over head for those should be negligible. A good place would be to do it as part of transport_kmap_data_sg, after adding a data direction flag to it so that we'll only do it for the from device direction. -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] target: Add target_submit_cmd_map_mem and convert tcm_loop+tcm_vhost
This should also grow a patch 5 to unexport transport_generic_map_mem_to_cmd and mark it static. -- 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/3] virtio-scsi updates for 3.7
James, here is a resend of the three pending patches for virtio-scsi, with the bugfix first. Thanks, Paolo Paolo Bonzini (2): virtio-scsi: fix LUNs greater than 255 virtio-scsi: support online resizing of disks Richard W.M. Jones (1): virtio-scsi: initialize scatterlist structure drivers/scsi/virtio_scsi.c | 39 --- include/linux/virtio_scsi.h |2 ++ 2 files changed, 38 insertions(+), 3 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/3] virtio-scsi: initialize scatterlist structure
From: Richard W.M. Jones rjo...@redhat.com The sg struct is used without being initialized, which breaks when CONFIG_DEBUG_SG is enabled. Cc: sta...@vger.kernel.org Signed-off-by: Richard W.M. Jones rjo...@redhat.com Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- drivers/scsi/virtio_scsi.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c index 3e79a2f..7554d78 100644 --- a/drivers/scsi/virtio_scsi.c +++ b/drivers/scsi/virtio_scsi.c @@ -219,7 +219,7 @@ static int virtscsi_kick_event(struct virtio_scsi *vscsi, struct scatterlist sg; unsigned long flags; - sg_set_buf(sg, event_node-event, sizeof(struct virtio_scsi_event)); + sg_init_one(sg, event_node-event, sizeof(struct virtio_scsi_event)); spin_lock_irqsave(vscsi-event_vq.vq_lock, flags); -- 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 3/3] virtio-scsi: support online resizing of disks
Support the LUN parameter change event. Currently, the host fires this event
when the capacity of a disk is changed from the virtual machine monitor.
The resize then appears in the kernel log like this:
sd 0:0:0:0: [sda] 46137344 512-byte logical blocks: (23.6 GB/22.0 GIb)
sda: detected capacity change from 22548578304 to 23622320128
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
drivers/scsi/virtio_scsi.c | 31 ++-
include/linux/virtio_scsi.h |2 ++
2 files changed, 32 insertions(+), 1 deletions(-)
diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index a7cf726..595af1a 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -279,6 +279,31 @@ static void virtscsi_handle_transport_reset(struct
virtio_scsi *vscsi,
}
}
+static void virtscsi_handle_param_change(struct virtio_scsi *vscsi,
+struct virtio_scsi_event *event)
+{
+ struct scsi_device *sdev;
+ struct Scsi_Host *shost = virtio_scsi_host(vscsi-vdev);
+ unsigned int target = event-lun[1];
+ unsigned int lun = (event-lun[2] 8) | event-lun[3];
+ u8 asc = event-reason 255;
+ u8 ascq = event-reason 8;
+
+ sdev = scsi_device_lookup(shost, 0, target, lun);
+ if (!sdev) {
+ pr_err(SCSI device %d 0 %d %d not found\n,
+ shost-host_no, target, lun);
+ return;
+ }
+
+ /* Handle Parameters changed, Mode parameters changed, and
+ Capacity data has changed. */
+ if (asc == 0x2a (ascq == 0x00 || ascq == 0x01 || ascq == 0x09))
+ scsi_rescan_device(sdev-sdev_gendev);
+
+ scsi_device_put(sdev);
+}
+
static void virtscsi_handle_event(struct work_struct *work)
{
struct virtio_scsi_event_node *event_node =
@@ -297,6 +322,9 @@ static void virtscsi_handle_event(struct work_struct *work)
case VIRTIO_SCSI_T_TRANSPORT_RESET:
virtscsi_handle_transport_reset(vscsi, event);
break;
+ case VIRTIO_SCSI_T_PARAM_CHANGE:
+ virtscsi_handle_param_change(vscsi, event);
+ break;
default:
pr_err(Unsupport virtio scsi event %x\n, event-event);
}
@@ -737,7 +765,8 @@ static struct virtio_device_id id_table[] = {
};
static unsigned int features[] = {
- VIRTIO_SCSI_F_HOTPLUG
+ VIRTIO_SCSI_F_HOTPLUG,
+ VIRTIO_SCSI_F_CHANGE,
};
static struct virtio_driver virtio_scsi_driver = {
diff --git a/include/linux/virtio_scsi.h b/include/linux/virtio_scsi.h
index dc8d305..d6b4440 100644
--- a/include/linux/virtio_scsi.h
+++ b/include/linux/virtio_scsi.h
@@ -72,6 +72,7 @@ struct virtio_scsi_config {
/* Feature Bits */
#define VIRTIO_SCSI_F_INOUT0
#define VIRTIO_SCSI_F_HOTPLUG 1
+#define VIRTIO_SCSI_F_CHANGE 2
/* Response codes */
#define VIRTIO_SCSI_S_OK 0
@@ -108,6 +109,7 @@ struct virtio_scsi_config {
#define VIRTIO_SCSI_T_NO_EVENT 0
#define VIRTIO_SCSI_T_TRANSPORT_RESET 1
#define VIRTIO_SCSI_T_ASYNC_NOTIFY 2
+#define VIRTIO_SCSI_T_PARAM_CHANGE 3
/* Reasons of transport reset event */
#define VIRTIO_SCSI_EVT_RESET_HARD 0
--
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 2/3] virtio-scsi: fix LUNs greater than 255
virtio-scsi needs to report LUNs greater than 256 using the flat format. Because the Linux SCSI layer just maps the SCSI LUN to an u32, without any parsing, these end up in the range from 16640 to 32767. Fix max_lun to account for the possibility that logical unit numbers are encoded with the flat format. Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- drivers/scsi/virtio_scsi.c |6 +- 1 files changed, 5 insertions(+), 1 deletions(-) diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c index 7554d78..a7cf726 100644 --- a/drivers/scsi/virtio_scsi.c +++ b/drivers/scsi/virtio_scsi.c @@ -677,7 +677,11 @@ static int __devinit virtscsi_probe(struct virtio_device *vdev) cmd_per_lun = virtscsi_config_get(vdev, cmd_per_lun) ?: 1; shost-cmd_per_lun = min_t(u32, cmd_per_lun, shost-can_queue); shost-max_sectors = virtscsi_config_get(vdev, max_sectors) ?: 0x; - shost-max_lun = virtscsi_config_get(vdev, max_lun) + 1; + + /* LUNs 256 are reported with format 1, so they go in the range +* 16640-32767. +*/ + shost-max_lun = virtscsi_config_get(vdev, max_lun) + 1 + 0x4000; shost-max_id = num_targets; shost-max_channel = 0; shost-max_cmd_len = VIRTIO_SCSI_CDB_SIZE; -- 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 1/1] Drivers: scsi: storvsc: Account for in-transit packets in the RESET path
Properly account for I/O in transit before returning from the RESET call. In the absense of this patch, we could have a situation where the host may respond to a command that was issued prior to the issuance of the RESET command at some arbitrary time after responding to the RESET command. Currently, the host does not do anything with the RESET command. Signed-off-by: K. Y. Srinivasan k...@microsoft.com Cc: sta...@vger.kernel.org --- drivers/scsi/storvsc_drv.c |5 + 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c index 528d52b..0144078 100644 --- a/drivers/scsi/storvsc_drv.c +++ b/drivers/scsi/storvsc_drv.c @@ -1221,7 +1221,12 @@ static int storvsc_host_reset_handler(struct scsi_cmnd *scmnd) /* * At this point, all outstanding requests in the adapter * should have been flushed out and return to us +* There is a potential race here where the host may be in +* the process of responding when we return from here. +* Just wait for all in-transit packets to be accounted for +* before we return from here. */ + storvsc_wait_to_drain(stor_device); return SUCCESS; } -- 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 1/6] target/file: Re-enable optional fd_buffered_io=1 operation
Christoph Hellwig, on 10/01/2012 04:46 AM wrote: On Sun, Sep 30, 2012 at 05:58:11AM +, Nicholas A. Bellinger wrote: From: Nicholas Bellingern...@linux-iscsi.org This patch re-adds the ability to optionally run in buffered FILEIO mode (eg: w/o O_DSYNC) for device backends in order to once again use the Linux buffered cache as a write-back storage mechanism. This difference with this patch is that fd_create_virtdevice() now forces the explicit setting of emulate_write_cache=1 when buffered FILEIO operation has been enabled. What this lacks is a clear reason why you would enable this inherently unsafe mode. While there is some clear precedence to allow people doing stupid thing I'd least like a rationale for it, and it being documented as unsafe. Nowadays nearly all serious applications are transactional, and know how to flush storage cache between transactions. That means that write back caching is absolutely safe for them. No data can't be lost in any circumstances. Welcome to the 21 century, Christoph! Vlad -- 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 1/6] target/file: Re-enable optional fd_buffered_io=1 operation
On Mon, 2012-10-01 at 04:46 -0400, Christoph Hellwig wrote: On Sun, Sep 30, 2012 at 05:58:11AM +, Nicholas A. Bellinger wrote: From: Nicholas Bellinger n...@linux-iscsi.org This patch re-adds the ability to optionally run in buffered FILEIO mode (eg: w/o O_DSYNC) for device backends in order to once again use the Linux buffered cache as a write-back storage mechanism. This difference with this patch is that fd_create_virtdevice() now forces the explicit setting of emulate_write_cache=1 when buffered FILEIO operation has been enabled. What this lacks is a clear reason why you would enable this inherently unsafe mode. While there is some clear precedence to allow people doing stupid thing I'd least like a rationale for it, and it being documented as unsafe. +/* Indention error. Fixed +* Optionally allow fd_buffered_io=1 to be enabled for people +* who know what they are doing w/o O_DSYNC. +*/ This comment doesn't explain at all what's going on here. It should be something like /* * Unsafe mode allows disabling data integrity by not forcing * data out to disk in writethrough cache mode. Only to be used * for benchmark cheating or similar purposes. */ How about the following instead..? /* * Optionally allow fd_buffered_io=1 to be enabled for people * who want use the fs buffer cache as an WriteCache mechanism. * * This means that in event of a hard failure, there is a risk * of silent data-loss if the SCSI client has *not* performed a * forced unit access (FUA) write, or issued SYNCHRONIZE_CACHE * to write-out the entire device cache. */ #define FBDF_HAS_PATH 0x01 #define FBDF_HAS_SIZE 0x02 +#define FDBD_USE_BUFFERED_IO 0x04 This should be named BDBD_UNSAFE As we are keeping the same fd_buffered_io key=value usage that rtslib user-space already knows about, how using FDBD_HAS_BUFFERED_IO_WCE instead for consistency..? -- 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] target/iscsi: precedence bug in iscsit_set_dataout_sequence_values()
On Tue, 2012-10-02 at 11:22 +0300, Dan Carpenter wrote:
Clang warns about this bug:
drivers/target/iscsi/iscsi_target_erl0.c:52:45: warning: operator '?:'
has lower precedence than '+'; '+' will be evaluated first
[-Wparentheses]
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
---
Please review this very carefully because I haven't tested it. It could
be that the check should be:
(data_done + data_length FirstBurstLength) ? FirstBurstLength :
data_length);
Instead of what I have which is:
data_done + (data_length FirstBurstLength ? FirstBurstLength : data_length);
diff --git a/drivers/target/iscsi/iscsi_target_erl0.c
b/drivers/target/iscsi/iscsi_target_erl0.c
index 1a02016..2067efd 100644
--- a/drivers/target/iscsi/iscsi_target_erl0.c
+++ b/drivers/target/iscsi/iscsi_target_erl0.c
@@ -48,9 +48,9 @@ void iscsit_set_dataout_sequence_values(
if (cmd-unsolicited_data) {
cmd-seq_start_offset = cmd-write_data_done;
cmd-seq_end_offset = (cmd-write_data_done +
- (cmd-se_cmd.data_length
- conn-sess-sess_ops-FirstBurstLength) ?
- conn-sess-sess_ops-FirstBurstLength :
cmd-se_cmd.data_length);
+ ((cmd-se_cmd.data_length
+ conn-sess-sess_ops-FirstBurstLength) ?
+ conn-sess-sess_ops-FirstBurstLength :
cmd-se_cmd.data_length));
return;
}
--
This is indeed the original intention and your patch is correct, so
applied to for-next.
Thank you!
--nab
--
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 1/4] target: Add target_submit_cmd_map_mem for SGL fabric memory passthrough
On Tue, 2012-10-02 at 11:17 -0400, Christoph Hellwig wrote: On Tue, Oct 02, 2012 at 07:15:44AM +, Nicholas A. Bellinger wrote: From: Nicholas Bellinger n...@linux-iscsi.org This patch adds a new TARGET_SCF_MAP_MEM flag within __target_submit_cmd() + a target_submit_cmd_map_mem() wrapper to pass pre-allocated SGL memory using existing transport_generic_map_mem_to_cmd() logic into the generic target submit I/O codepath. Instead of requiring a flag we could simply check for the sglist_count to be non-zero instead of requiring multiple pieces of information to be in sync. Mmmm, fair point here. In that case I'd rather rename __target_submit_cmd() to target_submit_cmd_mapsgl(), and update the target_submit_cmd() wrapper accordingly. I'd also be tempted to remove the wrappers and just expose what is __target_submit_cmd now to the drivers, but that can be done independently as a cleanup. nod, dropping the extra target_submit_cmd_map_mem bit + flag now. Thanks hch! --nab -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH-v2 0/4] target: Add target_submit_cmd_map_sgls + convert tcm_loop+tcm_vhost
From: Nicholas Bellinger n...@linux-iscsi.org Hi folks, Here is a -v2 of the series to allow target_submit_cmd() logic to accept SGL passthrough memory using existing transport_generic_map_mem_to_cmd() code based upon hch's feedback this morning. The changelog from v1 - v2 includes: - Rename target_submit_cmd_map_mem() - target_submit_cmd_map_sgls() and export kernel symbol to fabrics. - Drop TARGET_SCF_MAP_MEM flag in favor of non zero sgl_count check - Drop TARGET_SCF_MAP_CLEAR_MEM flag, and zero control CDB READ payload for all fabrics doing SGL passthrough - Update tcm_loop+tcm_vhost to use target_submit_cmd_map_sgls All of the v2 changes are straight-forward enough, but please let us know if there are any more concerns here. Thank you! --nab Nicholas Bellinger (4): target: Add target_submit_cmd_map_sgls for SGL fabric memory passthrough tcm_loop: Convert I/O path to use target_submit_cmd_map_sgls target: Add control CDB READ payload zero work-around tcm_vhost: Convert I/O path to use target_submit_cmd_map_sgls drivers/target/loopback/tcm_loop.c | 62 +++ drivers/target/target_core_transport.c | 84 ++-- drivers/vhost/tcm_vhost.c | 68 + drivers/vhost/tcm_vhost.h |8 +++ include/target/target_core_fabric.h|3 + 5 files changed, 112 insertions(+), 113 deletions(-) -- 1.7.2.5 -- To unsubscribe from this list: send the line unsubscribe linux-scsi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH-v2 1/4] target: Add target_submit_cmd_map_sgls for SGL fabric memory passthrough
From: Nicholas Bellinger n...@linux-iscsi.org
This patch adds a new target_submit_cmd_map_sgls() to pass pre-allocated
SGL memory using transport_generic_map_mem_to_cmd() logic into the generic
target submit I/O codepath.
It also adds a target_submit_cmd() wrapper around target_submit_cmd_map_sgls()
for existing fabric code that already assumes internal target-core SGL memory
allocation.
(v2: Rename to target_submit_cmd_map_sgls + drop TARGET_SCF_MAP_MEM flag
in favor of non zero sgl_count check)
Reported-by: Christoph Hellwig h...@lst.de
Cc: Christoph Hellwig h...@lst.de
Signed-off-by: Nicholas Bellinger n...@linux-iscsi.org
---
drivers/target/target_core_transport.c | 65 +---
include/target/target_core_base.h |1 +
include/target/target_core_fabric.h|3 +
3 files changed, 63 insertions(+), 6 deletions(-)
diff --git a/drivers/target/target_core_transport.c
b/drivers/target/target_core_transport.c
index 221f67f..d96d9aa 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -1455,8 +1455,9 @@ int transport_handle_cdb_direct(
}
EXPORT_SYMBOL(transport_handle_cdb_direct);
-/**
- * target_submit_cmd - lookup unpacked lun and submit uninitialized se_cmd
+/*
+ * target_submit_cmd_map_sgls - lookup unpacked lun and submit uninitialized
+ * se_cmd + use pre-allocated SGL memory.
*
* @se_cmd: command descriptor to submit
* @se_sess: associated se_sess for endpoint
@@ -1467,6 +1468,10 @@ EXPORT_SYMBOL(transport_handle_cdb_direct);
* @task_addr: SAM task attribute
* @data_dir: DMA data direction
* @flags: flags for command submission from target_sc_flags_tables
+ * @sgl: struct scatterlist memory for unidirectional mapping
+ * @sgl_count: scatterlist count for unidirectional mapping
+ * @sgl_bidi: struct scatterlist memory for bidirectional READ mapping
+ * @sgl_bidi_count: scatterlist count for bidirectional READ mapping
*
* Returns non zero to signal active I/O shutdown failure. All other
* setup exceptions will be returned as a SCSI CHECK_CONDITION response,
@@ -1474,10 +1479,12 @@ EXPORT_SYMBOL(transport_handle_cdb_direct);
*
* This may only be called from process context, and also currently
* assumes internal allocation of fabric payload buffer by target-core.
- **/
-int target_submit_cmd(struct se_cmd *se_cmd, struct se_session *se_sess,
+ */
+int target_submit_cmd_map_sgls(struct se_cmd *se_cmd, struct se_session
*se_sess,
unsigned char *cdb, unsigned char *sense, u32 unpacked_lun,
- u32 data_length, int task_attr, int data_dir, int flags)
+ u32 data_length, int task_attr, int data_dir, int flags,
+ struct scatterlist *sgl, u32 sgl_count,
+ struct scatterlist *sgl_bidi, u32 sgl_bidi_count)
{
struct se_portal_group *se_tpg;
int rc;
@@ -1524,7 +1531,21 @@ int target_submit_cmd(struct se_cmd *se_cmd, struct
se_session *se_sess,
transport_generic_request_failure(se_cmd);
return 0;
}
-
+ /*
+* When a non zero sgl_count has been passed perform SGL passthrough
+* mapping for pre-allocated fabric memory instead of having target
+* core perform an internal SGL allocation..
+*/
+ if (sgl_count != 0) {
+ BUG_ON(!sgl);
+
+ rc = transport_generic_map_mem_to_cmd(se_cmd, sgl, sgl_count,
+ sgl_bidi, sgl_bidi_count);
+ if (rc != 0) {
+ transport_generic_request_failure(se_cmd);
+ return 0;
+ }
+ }
/*
* Check if we need to delay processing because of ALUA
* Active/NonOptimized primary access state..
@@ -1534,6 +1555,38 @@ int target_submit_cmd(struct se_cmd *se_cmd, struct
se_session *se_sess,
transport_handle_cdb_direct(se_cmd);
return 0;
}
+EXPORT_SYMBOL(target_submit_cmd_map_sgls);
+
+/*
+ * target_submit_cmd - lookup unpacked lun and submit uninitialized se_cmd
+ *
+ * @se_cmd: command descriptor to submit
+ * @se_sess: associated se_sess for endpoint
+ * @cdb: pointer to SCSI CDB
+ * @sense: pointer to SCSI sense buffer
+ * @unpacked_lun: unpacked LUN to reference for struct se_lun
+ * @data_length: fabric expected data transfer length
+ * @task_addr: SAM task attribute
+ * @data_dir: DMA data direction
+ * @flags: flags for command submission from target_sc_flags_tables
+ *
+ * Returns non zero to signal active I/O shutdown failure. All other
+ * setup exceptions will be returned as a SCSI CHECK_CONDITION response,
+ * but still return zero here.
+ *
+ * This may only be called from process context, and also currently
+ * assumes internal allocation of fabric payload buffer by target-core.
+ *
+ * It also assumes interal target core SGL memory allocation.
+ */
+int target_submit_cmd(struct se_cmd *se_cmd, struct
[PATCH-v2 2/4] tcm_loop: Convert I/O path to use target_submit_cmd_map_sgls
From: Nicholas Bellinger n...@linux-iscsi.org
This patch converts tcm_loop to use target_submit_cmd_map_sgls() for
I/O submission and mapping of pre-allocated SGL memory from incoming
scsi_cmnd - se_cmd descriptors.
This includes removing the original open-coded fabric uses of target
core callers to support transport_generic_map_mem_to_cmd() between
target_setup_cmd_from_cdb() and transport_handle_cdb_direct() logic.
(v2: Use renamed target_submit_cmd_map_sgls)
Reported-by: Christoph Hellwig h...@lst.de
Reviewed-by: Christoph Hellwig h...@lst.de
Signed-off-by: Nicholas Bellinger n...@linux-iscsi.org
---
drivers/target/loopback/tcm_loop.c | 62 +---
include/target/target_core_base.h |1 -
2 files changed, 8 insertions(+), 55 deletions(-)
diff --git a/drivers/target/loopback/tcm_loop.c
b/drivers/target/loopback/tcm_loop.c
index 7a0da1a..2d444b1 100644
--- a/drivers/target/loopback/tcm_loop.c
+++ b/drivers/target/loopback/tcm_loop.c
@@ -166,7 +166,7 @@ static void tcm_loop_submission_work(struct work_struct
*work)
struct tcm_loop_tpg *tl_tpg;
struct scatterlist *sgl_bidi = NULL;
u32 sgl_bidi_count = 0;
- int ret;
+ int rc;
tl_hba = *(struct tcm_loop_hba **)shost_priv(sc-device-host);
tl_tpg = tl_hba-tl_hba_tpgs[sc-device-id];
@@ -187,12 +187,6 @@ static void tcm_loop_submission_work(struct work_struct
*work)
set_host_byte(sc, DID_ERROR);
goto out_done;
}
-
- transport_init_se_cmd(se_cmd, tl_tpg-tl_se_tpg.se_tpg_tfo,
- tl_nexus-se_sess,
- scsi_bufflen(sc), sc-sc_data_direction,
- tcm_loop_sam_attr(sc), tl_cmd-tl_sense_buf[0]);
-
if (scsi_bidi_cmnd(sc)) {
struct scsi_data_buffer *sdb = scsi_in(sc);
@@ -201,56 +195,16 @@ static void tcm_loop_submission_work(struct work_struct
*work)
se_cmd-se_cmd_flags |= SCF_BIDI;
}
-
- if (transport_lookup_cmd_lun(se_cmd, tl_cmd-sc-device-lun) 0) {
- kmem_cache_free(tcm_loop_cmd_cache, tl_cmd);
+ 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),
+ sc-sc_data_direction, 0,
+ scsi_sglist(sc), scsi_sg_count(sc),
+ sgl_bidi, sgl_bidi_count);
+ if (rc 0) {
set_host_byte(sc, DID_NO_CONNECT);
goto out_done;
}
-
- /*
-* Because some userspace code via scsi-generic do not memset their
-* associated read buffers, go ahead and do that here for type
-* non-data CDBs. Also note that this is currently guaranteed to be a
-* single SGL for this case by target core in
-* target_setup_cmd_from_cdb() - transport_generic_cmd_sequencer().
-*/
- if (!(se_cmd-se_cmd_flags SCF_SCSI_DATA_CDB)
- se_cmd-data_direction == DMA_FROM_DEVICE) {
- struct scatterlist *sg = scsi_sglist(sc);
- unsigned char *buf = kmap(sg_page(sg)) + sg-offset;
-
- if (buf != NULL) {
- memset(buf, 0, sg-length);
- kunmap(sg_page(sg));
- }
- }
-
- ret = target_setup_cmd_from_cdb(se_cmd, sc-cmnd);
- if (ret == -ENOMEM) {
- transport_send_check_condition_and_sense(se_cmd,
- TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE, 0);
- transport_generic_free_cmd(se_cmd, 0);
- return;
- } else if (ret 0) {
- if (se_cmd-se_cmd_flags SCF_SCSI_RESERVATION_CONFLICT)
- tcm_loop_queue_status(se_cmd);
- else
- transport_send_check_condition_and_sense(se_cmd,
- se_cmd-scsi_sense_reason, 0);
- transport_generic_free_cmd(se_cmd, 0);
- return;
- }
-
- ret = transport_generic_map_mem_to_cmd(se_cmd, scsi_sglist(sc),
- scsi_sg_count(sc), sgl_bidi, sgl_bidi_count);
- if (ret) {
- transport_send_check_condition_and_sense(se_cmd,
- se_cmd-scsi_sense_reason, 0);
- transport_generic_free_cmd(se_cmd, 0);
- return;
- }
- transport_handle_cdb_direct(se_cmd);
return;
out_done:
diff --git a/include/target/target_core_base.h
b/include/target/target_core_base.h
index 6309298..5be8937 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -220,7 +220,6 @@ enum target_sc_flags_table {
TARGET_SCF_BIDI_OP = 0x01,
TARGET_SCF_ACK_KREF = 0x02,
TARGET_SCF_UNKNOWN_SIZE = 0x04,
-
[PATCH-v2 4/4] tcm_vhost: Convert I/O path to use target_submit_cmd_map_sgls
From: Nicholas Bellinger n...@linux-iscsi.org
This patch converts tcm_vhost to use target_submit_cmd_map_sgls() for
I/O submission and mapping of pre-allocated SGL memory from incoming
virtio-scsi SGL memory - se_cmd descriptors.
This includes removing the original open-coded fabric uses of target
core callers to support transport_generic_map_mem_to_cmd() between
target_setup_cmd_from_cdb() and transport_handle_cdb_direct() logic.
It also includes adding a handful of new tcm_vhost_cmnd member +
assignments in vhost_scsi_allocate_cmd() used from cmwq process
context I/O submission within tcm_vhost_submission_work()
(v2: Use renamed target_submit_cmd_map_sgls)
Reported-by: Christoph Hellwig h...@lst.de
Cc: Christoph Hellwig h...@lst.de
Acked-by: Michael S. Tsirkin m...@redhat.com
Cc: Paolo Bonzini pbonz...@redhat.com
Cc: Stefan Hajnoczi stefa...@gmail.com
Signed-off-by: Nicholas Bellinger n...@linux-iscsi.org
---
drivers/vhost/tcm_vhost.c | 68 +---
drivers/vhost/tcm_vhost.h |8 +
2 files changed, 22 insertions(+), 54 deletions(-)
diff --git a/drivers/vhost/tcm_vhost.c b/drivers/vhost/tcm_vhost.c
index 89dc99b..aa31692 100644
--- a/drivers/vhost/tcm_vhost.c
+++ b/drivers/vhost/tcm_vhost.c
@@ -415,10 +415,7 @@ static struct tcm_vhost_cmd *vhost_scsi_allocate_cmd(
{
struct tcm_vhost_cmd *tv_cmd;
struct tcm_vhost_nexus *tv_nexus;
- struct se_portal_group *se_tpg = tv_tpg-se_tpg;
struct se_session *se_sess;
- struct se_cmd *se_cmd;
- int sam_task_attr;
tv_nexus = tv_tpg-tpg_nexus;
if (!tv_nexus) {
@@ -434,23 +431,11 @@ static struct tcm_vhost_cmd *vhost_scsi_allocate_cmd(
}
INIT_LIST_HEAD(tv_cmd-tvc_completion_list);
tv_cmd-tvc_tag = v_req-tag;
+ tv_cmd-tvc_task_attr = v_req-task_attr;
+ tv_cmd-tvc_exp_data_len = exp_data_len;
+ tv_cmd-tvc_data_direction = data_direction;
+ tv_cmd-tvc_nexus = tv_nexus;
- se_cmd = tv_cmd-tvc_se_cmd;
- /*
-* Locate the SAM Task Attr from virtio_scsi_cmd_req
-*/
- sam_task_attr = v_req-task_attr;
- /*
-* Initialize struct se_cmd descriptor from TCM infrastructure
-*/
- transport_init_se_cmd(se_cmd, se_tpg-se_tpg_tfo, se_sess, exp_data_len,
- data_direction, sam_task_attr,
- tv_cmd-tvc_sense_buf[0]);
-
-#if 0 /* FIXME: vhost_scsi_allocate_cmd() BIDI operation */
- if (bidi)
- se_cmd-se_cmd_flags |= SCF_BIDI;
-#endif
return tv_cmd;
}
@@ -549,37 +534,10 @@ static void tcm_vhost_submission_work(struct work_struct
*work)
{
struct tcm_vhost_cmd *tv_cmd =
container_of(work, struct tcm_vhost_cmd, work);
+ struct tcm_vhost_nexus *tv_nexus;
struct se_cmd *se_cmd = tv_cmd-tvc_se_cmd;
struct scatterlist *sg_ptr, *sg_bidi_ptr = NULL;
int rc, sg_no_bidi = 0;
- /*
-* Locate the struct se_lun pointer based on v_req-lun, and
-* attach it to struct se_cmd
-*/
- rc = transport_lookup_cmd_lun(tv_cmd-tvc_se_cmd, tv_cmd-tvc_lun);
- if (rc 0) {
- pr_err(Failed to look up lun: %d\n, tv_cmd-tvc_lun);
- transport_send_check_condition_and_sense(tv_cmd-tvc_se_cmd,
- tv_cmd-tvc_se_cmd.scsi_sense_reason, 0);
- transport_generic_free_cmd(se_cmd, 0);
- return;
- }
-
- rc = target_setup_cmd_from_cdb(se_cmd, tv_cmd-tvc_cdb);
- if (rc == -ENOMEM) {
- transport_send_check_condition_and_sense(se_cmd,
- TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE, 0);
- transport_generic_free_cmd(se_cmd, 0);
- return;
- } else if (rc 0) {
- if (se_cmd-se_cmd_flags SCF_SCSI_RESERVATION_CONFLICT)
- tcm_vhost_queue_status(se_cmd);
- else
- transport_send_check_condition_and_sense(se_cmd,
- se_cmd-scsi_sense_reason, 0);
- transport_generic_free_cmd(se_cmd, 0);
- return;
- }
if (tv_cmd-tvc_sgl_count) {
sg_ptr = tv_cmd-tvc_sgl;
@@ -597,17 +555,19 @@ static void tcm_vhost_submission_work(struct work_struct
*work)
} else {
sg_ptr = NULL;
}
-
- rc = transport_generic_map_mem_to_cmd(se_cmd, sg_ptr,
- tv_cmd-tvc_sgl_count, sg_bidi_ptr,
- sg_no_bidi);
+ tv_nexus = tv_cmd-tvc_nexus;
+
+ rc = target_submit_cmd_map_sgls(se_cmd, tv_nexus-tvn_se_sess,
+ tv_cmd-tvc_cdb, tv_cmd-tvc_sense_buf[0],
+ tv_cmd-tvc_lun, tv_cmd-tvc_exp_data_len,
+ tv_cmd-tvc_task_attr, tv_cmd-tvc_data_direction,
+ 0, sg_ptr,
Re: [PATCH-v2 1/4] target: Add target_submit_cmd_map_sgls for SGL fabric memory passthrough
On Tue, 2012-10-02 at 21:16 +, Nicholas A. Bellinger wrote:
From: Nicholas Bellinger n...@linux-iscsi.org
This patch adds a new target_submit_cmd_map_sgls() to pass pre-allocated
SGL memory using transport_generic_map_mem_to_cmd() logic into the generic
target submit I/O codepath.
It also adds a target_submit_cmd() wrapper around target_submit_cmd_map_sgls()
for existing fabric code that already assumes internal target-core SGL memory
allocation.
(v2: Rename to target_submit_cmd_map_sgls + drop TARGET_SCF_MAP_MEM flag
in favor of non zero sgl_count check)
Reported-by: Christoph Hellwig h...@lst.de
Cc: Christoph Hellwig h...@lst.de
Signed-off-by: Nicholas Bellinger n...@linux-iscsi.org
---
drivers/target/target_core_transport.c | 65
+---
include/target/target_core_base.h |1 +
include/target/target_core_fabric.h|3 +
3 files changed, 63 insertions(+), 6 deletions(-)
diff --git a/include/target/target_core_base.h
b/include/target/target_core_base.h
index 5be8937..6309298 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -220,6 +220,7 @@ enum target_sc_flags_table {
TARGET_SCF_BIDI_OP = 0x01,
TARGET_SCF_ACK_KREF = 0x02,
TARGET_SCF_UNKNOWN_SIZE = 0x04,
+ TARGET_SCF_MAP_MEM = 0x08,
};
Whoops, removing this left-over bit now before pushing into for-next.
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 scsi] Short the path length of scsi_cmd_to_driver()
Li == Li Zhong zh...@linux.vnet.ibm.com writes: Li As suggested by James: this patch tries to short the path length of Li scsi_cmd_to_driver(). As only REQ_TYPE_BLOCK_PC commands can be Li submitted without a driver, so we could avoid the related NULL Li checking, as long as we make sure we don't use it for Li REQ_TYPE_BLOCK_PC type commands. Plus, this fixes a bug where you Li get different behaviors from REQ_TYPE_BLOCK_PC commands when a Li driver is and isn't attached. Acked-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: [SCSI] mpt2sas T10 DIF fixes
Pasi == Pasi Kärkkäinen pa...@iki.fi writes: 2 of the patches are absolutely trivial and should just be applied. However, LSI objects to patch #3 which tweaks an incorrect NVDATA flag from within the driver. Instead LSI has promised to revert to the original behavior in their firmware/NVDATA updates. Eric, can you give us an update? There was another bug report on linux-scsi just a couple of weeks ago. Pasi Hopefully these T10 patches can go in during the 3.7 merge window! The 2 trivial patches are queued for 3.7, yes. LSI are still working the bad NVDATA default issue. -- 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 07/17] be2iscsi: Fix the kernel panic in blkiopoll disable mode
-Original Message- From: Michael Christie [mailto:micha...@cs.wisc.edu] Sent: Saturday, September 29, 2012 9:32 AM To: John, Sony Cc: linux-scsi@vger.kernel.org; Kallickal, Jayamohan Subject: Re: [PATCH 07/17] be2iscsi: Fix the kernel panic in blkiopoll disable mode On Sep 28, 2012, at 8:32 PM, John Soni Jose sony.joh...@emulex.com wrote: From: Jayamohan Kallickal jayamohan.kallic...@emulex.com Fix kernel panic issue while running IO in blk_iopoll disable mode. What was the bug exactly? The task completion notification comes on an EQ. In existing code getting EQ from the workQ was not proper, because of which the kernel panic used to happen. Creating UNBOUND WQ for each EQ in the driver. What is the benefit of doing this vs per hba like before? EQ's are created based on the NUM_CPU in the machine. Notification for task completion comes on an EQ. There could be multiple entries in the EQ that need to be processed and driver iterates the list of entries on that EQ does the required processing and then re-arms EQ for further interrupts. The issue with per hba WQ was it was not possible to get the EQ on which the interrupt has come. In the new infrastructure a single_thread WQ is assigned to each EQ that is created. The EQ structure ptr is passed as the device data structure while registering the interrupts. When interrupts comes, driver can just queue_work for that EQ from the interrupt handler. Why WQ_UNBOUND? It seems other drivers prefer it the other way. The intention of using the flag WQ_UNBOUND was to have a single_work_queue across all cores for each EQ created. Default implementation of alloc_workqueue is creating workqueue per core, in driver EQ creation depends on NUM_CPU and based on that too many workqueues will be created which can be counter productive. This scenario is hit only in blk_iopoll disable mode and the latest kernel it is always enabled. So used the WQ_UNBOUND flag to make is as single thread. Thanks Sony -- 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
Soft lockup in scsi_remove_target under 3.6 (regression from 3.5)
Upgraded to 3.6 today on my dev box and after seeing an FC attached SAN go down and come back up (due to an expected reboot) I started getting the following in my logs. It continues even after the array is back and functioning - I'm seeing: kernel:[109104.348034] BUG: soft lockup - CPU#6 stuck for 23s! [kworker/6:0:30692] repeated on logged in sessions and backtraces like the following (this is the first). I don't see the same problem under 3.5. [10819.389706] device-mapper: multipath: Failing path 8:240. [11233.683936] device-mapper: multipath: Failing path 8:240. [108394.592042] rport-10:0-4: blocked FC remote port time out: removing target and saving binding [108394.609594] sd 10:0:1:0: rejecting I/O to offline device [108394.620457] lpfc :0c:00.0: 2:(0):0203 Devloss timeout on WWPN 21:11:00:02:ac:01:86:06 NPort x030500 Data: x0 x7 x0 [108394.620591] sd 10:0:1:0: alua: Detached [108394.650159] sd 10:0:1:0: [sdbc] Synchronizing SCSI cache [108394.661071] sd 10:0:1:0: [sdbc] [108394.667877] Result: hostbyte=DID_NO_CONNECT driverbyte=DRIVER_OK [108394.680154] ses 10:0:1:254: alua: Detached [108420.348032] BUG: soft lockup - CPU#6 stuck for 23s! [kworker/6:0:30692] [108420.352003] Modules linked in: nfsv4 autofs4 ip6table_filter ip6_tables iptable_filter ip_tables ebtable_nat ebtables x_tables rpcsec_gss_krb5 ipv6 nfsd nfs_acl auth_rpcgss nfs lockd sunrpc dm_round_robin dm_multipath ipmi_devintf ipmi_si ipmi_msghandler sg evdev acpi_cpufreq freq_table serio_raw mperf processor button thermal_sys coretemp kvm_intel kvm lpc_ich ioatdma mfd_core tpm_tis i2c_i801 tpm microcode tpm_bios rng_core i2c_core i5k_amb dca ses enclosure ata_generic lpfc ata_piix scsi_transport_fc scsi_tgt [108420.352003] CPU 6 [108420.352003] Pid: 30692, comm: kworker/6:0 Not tainted 3.6.0 #5 Intel S5000PAL./S5000PAL0 [108420.352003] RIP: 0010:[8134e744] [8134e744] _raw_spin_unlock_irqrestore+0x5/0x6 [108420.352003] RSP: 0018:8802563a7d98 EFLAGS: 0286 [108420.352003] RAX: 88024e975000 RBX: 00bb RCX: [108420.352003] RDX: RSI: 0286 RDI: 88024e975050 [108420.352003] RBP: 88024e975000 R08: R09: 8166f890 [108420.352003] R10: 88024e975000 R11: a00d6bf0 R12: [108420.352003] R13: 8166f890 R14: 88024e975000 R15: a00d6bf0 [108420.352003] FS: () GS:88025fd8() knlGS: [108420.352003] CS: 0010 DS: ES: CR0: 8005003b [108420.352003] CR2: 7f5b5dec6070 CR3: 00024f0eb000 CR4: 07e0 [108420.352003] DR0: DR1: DR2: [108420.352003] DR3: DR6: 0ff0 DR7: 0400 [108420.352003] Process kworker/6:0 (pid: 30692, threadinfo 8802563a6000, task 880252f32a10) [108420.352003] Stack: [108420.352003] 8125a498 88025fd8d080 0286 a0015c28 [108420.352003] 88024d1207c0 88025fd8d080 88025fd98100 a0015c28 [108420.352003] 88024f22abd8 81045d07 00012240 [108420.352003] Call Trace: [108420.352003] [8125a498] ? scsi_remove_target+0x138/0x154 [108420.352003] [a0015c28] ? store_fc_host_system_hostname+0x66/0x66 [scsi_transport_fc] [108420.352003] [a0015c28] ? store_fc_host_system_hostname+0x66/0x66 [scsi_transport_fc] [108420.352003] [81045d07] ? process_one_work+0x1f8/0x30a [108420.352003] [81046034] ? worker_thread+0x21b/0x314 [108420.352003] [81045e19] ? process_one_work+0x30a/0x30a [108420.352003] [81045e19] ? process_one_work+0x30a/0x30a [108420.352003] [810496cf] ? kthread+0x81/0x89 [108420.352003] [81350174] ? kernel_thread_helper+0x4/0x10 [108420.352003] [8104964e] ? kthread_freezable_should_stop+0x4e/0x4e [108420.352003] [81350170] ? gs_change+0xb/0xb [108420.352003] Code: 66 39 d0 0f 94 c0 0f b6 c0 c3 fa b8 00 01 00 00 f0 66 0f c1 07 88 c2 66 c1 e8 08 38 c2 74 06 f3 90 8a 17 eb f6 c3 80 07 01 56 9d c3 83 ca ff f0 0f c1 17 b8 01 00 00 00 ff ca 79 05 f0 ff 07 30 [108448.348033] BUG: soft lockup - CPU#6 stuck for 22s! [kworker/6:0:30692] [108448.352003] Modules linked in: nfsv4 autofs4 ip6table_filter ip6_tables iptable_filter ip_tables ebtable_nat ebtables x_tables rpcsec_gss_krb5 ipv6 nfsd nfs_acl auth_rpcgss nfs lockd sunrpc dm_round_robin dm_multipath ipmi_devintf ipmi_si ipmi_msghandler sg evdev acpi_cpufreq freq_table serio_raw mperf processor button thermal_sys coretemp kvm_intel kvm lpc_ich ioatdma mfd_core tpm_tis i2c_i801 tpm microcode tpm_bios rng_core i2c_core i5k_amb dca ses enclosure ata_generic lpfc ata_piix scsi_transport_fc scsi_tgt [108448.352003] CPU 6 [108448.352003] Pid: 30692, comm: kworker/6:0 Not tainted 3.6.0 #5 Intel S5000PAL./S5000PAL0 [108448.352003] RIP:
Re: [PATCH 07/17] be2iscsi: Fix the kernel panic in blkiopoll disable mode
On 10/02/2012 06:51 PM, John, Sony wrote: Creating UNBOUND WQ for each EQ in the driver. What is the benefit of doing this vs per hba like before? EQ's are created based on the NUM_CPU in the machine. Notification for task completion comes on an EQ. There could be multiple entries in the EQ that need to be processed and driver iterates the list of entries on that EQ does the required processing and then re-arms EQ for further interrupts. The issue with per hba WQ was it was not possible to get the EQ on which the interrupt has come. In the new infrastructure a single_thread WQ is assigned to each EQ that is created. The EQ structure ptr is passed as the device data structure while registering the interrupts. When interrupts comes, driver can just queue_work for that EQ from the interrupt handler. Why not? Maybe I am not understanding you. You can allocate a workqueue_struct per hba, but embed a work struct in each be_queue_info. Then do: queue_work(phba-wq, eq-work_cqs); You would then keep + INIT_WORK(pbe_eq-work_cqs, beiscsi_process_all_cqs); in your patch and in the the work function for the eq-work_cqs you would get the eq like you wanted. You also then do not have to worry about too many workqueues being made and you do not have to use WQ_UNBOUND as a work around. Why WQ_UNBOUND? It seems other drivers prefer it the other way. The intention of using the flag WQ_UNBOUND was to have a single_work_queue across all cores for each EQ created. Default implementation of alloc_workqueue is creating workqueue per core, in driver EQ creation depends on NUM_CPU and based on that too many workqueues will be created which can be counter productive. This scenario is hit only in blk_iopoll disable mode and the latest kernel it is always enabled. So used the WQ_UNBOUND flag to make is as single thread. Thanks Sony -- 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 -- 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: Soft lockup in scsi_remove_target under 3.6 (regression from 3.5)
On 10/02/2012 07:43 PM, Jonathan McDowell wrote: Upgraded to 3.6 today on my dev box and after seeing an FC attached SAN go down and come back up (due to an expected reboot) I started getting the following in my logs. It continues even after the array is back and functioning - I'm seeing: kernel:[109104.348034] BUG: soft lockup - CPU#6 stuck for 23s! [kworker/6:0:30692] repeated on logged in sessions and backtraces like the following (this is the first). I don't see the same problem under 3.5. I think you need this patch http://marc.info/?l=linux-scsim=134621716223056w=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
