Re: [PATCH-v2 10/17] target: Add protection SGLs to target_submit_cmd_map_sgls
On 1/22/2014 12:17 AM, Nicholas A. Bellinger wrote: On Sun, 2014-01-19 at 14:12 +0200, Sagi Grimberg wrote: On 1/19/2014 4:44 AM, Nicholas A. Bellinger wrote: From: Nicholas Bellinger This patch adds support to target_submit_cmd_map_sgls() for accepting 'sgl_prot' + 'sgl_prot_count' parameters for DIF protection information. Note the passed parameters are stored at se_cmd->t_prot_sg and se_cmd->t_prot_nents respectively. Also, update tcm_loop and vhost-scsi fabrics usage of target_submit_cmd_map_sgls() to take into account the new parameters. I didn't see that you added protection allocation to transports that does not use target_submit_cmd_map_sgls() - which happens to be iSCSI/iSER/SRP :( Don't you think that prot SG allocation should be added also to target_alloc_sgl()? by then se_cmd should contain the protection attributes and this routine can know if it needs to allocate prot_sg as well. This is how I used it... Yes, this specific bit was left out for the moment as no code in the patch for v3.14 actually uses it.. I'm planning to add it to for-next -> v3.15 code as soon as the merge window closes. --nab Yes, that makes sense to me. Sagi. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH-v2 10/17] target: Add protection SGLs to target_submit_cmd_map_sgls
On 1/22/2014 12:17 AM, Nicholas A. Bellinger wrote: On Sun, 2014-01-19 at 14:12 +0200, Sagi Grimberg wrote: On 1/19/2014 4:44 AM, Nicholas A. Bellinger wrote: From: Nicholas Bellinger n...@linux-iscsi.org This patch adds support to target_submit_cmd_map_sgls() for accepting 'sgl_prot' + 'sgl_prot_count' parameters for DIF protection information. Note the passed parameters are stored at se_cmd-t_prot_sg and se_cmd-t_prot_nents respectively. Also, update tcm_loop and vhost-scsi fabrics usage of target_submit_cmd_map_sgls() to take into account the new parameters. I didn't see that you added protection allocation to transports that does not use target_submit_cmd_map_sgls() - which happens to be iSCSI/iSER/SRP :( Don't you think that prot SG allocation should be added also to target_alloc_sgl()? by then se_cmd should contain the protection attributes and this routine can know if it needs to allocate prot_sg as well. This is how I used it... Yes, this specific bit was left out for the moment as no code in the patch for v3.14 actually uses it.. I'm planning to add it to for-next - v3.15 code as soon as the merge window closes. --nab Yes, that makes sense to me. Sagi. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH-v2 10/17] target: Add protection SGLs to target_submit_cmd_map_sgls
On Sun, 2014-01-19 at 14:12 +0200, Sagi Grimberg wrote: > On 1/19/2014 4:44 AM, Nicholas A. Bellinger wrote: > > From: Nicholas Bellinger > > > > This patch adds support to target_submit_cmd_map_sgls() for > > accepting 'sgl_prot' + 'sgl_prot_count' parameters for > > DIF protection information. > > > > Note the passed parameters are stored at se_cmd->t_prot_sg > > and se_cmd->t_prot_nents respectively. > > > > Also, update tcm_loop and vhost-scsi fabrics usage of > > target_submit_cmd_map_sgls() to take into account the > > new parameters. > > I didn't see that you added protection allocation to transports that > does not use > target_submit_cmd_map_sgls() - which happens to be iSCSI/iSER/SRP :( > > Don't you think that prot SG allocation should be added also to > target_alloc_sgl()? by then > se_cmd should contain the protection attributes and this routine can > know if it needs to > allocate prot_sg as well. This is how I used it... Yes, this specific bit was left out for the moment as no code in the patch for v3.14 actually uses it.. I'm planning to add it to for-next -> v3.15 code as soon as the merge window closes. --nab -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH-v2 10/17] target: Add protection SGLs to target_submit_cmd_map_sgls
On Sun, 2014-01-19 at 14:12 +0200, Sagi Grimberg wrote: On 1/19/2014 4:44 AM, Nicholas A. Bellinger wrote: From: Nicholas Bellinger n...@linux-iscsi.org This patch adds support to target_submit_cmd_map_sgls() for accepting 'sgl_prot' + 'sgl_prot_count' parameters for DIF protection information. Note the passed parameters are stored at se_cmd-t_prot_sg and se_cmd-t_prot_nents respectively. Also, update tcm_loop and vhost-scsi fabrics usage of target_submit_cmd_map_sgls() to take into account the new parameters. I didn't see that you added protection allocation to transports that does not use target_submit_cmd_map_sgls() - which happens to be iSCSI/iSER/SRP :( Don't you think that prot SG allocation should be added also to target_alloc_sgl()? by then se_cmd should contain the protection attributes and this routine can know if it needs to allocate prot_sg as well. This is how I used it... Yes, this specific bit was left out for the moment as no code in the patch for v3.14 actually uses it.. I'm planning to add it to for-next - v3.15 code as soon as the merge window closes. --nab -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH-v2 10/17] target: Add protection SGLs to target_submit_cmd_map_sgls
On 1/19/2014 4:44 AM, Nicholas A. Bellinger wrote: From: Nicholas Bellinger This patch adds support to target_submit_cmd_map_sgls() for accepting 'sgl_prot' + 'sgl_prot_count' parameters for DIF protection information. Note the passed parameters are stored at se_cmd->t_prot_sg and se_cmd->t_prot_nents respectively. Also, update tcm_loop and vhost-scsi fabrics usage of target_submit_cmd_map_sgls() to take into account the new parameters. I didn't see that you added protection allocation to transports that does not use target_submit_cmd_map_sgls() - which happens to be iSCSI/iSER/SRP :( Don't you think that prot SG allocation should be added also to target_alloc_sgl()? by then se_cmd should contain the protection attributes and this routine can know if it needs to allocate prot_sg as well. This is how I used it... Cc: Martin K. Petersen Cc: Christoph Hellwig Cc: Hannes Reinecke Cc: Sagi Grimberg Cc: Or Gerlitz Signed-off-by: Nicholas Bellinger --- drivers/target/loopback/tcm_loop.c |2 +- drivers/target/target_core_transport.c | 16 ++-- drivers/vhost/scsi.c |2 +- include/target/target_core_fabric.h|3 ++- 4 files changed, 18 insertions(+), 5 deletions(-) diff --git a/drivers/target/loopback/tcm_loop.c b/drivers/target/loopback/tcm_loop.c index 763ee45..112b795 100644 --- a/drivers/target/loopback/tcm_loop.c +++ b/drivers/target/loopback/tcm_loop.c @@ -217,7 +217,7 @@ static void tcm_loop_submission_work(struct work_struct *work) 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); + sgl_bidi, sgl_bidi_count, NULL, 0); if (rc < 0) { set_host_byte(sc, DID_NO_CONNECT); goto out_done; diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index fa4fc04..aebe0bb 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -1310,6 +1310,8 @@ transport_generic_map_mem_to_cmd(struct se_cmd *cmd, struct scatterlist *sgl, * @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 + * @sgl_prot: struct scatterlist memory protection information + * @sgl_prot_count: scatterlist count for protection information * * Returns non zero to signal active I/O shutdown failure. All other * setup exceptions will be returned as a SCSI CHECK_CONDITION response, @@ -1322,7 +1324,8 @@ 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, struct scatterlist *sgl, u32 sgl_count, - struct scatterlist *sgl_bidi, u32 sgl_bidi_count) + struct scatterlist *sgl_bidi, u32 sgl_bidi_count, + struct scatterlist *sgl_prot, u32 sgl_prot_count) { struct se_portal_group *se_tpg; sense_reason_t rc; @@ -1364,6 +1367,14 @@ int target_submit_cmd_map_sgls(struct se_cmd *se_cmd, struct se_session *se_sess target_put_sess_cmd(se_sess, se_cmd); return 0; } + /* +* Save pointers for SGLs containing protection information, +* if present. +*/ + if (sgl_prot_count) { + se_cmd->t_prot_sg = sgl_prot; + se_cmd->t_prot_nents = sgl_prot_count; + } rc = target_setup_cmd_from_cdb(se_cmd, cdb); if (rc != 0) { @@ -1406,6 +1417,7 @@ int target_submit_cmd_map_sgls(struct se_cmd *se_cmd, struct se_session *se_sess return 0; } } + /* * Check if we need to delay processing because of ALUA * Active/NonOptimized primary access state.. @@ -1445,7 +1457,7 @@ int target_submit_cmd(struct se_cmd *se_cmd, struct se_session *se_sess, { return target_submit_cmd_map_sgls(se_cmd, se_sess, cdb, sense, unpacked_lun, data_length, task_attr, data_dir, - flags, NULL, 0, NULL, 0); + flags, NULL, 0, NULL, 0, NULL, 0); } EXPORT_SYMBOL(target_submit_cmd); diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c index f175629..84488a8 100644 --- a/drivers/vhost/scsi.c +++ b/drivers/vhost/scsi.c @@ -889,7 +889,7 @@ static void tcm_vhost_submission_work(struct work_struct *work) cmd->tvc_lun, cmd->tvc_exp_data_len, cmd->tvc_task_attr, cmd->tvc_data_direction, TARGET_SCF_ACK_KREF, sg_ptr, cmd->tvc_sgl_count, - sg_bidi_ptr,
Re: [PATCH-v2 10/17] target: Add protection SGLs to target_submit_cmd_map_sgls
On 1/19/2014 4:44 AM, Nicholas A. Bellinger wrote: From: Nicholas Bellinger n...@linux-iscsi.org This patch adds support to target_submit_cmd_map_sgls() for accepting 'sgl_prot' + 'sgl_prot_count' parameters for DIF protection information. Note the passed parameters are stored at se_cmd-t_prot_sg and se_cmd-t_prot_nents respectively. Also, update tcm_loop and vhost-scsi fabrics usage of target_submit_cmd_map_sgls() to take into account the new parameters. I didn't see that you added protection allocation to transports that does not use target_submit_cmd_map_sgls() - which happens to be iSCSI/iSER/SRP :( Don't you think that prot SG allocation should be added also to target_alloc_sgl()? by then se_cmd should contain the protection attributes and this routine can know if it needs to allocate prot_sg as well. This is how I used it... Cc: Martin K. Petersen martin.peter...@oracle.com Cc: Christoph Hellwig h...@lst.de Cc: Hannes Reinecke h...@suse.de Cc: Sagi Grimberg sa...@mellanox.com Cc: Or Gerlitz ogerl...@mellanox.com Signed-off-by: Nicholas Bellinger n...@linux-iscsi.org --- drivers/target/loopback/tcm_loop.c |2 +- drivers/target/target_core_transport.c | 16 ++-- drivers/vhost/scsi.c |2 +- include/target/target_core_fabric.h|3 ++- 4 files changed, 18 insertions(+), 5 deletions(-) diff --git a/drivers/target/loopback/tcm_loop.c b/drivers/target/loopback/tcm_loop.c index 763ee45..112b795 100644 --- a/drivers/target/loopback/tcm_loop.c +++ b/drivers/target/loopback/tcm_loop.c @@ -217,7 +217,7 @@ static void tcm_loop_submission_work(struct work_struct *work) 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); + sgl_bidi, sgl_bidi_count, NULL, 0); if (rc 0) { set_host_byte(sc, DID_NO_CONNECT); goto out_done; diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index fa4fc04..aebe0bb 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -1310,6 +1310,8 @@ transport_generic_map_mem_to_cmd(struct se_cmd *cmd, struct scatterlist *sgl, * @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 + * @sgl_prot: struct scatterlist memory protection information + * @sgl_prot_count: scatterlist count for protection information * * Returns non zero to signal active I/O shutdown failure. All other * setup exceptions will be returned as a SCSI CHECK_CONDITION response, @@ -1322,7 +1324,8 @@ 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, struct scatterlist *sgl, u32 sgl_count, - struct scatterlist *sgl_bidi, u32 sgl_bidi_count) + struct scatterlist *sgl_bidi, u32 sgl_bidi_count, + struct scatterlist *sgl_prot, u32 sgl_prot_count) { struct se_portal_group *se_tpg; sense_reason_t rc; @@ -1364,6 +1367,14 @@ int target_submit_cmd_map_sgls(struct se_cmd *se_cmd, struct se_session *se_sess target_put_sess_cmd(se_sess, se_cmd); return 0; } + /* +* Save pointers for SGLs containing protection information, +* if present. +*/ + if (sgl_prot_count) { + se_cmd-t_prot_sg = sgl_prot; + se_cmd-t_prot_nents = sgl_prot_count; + } rc = target_setup_cmd_from_cdb(se_cmd, cdb); if (rc != 0) { @@ -1406,6 +1417,7 @@ int target_submit_cmd_map_sgls(struct se_cmd *se_cmd, struct se_session *se_sess return 0; } } + /* * Check if we need to delay processing because of ALUA * Active/NonOptimized primary access state.. @@ -1445,7 +1457,7 @@ int target_submit_cmd(struct se_cmd *se_cmd, struct se_session *se_sess, { return target_submit_cmd_map_sgls(se_cmd, se_sess, cdb, sense, unpacked_lun, data_length, task_attr, data_dir, - flags, NULL, 0, NULL, 0); + flags, NULL, 0, NULL, 0, NULL, 0); } EXPORT_SYMBOL(target_submit_cmd); diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c index f175629..84488a8 100644 --- a/drivers/vhost/scsi.c +++ b/drivers/vhost/scsi.c @@ -889,7 +889,7 @@ static void tcm_vhost_submission_work(struct work_struct *work) cmd-tvc_lun, cmd-tvc_exp_data_len, cmd-tvc_task_attr,