Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1656941 > unrolled thread

[PATCH 0/3] target: Add TARGET_SCF_LOOKUP_LUN_FROM_TAG support

Started by"Nicholas A. Bellinger" <nab@linux-iscsi.org>
First post2017-06-04 00:10 +0200
Last post2017-06-07 07:10 +0200
Articles 8 — 4 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/3] target: Add TARGET_SCF_LOOKUP_LUN_FROM_TAG support "Nicholas A. Bellinger" <nab@linux-iscsi.org> - 2017-06-04 00:10 +0200
    [PATCH 2/3] target: Add TARGET_SCF_LOOKUP_LUN_FROM_TAG support for ABORT_TASK "Nicholas A. Bellinger" <nab@linux-iscsi.org> - 2017-06-04 00:10 +0200
      Re: [PATCH 2/3] target: Add TARGET_SCF_LOOKUP_LUN_FROM_TAG support  for ABORT_TASK Bart Van Assche <Bart.VanAssche@sandisk.com> - 2017-06-05 18:00 +0200
      Re: [PATCH 2/3] target: Add TARGET_SCF_LOOKUP_LUN_FROM_TAG support  for ABORT_TASK "Tran, Quinn" <Quinn.Tran@cavium.com> - 2017-06-07 03:20 +0200
    [PATCH 1/3] target: Add support for TMR percpu reference counting "Nicholas A. Bellinger" <nab@linux-iscsi.org> - 2017-06-04 00:10 +0200
      Re: [PATCH 1/3] target: Add support for TMR percpu reference counting "Tran, Quinn" <Quinn.Tran@cavium.com> - 2017-06-07 03:20 +0200
    Re: [PATCH 0/3] target: Add TARGET_SCF_LOOKUP_LUN_FROM_TAG support "Madhani, Himanshu" <Himanshu.Madhani@cavium.com> - 2017-06-05 17:40 +0200
    Re: [PATCH 0/3] target: Add TARGET_SCF_LOOKUP_LUN_FROM_TAG support "Madhani, Himanshu" <Himanshu.Madhani@cavium.com> - 2017-06-07 07:10 +0200

#1656941 — [PATCH 0/3] target: Add TARGET_SCF_LOOKUP_LUN_FROM_TAG support

From"Nicholas A. Bellinger" <nab@linux-iscsi.org>
Date2017-06-04 00:10 +0200
Subject[PATCH 0/3] target: Add TARGET_SCF_LOOKUP_LUN_FROM_TAG support
Message-ID<tOpp7-4fW-3@gated-at.bofh.it>
From: Nicholas Bellinger <nab@linux-iscsi.org>

Hi Himanshu + Quinn,

Here is a small series to introduce proper percpu se_lun->lun_ref
counting for TMR, and add common code in target_submit_tmr() to
do tag lookup for unpacked_lun in order to drop the original
driver specific lookup within __qlt_24xx_handle_abts().

It's rather straight-forward, so review and test as a v4.13 item.

Thanks!

Nicholas Bellinger (3):
  target: Add support for TMR percpu reference counting
  target: Add TARGET_SCF_LOOKUP_LUN_FROM_TAG support for ABORT_TASK
  qla2xxx: Convert QLA_TGT_ABTS to TARGET_SCF_LOOKUP_LUN_FROM_TAG

 drivers/scsi/qla2xxx/qla_target.c      | 39 ++++++-----------------
 drivers/scsi/qla2xxx/tcm_qla2xxx.c     |  4 ++-
 drivers/target/target_core_device.c    | 14 ++++++---
 drivers/target/target_core_transport.c | 56 ++++++++++++++++++++++++++++------
 include/target/target_core_base.h      |  3 +-
 5 files changed, 71 insertions(+), 45 deletions(-)

-- 
1.9.1

[toc] | [next] | [standalone]

#1656943 — [PATCH 2/3] target: Add TARGET_SCF_LOOKUP_LUN_FROM_TAG support for ABORT_TASK

From"Nicholas A. Bellinger" <nab@linux-iscsi.org>
Date2017-06-04 00:10 +0200
Subject[PATCH 2/3] target: Add TARGET_SCF_LOOKUP_LUN_FROM_TAG support for ABORT_TASK
Message-ID<tOpp7-4fW-7@gated-at.bofh.it>
In reply to#1656941
From: Nicholas Bellinger <nab@linux-iscsi.org>

This patch introduces support in target_submit_tmr() for locating a
unpacked_lun from an existing se_cmd->tag during ABORT_TASK.

When TARGET_SCF_LOOKUP_LUN_FROM_TAG is set, target_submit_tmr()
will do the extra lookup via target_lookup_lun_from_tag() and
subsequently invoke transport_lookup_tmr_lun() so a proper
percpu se_lun->lun_ref is taken before workqueue dispatch into
se_device->tmr_wq happens.

Aside from the extra target_lookup_lun_from_tag(), the existing
code-path remains unchanged.

Cc: Himanshu Madhani <himanshu.madhani@cavium.com>
Cc: Quinn Tran <quinn.tran@cavium.com>
Cc: Mike Christie <mchristi@redhat.com>
Cc: Hannes Reinecke <hare@suse.com>
Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 drivers/target/target_core_transport.c | 53 ++++++++++++++++++++++++++++------
 include/target/target_core_base.h      |  3 +-
 2 files changed, 46 insertions(+), 10 deletions(-)

diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index 83bfc97..dbb8101 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -1592,6 +1592,29 @@ static void target_complete_tmr_failure(struct work_struct *work)
 	transport_cmd_check_stop_to_fabric(se_cmd);
 }
 
+static bool target_lookup_lun_from_tag(struct se_session *se_sess, u64 tag,
+				       u64 *unpacked_lun)
+{
+	struct se_cmd *se_cmd;
+	unsigned long flags;
+	bool ret = false;
+
+	spin_lock_irqsave(&se_sess->sess_cmd_lock, flags);
+	list_for_each_entry(se_cmd, &se_sess->sess_cmd_list, se_cmd_list) {
+		if (se_cmd->se_cmd_flags & SCF_SCSI_TMR_CDB)
+			continue;
+
+		if (se_cmd->tag == tag) {
+			*unpacked_lun = se_cmd->orig_fe_lun;
+			ret = true;
+			break;
+		}
+	}
+	spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags);
+
+	return ret;
+}
+
 /**
  * target_submit_tmr - lookup unpacked lun and submit uninitialized se_cmd
  *                     for TMR CDBs
@@ -1639,19 +1662,31 @@ int target_submit_tmr(struct se_cmd *se_cmd, struct se_session *se_sess,
 		core_tmr_release_req(se_cmd->se_tmr_req);
 		return ret;
 	}
+	/*
+	 * If this is ABORT_TASK with no explicit fabric provided LUN,
+	 * go ahead and search active session tags for a match to figure
+	 * out unpacked_lun for the original se_cmd.
+	 */
+	if (tm_type == TMR_ABORT_TASK && (flags & TARGET_SCF_LOOKUP_LUN_FROM_TAG)) {
+		if (!target_lookup_lun_from_tag(se_sess, tag, &unpacked_lun))
+			goto failure;
+	}
 
 	ret = transport_lookup_tmr_lun(se_cmd, unpacked_lun);
-	if (ret) {
-		/*
-		 * For callback during failure handling, push this work off
-		 * to process context with TMR_LUN_DOES_NOT_EXIST status.
-		 */
-		INIT_WORK(&se_cmd->work, target_complete_tmr_failure);
-		schedule_work(&se_cmd->work);
-		return 0;
-	}
+	if (ret)
+		goto failure;
+
 	transport_generic_handle_tmr(se_cmd);
 	return 0;
+
+	/*
+	 * For callback during failure handling, push this work off
+	 * to process context with TMR_LUN_DOES_NOT_EXIST status.
+	 */
+failure:
+	INIT_WORK(&se_cmd->work, target_complete_tmr_failure);
+	schedule_work(&se_cmd->work);
+	return 0;
 }
 EXPORT_SYMBOL(target_submit_tmr);
 
diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
index db2c7b3..a3af69f 100644
--- a/include/target/target_core_base.h
+++ b/include/target/target_core_base.h
@@ -188,7 +188,8 @@ enum target_sc_flags_table {
 	TARGET_SCF_BIDI_OP		= 0x01,
 	TARGET_SCF_ACK_KREF		= 0x02,
 	TARGET_SCF_UNKNOWN_SIZE		= 0x04,
-	TARGET_SCF_USE_CPUID	= 0x08,
+	TARGET_SCF_USE_CPUID		= 0x08,
+	TARGET_SCF_LOOKUP_LUN_FROM_TAG	= 0x10,
 };
 
 /* fabric independent task management function values */
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1657682 — Re: [PATCH 2/3] target: Add TARGET_SCF_LOOKUP_LUN_FROM_TAG support for ABORT_TASK

FromBart Van Assche <Bart.VanAssche@sandisk.com>
Date2017-06-05 18:00 +0200
SubjectRe: [PATCH 2/3] target: Add TARGET_SCF_LOOKUP_LUN_FROM_TAG support for ABORT_TASK
Message-ID<tP2A9-4My-1@gated-at.bofh.it>
In reply to#1656943
On Sat, 2017-06-03 at 22:10 +0000, Nicholas A. Bellinger wrote:
> +static bool target_lookup_lun_from_tag(struct se_session *se_sess, u64 tag,
> +				       u64 *unpacked_lun)
> +{
> +	struct se_cmd *se_cmd;
> +	unsigned long flags;
> +	bool ret = false;
> +
> +	spin_lock_irqsave(&se_sess->sess_cmd_lock, flags);
> +	list_for_each_entry(se_cmd, &se_sess->sess_cmd_list, se_cmd_list) {
> +		if (se_cmd->se_cmd_flags & SCF_SCSI_TMR_CDB)
> +			continue;
> +
> +		if (se_cmd->tag == tag) {
> +			*unpacked_lun = se_cmd->orig_fe_lun;
> +			ret = true;
> +			break;
> +		}
> +	}
> +	spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags);
> +
> +	return ret;
> +}
> +
>  /**
>   * target_submit_tmr - lookup unpacked lun and submit uninitialized se_cmd
>   *                     for TMR CDBs
> @@ -1639,19 +1662,31 @@ int target_submit_tmr(struct se_cmd *se_cmd, struct se_session *se_sess,
>  		core_tmr_release_req(se_cmd->se_tmr_req);
>  		return ret;
>  	}
> +	/*
> +	 * If this is ABORT_TASK with no explicit fabric provided LUN,
> +	 * go ahead and search active session tags for a match to figure
> +	 * out unpacked_lun for the original se_cmd.
> +	 */
> +	if (tm_type == TMR_ABORT_TASK && (flags & TARGET_SCF_LOOKUP_LUN_FROM_TAG)) {
> +		if (!target_lookup_lun_from_tag(se_sess, tag, &unpacked_lun))
> +			goto failure;
> +	}
>  
>  	ret = transport_lookup_tmr_lun(se_cmd, unpacked_lun);
> -	if (ret) {
> -		/*
> -		 * For callback during failure handling, push this work off
> -		 * to process context with TMR_LUN_DOES_NOT_EXIST status.
> -		 */
> -		INIT_WORK(&se_cmd->work, target_complete_tmr_failure);
> -		schedule_work(&se_cmd->work);
> -		return 0;
> -	}
> +	if (ret)
> +		goto failure;
> +
>  	transport_generic_handle_tmr(se_cmd);
>  	return 0;

Hello Nic,

So after LUN lookup has finished sess_cmd_lock lock is dropped, a context
switch occurs due to the queue_work() call in transport_generic_handle_tmr()
and next core_tmr_abort_task() reacquires that lock? Sorry but I'm afraid
that if after that lock is dropped and before it is reacquired a command
finishes and the initiator reuses the same tag for another command for the
same LUN that this may result in the wrong command being aborted.

Bart.

[toc] | [prev] | [next] | [standalone]

#1659326 — Re: [PATCH 2/3] target: Add TARGET_SCF_LOOKUP_LUN_FROM_TAG support for ABORT_TASK

From"Tran, Quinn" <Quinn.Tran@cavium.com>
Date2017-06-07 03:20 +0200
SubjectRe: [PATCH 2/3] target: Add TARGET_SCF_LOOKUP_LUN_FROM_TAG support for ABORT_TASK
Message-ID<tPxND-8aQ-1@gated-at.bofh.it>
In reply to#1656943
Looks Good.

Regards,
Quinn Tran

-----Original Message-----
From: Nicholas Bellinger <nab@linux-iscsi.org>
Date: Saturday, June 3, 2017 at 3:10 PM
To: target-devel <target-devel@vger.kernel.org>
Cc: linux-scsi <linux-scsi@vger.kernel.org>, lkml <linux-kernel@vger.kernel.org>, Nicholas Bellinger <nab@linux-iscsi.org>, "Madhani, Himanshu" <Himanshu.Madhani@cavium.com>, "Tran, Quinn" <Quinn.Tran@cavium.com>, Mike Christie <mchristi@redhat.com>, Hannes Reinecke <hare@suse.com>, Christoph Hellwig <hch@lst.de>
Subject: [PATCH 2/3] target: Add TARGET_SCF_LOOKUP_LUN_FROM_TAG support for ABORT_TASK

    From: Nicholas Bellinger <nab@linux-iscsi.org>
    
    This patch introduces support in target_submit_tmr() for locating a
    unpacked_lun from an existing se_cmd->tag during ABORT_TASK.
    
    When TARGET_SCF_LOOKUP_LUN_FROM_TAG is set, target_submit_tmr()
    will do the extra lookup via target_lookup_lun_from_tag() and
    subsequently invoke transport_lookup_tmr_lun() so a proper
    percpu se_lun->lun_ref is taken before workqueue dispatch into
    se_device->tmr_wq happens.
    
    Aside from the extra target_lookup_lun_from_tag(), the existing
    code-path remains unchanged.
    
    Cc: Himanshu Madhani <himanshu.madhani@cavium.com>
    Cc: Quinn Tran <quinn.tran@cavium.com>
    Cc: Mike Christie <mchristi@redhat.com>
    Cc: Hannes Reinecke <hare@suse.com>
    Cc: Christoph Hellwig <hch@lst.de>
    Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
    ---
     drivers/target/target_core_transport.c | 53 ++++++++++++++++++++++++++++------
     include/target/target_core_base.h      |  3 +-
     2 files changed, 46 insertions(+), 10 deletions(-)
    
    diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
    index 83bfc97..dbb8101 100644
    --- a/drivers/target/target_core_transport.c
    +++ b/drivers/target/target_core_transport.c
    @@ -1592,6 +1592,29 @@ static void target_complete_tmr_failure(struct work_struct *work)
     	transport_cmd_check_stop_to_fabric(se_cmd);
     }
     
    +static bool target_lookup_lun_from_tag(struct se_session *se_sess, u64 tag,
    +				       u64 *unpacked_lun)
    +{
    +	struct se_cmd *se_cmd;
    +	unsigned long flags;
    +	bool ret = false;
    +
    +	spin_lock_irqsave(&se_sess->sess_cmd_lock, flags);
    +	list_for_each_entry(se_cmd, &se_sess->sess_cmd_list, se_cmd_list) {
    +		if (se_cmd->se_cmd_flags & SCF_SCSI_TMR_CDB)
    +			continue;
    +
    +		if (se_cmd->tag == tag) {
    +			*unpacked_lun = se_cmd->orig_fe_lun;
    +			ret = true;
    +			break;
    +		}
    +	}
    +	spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags);
    +
    +	return ret;
    +}
    +
     /**
      * target_submit_tmr - lookup unpacked lun and submit uninitialized se_cmd
      *                     for TMR CDBs
    @@ -1639,19 +1662,31 @@ int target_submit_tmr(struct se_cmd *se_cmd, struct se_session *se_sess,
     		core_tmr_release_req(se_cmd->se_tmr_req);
     		return ret;
     	}
    +	/*
    +	 * If this is ABORT_TASK with no explicit fabric provided LUN,
    +	 * go ahead and search active session tags for a match to figure
    +	 * out unpacked_lun for the original se_cmd.
    +	 */
    +	if (tm_type == TMR_ABORT_TASK && (flags & TARGET_SCF_LOOKUP_LUN_FROM_TAG)) {
    +		if (!target_lookup_lun_from_tag(se_sess, tag, &unpacked_lun))
    +			goto failure;
    +	}
     
     	ret = transport_lookup_tmr_lun(se_cmd, unpacked_lun);
    -	if (ret) {
    -		/*
    -		 * For callback during failure handling, push this work off
    -		 * to process context with TMR_LUN_DOES_NOT_EXIST status.
    -		 */
    -		INIT_WORK(&se_cmd->work, target_complete_tmr_failure);
    -		schedule_work(&se_cmd->work);
    -		return 0;
    -	}
    +	if (ret)
    +		goto failure;
    +
     	transport_generic_handle_tmr(se_cmd);
     	return 0;
    +
    +	/*
    +	 * For callback during failure handling, push this work off
    +	 * to process context with TMR_LUN_DOES_NOT_EXIST status.
    +	 */
    +failure:
    +	INIT_WORK(&se_cmd->work, target_complete_tmr_failure);
    +	schedule_work(&se_cmd->work);
    +	return 0;
     }
     EXPORT_SYMBOL(target_submit_tmr);
     
    diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h
    index db2c7b3..a3af69f 100644
    --- a/include/target/target_core_base.h
    +++ b/include/target/target_core_base.h
    @@ -188,7 +188,8 @@ enum target_sc_flags_table {
     	TARGET_SCF_BIDI_OP		= 0x01,
     	TARGET_SCF_ACK_KREF		= 0x02,
     	TARGET_SCF_UNKNOWN_SIZE		= 0x04,
    -	TARGET_SCF_USE_CPUID	= 0x08,
    +	TARGET_SCF_USE_CPUID		= 0x08,
    +	TARGET_SCF_LOOKUP_LUN_FROM_TAG	= 0x10,
     };
     
     /* fabric independent task management function values */
    -- 
    1.9.1

[toc] | [prev] | [next] | [standalone]

#1656944 — [PATCH 1/3] target: Add support for TMR percpu reference counting

From"Nicholas A. Bellinger" <nab@linux-iscsi.org>
Date2017-06-04 00:10 +0200
Subject[PATCH 1/3] target: Add support for TMR percpu reference counting
Message-ID<tOpp7-4fW-9@gated-at.bofh.it>
In reply to#1656941
From: Nicholas Bellinger <nab@linux-iscsi.org>

This patch introduces TMR percpu reference counting using
se_lun->lun_ref in transport_lookup_tmr_lun(), following
how existing non TMR per se_lun reference counting works
within transport_lookup_cmd_lun().

It also adds explicit transport_lun_remove_cmd() calls to
drop the reference in the three tmr related locations that
invoke transport_cmd_check_stop_to_fabric();

   - target_tmr_work() during normal ->queue_tm_rsp()
   - target_complete_tmr_failure() during error ->queue_tm_rsp()
   - transport_generic_handle_tmr() during early failure

Also, note the exception paths in transport_generic_free_cmd()
and transport_cmd_finish_abort() already check SCF_SE_LUN_CMD,
and will invoke transport_lun_remove_cmd() when necessary.

Cc: Himanshu Madhani <himanshu.madhani@cavium.com>
Cc: Quinn Tran <quinn.tran@cavium.com>
Cc: Mike Christie <mchristi@redhat.com>
Cc: Hannes Reinecke <hare@suse.com>
Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
---
 drivers/target/target_core_device.c    | 14 ++++++++++----
 drivers/target/target_core_transport.c |  3 +++
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
index 8f0e0e3..11c80c4 100644
--- a/drivers/target/target_core_device.c
+++ b/drivers/target/target_core_device.c
@@ -168,11 +168,20 @@ int transport_lookup_tmr_lun(struct se_cmd *se_cmd, u64 unpacked_lun)
 	rcu_read_lock();
 	deve = target_nacl_find_deve(nacl, unpacked_lun);
 	if (deve) {
-		se_cmd->se_lun = rcu_dereference(deve->se_lun);
 		se_lun = rcu_dereference(deve->se_lun);
+
+		if (!percpu_ref_tryget_live(&se_lun->lun_ref)) {
+			se_lun = NULL;
+			goto out_unlock;
+		}
+
+		se_cmd->se_lun = rcu_dereference(deve->se_lun);
 		se_cmd->pr_res_key = deve->pr_res_key;
 		se_cmd->orig_fe_lun = unpacked_lun;
+		se_cmd->se_cmd_flags |= SCF_SE_LUN_CMD;
+		se_cmd->lun_ref_active = true;
 	}
+out_unlock:
 	rcu_read_unlock();
 
 	if (!se_lun) {
@@ -182,9 +191,6 @@ int transport_lookup_tmr_lun(struct se_cmd *se_cmd, u64 unpacked_lun)
 			unpacked_lun);
 		return -ENODEV;
 	}
-	/*
-	 * XXX: Add percpu se_lun->lun_ref reference count for TMR
-	 */
 	se_cmd->se_dev = rcu_dereference_raw(se_lun->lun_se_dev);
 	se_tmr->tmr_dev = rcu_dereference_raw(se_lun->lun_se_dev);
 
diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index f16a789..83bfc97 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -1588,6 +1588,7 @@ static void target_complete_tmr_failure(struct work_struct *work)
 	se_cmd->se_tmr_req->response = TMR_LUN_DOES_NOT_EXIST;
 	se_cmd->se_tfo->queue_tm_rsp(se_cmd);
 
+	transport_lun_remove_cmd(se_cmd);
 	transport_cmd_check_stop_to_fabric(se_cmd);
 }
 
@@ -3199,6 +3200,7 @@ static void target_tmr_work(struct work_struct *work)
 	cmd->se_tfo->queue_tm_rsp(cmd);
 
 check_stop:
+	transport_lun_remove_cmd(cmd);
 	transport_cmd_check_stop_to_fabric(cmd);
 }
 
@@ -3221,6 +3223,7 @@ int transport_generic_handle_tmr(
 		pr_warn_ratelimited("handle_tmr caught CMD_T_ABORTED TMR %d"
 			"ref_tag: %llu tag: %llu\n", cmd->se_tmr_req->function,
 			cmd->se_tmr_req->ref_task_tag, cmd->tag);
+		transport_lun_remove_cmd(cmd);
 		transport_cmd_check_stop_to_fabric(cmd);
 		return 0;
 	}
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1659328 — Re: [PATCH 1/3] target: Add support for TMR percpu reference counting

From"Tran, Quinn" <Quinn.Tran@cavium.com>
Date2017-06-07 03:20 +0200
SubjectRe: [PATCH 1/3] target: Add support for TMR percpu reference counting
Message-ID<tPxNE-8aQ-7@gated-at.bofh.it>
In reply to#1656944
Looks good.

Regards,
Quinn Tran

-----Original Message-----
From: Nicholas Bellinger <nab@linux-iscsi.org>
Date: Saturday, June 3, 2017 at 3:10 PM
To: target-devel <target-devel@vger.kernel.org>
Cc: linux-scsi <linux-scsi@vger.kernel.org>, lkml <linux-kernel@vger.kernel.org>, Nicholas Bellinger <nab@linux-iscsi.org>, "Madhani, Himanshu" <Himanshu.Madhani@cavium.com>, "Tran, Quinn" <Quinn.Tran@cavium.com>, Mike Christie <mchristi@redhat.com>, Hannes Reinecke <hare@suse.com>, Christoph Hellwig <hch@lst.de>
Subject: [PATCH 1/3] target: Add support for TMR percpu reference counting

    From: Nicholas Bellinger <nab@linux-iscsi.org>
    
    This patch introduces TMR percpu reference counting using
    se_lun->lun_ref in transport_lookup_tmr_lun(), following
    how existing non TMR per se_lun reference counting works
    within transport_lookup_cmd_lun().
    
    It also adds explicit transport_lun_remove_cmd() calls to
    drop the reference in the three tmr related locations that
    invoke transport_cmd_check_stop_to_fabric();
    
       - target_tmr_work() during normal ->queue_tm_rsp()
       - target_complete_tmr_failure() during error ->queue_tm_rsp()
       - transport_generic_handle_tmr() during early failure
    
    Also, note the exception paths in transport_generic_free_cmd()
    and transport_cmd_finish_abort() already check SCF_SE_LUN_CMD,
    and will invoke transport_lun_remove_cmd() when necessary.
    
    Cc: Himanshu Madhani <himanshu.madhani@cavium.com>
    Cc: Quinn Tran <quinn.tran@cavium.com>
    Cc: Mike Christie <mchristi@redhat.com>
    Cc: Hannes Reinecke <hare@suse.com>
    Cc: Christoph Hellwig <hch@lst.de>
    Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
    ---
     drivers/target/target_core_device.c    | 14 ++++++++++----
     drivers/target/target_core_transport.c |  3 +++
     2 files changed, 13 insertions(+), 4 deletions(-)
    
    diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c
    index 8f0e0e3..11c80c4 100644
    --- a/drivers/target/target_core_device.c
    +++ b/drivers/target/target_core_device.c
    @@ -168,11 +168,20 @@ int transport_lookup_tmr_lun(struct se_cmd *se_cmd, u64 unpacked_lun)
     	rcu_read_lock();
     	deve = target_nacl_find_deve(nacl, unpacked_lun);
     	if (deve) {
    -		se_cmd->se_lun = rcu_dereference(deve->se_lun);
     		se_lun = rcu_dereference(deve->se_lun);
    +
    +		if (!percpu_ref_tryget_live(&se_lun->lun_ref)) {
    +			se_lun = NULL;
    +			goto out_unlock;
    +		}
    +
    +		se_cmd->se_lun = rcu_dereference(deve->se_lun);
     		se_cmd->pr_res_key = deve->pr_res_key;
     		se_cmd->orig_fe_lun = unpacked_lun;
    +		se_cmd->se_cmd_flags |= SCF_SE_LUN_CMD;
    +		se_cmd->lun_ref_active = true;
     	}
    +out_unlock:
     	rcu_read_unlock();
     
     	if (!se_lun) {
    @@ -182,9 +191,6 @@ int transport_lookup_tmr_lun(struct se_cmd *se_cmd, u64 unpacked_lun)
     			unpacked_lun);
     		return -ENODEV;
     	}
    -	/*
    -	 * XXX: Add percpu se_lun->lun_ref reference count for TMR
    -	 */
     	se_cmd->se_dev = rcu_dereference_raw(se_lun->lun_se_dev);
     	se_tmr->tmr_dev = rcu_dereference_raw(se_lun->lun_se_dev);
     
    diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
    index f16a789..83bfc97 100644
    --- a/drivers/target/target_core_transport.c
    +++ b/drivers/target/target_core_transport.c
    @@ -1588,6 +1588,7 @@ static void target_complete_tmr_failure(struct work_struct *work)
     	se_cmd->se_tmr_req->response = TMR_LUN_DOES_NOT_EXIST;
     	se_cmd->se_tfo->queue_tm_rsp(se_cmd);
     
    +	transport_lun_remove_cmd(se_cmd);
     	transport_cmd_check_stop_to_fabric(se_cmd);
     }
     
    @@ -3199,6 +3200,7 @@ static void target_tmr_work(struct work_struct *work)
     	cmd->se_tfo->queue_tm_rsp(cmd);
     
     check_stop:
    +	transport_lun_remove_cmd(cmd);
     	transport_cmd_check_stop_to_fabric(cmd);
     }
     
    @@ -3221,6 +3223,7 @@ int transport_generic_handle_tmr(
     		pr_warn_ratelimited("handle_tmr caught CMD_T_ABORTED TMR %d"
     			"ref_tag: %llu tag: %llu\n", cmd->se_tmr_req->function,
     			cmd->se_tmr_req->ref_task_tag, cmd->tag);
    +		transport_lun_remove_cmd(cmd);
     		transport_cmd_check_stop_to_fabric(cmd);
     		return 0;
     	}
    -- 
    1.9.1

[toc] | [prev] | [next] | [standalone]

#1657674

From"Madhani, Himanshu" <Himanshu.Madhani@cavium.com>
Date2017-06-05 17:40 +0200
Message-ID<tP2gO-4G7-1@gated-at.bofh.it>
In reply to#1656941
> On Jun 3, 2017, at 3:10 PM, Nicholas A. Bellinger <nab@linux-iscsi.org> wrote:
> 
> From: Nicholas Bellinger <nab@linux-iscsi.org>
> 
> Hi Himanshu + Quinn,
> 
> Here is a small series to introduce proper percpu se_lun->lun_ref
> counting for TMR, and add common code in target_submit_tmr() to
> do tag lookup for unpacked_lun in order to drop the original
> driver specific lookup within __qlt_24xx_handle_abts().
> 
> It's rather straight-forward, so review and test as a v4.13 item.
> 

Sure will do. Thanks.

> Thanks!
> 
> Nicholas Bellinger (3):
>  target: Add support for TMR percpu reference counting
>  target: Add TARGET_SCF_LOOKUP_LUN_FROM_TAG support for ABORT_TASK
>  qla2xxx: Convert QLA_TGT_ABTS to TARGET_SCF_LOOKUP_LUN_FROM_TAG
> 
> drivers/scsi/qla2xxx/qla_target.c      | 39 ++++++-----------------
> drivers/scsi/qla2xxx/tcm_qla2xxx.c     |  4 ++-
> drivers/target/target_core_device.c    | 14 ++++++---
> drivers/target/target_core_transport.c | 56 ++++++++++++++++++++++++++++------
> include/target/target_core_base.h      |  3 +-
> 5 files changed, 71 insertions(+), 45 deletions(-)
> 
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe target-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


- Himanshu

[toc] | [prev] | [next] | [standalone]

#1659376

From"Madhani, Himanshu" <Himanshu.Madhani@cavium.com>
Date2017-06-07 07:10 +0200
Message-ID<tPBoe-2ah-7@gated-at.bofh.it>
In reply to#1656941
> On Jun 3, 2017, at 3:10 PM, Nicholas A. Bellinger <nab@linux-iscsi.org> wrote:
> 
> From: Nicholas Bellinger <nab@linux-iscsi.org>
> 
> Hi Himanshu + Quinn,
> 
> Here is a small series to introduce proper percpu se_lun->lun_ref
> counting for TMR, and add common code in target_submit_tmr() to
> do tag lookup for unpacked_lun in order to drop the original
> driver specific lookup within __qlt_24xx_handle_abts().
> 
> It's rather straight-forward, so review and test as a v4.13 item.
> 
> Thanks!
> 
> Nicholas Bellinger (3):
>  target: Add support for TMR percpu reference counting
>  target: Add TARGET_SCF_LOOKUP_LUN_FROM_TAG support for ABORT_TASK
>  qla2xxx: Convert QLA_TGT_ABTS to TARGET_SCF_LOOKUP_LUN_FROM_TAG
> 
> drivers/scsi/qla2xxx/qla_target.c      | 39 ++++++-----------------
> drivers/scsi/qla2xxx/tcm_qla2xxx.c     |  4 ++-
> drivers/target/target_core_device.c    | 14 ++++++---
> drivers/target/target_core_transport.c | 56 ++++++++++++++++++++++++++++------
> include/target/target_core_base.h      |  3 +-
> 5 files changed, 71 insertions(+), 45 deletions(-)
> 
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe target-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Series looks good. 

Acked-by: Himanshu Madhani <himanshu.madhani@cavium.com>

Thanks,
- Himanshu

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web