From: Bart Van Assche <Bart.VanAssche@sandisk.com>
To: "target-devel@vger.kernel.org" <target-devel@vger.kernel.org>,
        "nab@linux-iscsi.org" <nab@linux-iscsi.org>
CC: "linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "mchristi@redhat.com" <mchristi@redhat.com>,
        "himanshu.madhani@cavium.com" <himanshu.madhani@cavium.com>,
        "quinn.tran@cavium.com" <quinn.tran@cavium.com>,
        "hare@suse.com" <hare@suse.com>, "hch@lst.de" <hch@lst.de>
Subject: Re: [PATCH 2/3] target: Add TARGET_SCF_LOOKUP_LUN_FROM_TAG support
 for ABORT_TASK
Thread-Topic: [PATCH 2/3] target: Add TARGET_SCF_LOOKUP_LUN_FROM_TAG support
 for ABORT_TASK
Thread-Index: AQHS3LXGqsnyunHB1EO8NM1JQGhLhKIWb24A
Date: Mon, 5 Jun 2017 15:57:30 +0000
Message-ID: <1496678246.2623.5.camel@sandisk.com>
References: <1496527808-28789-1-git-send-email-nab@linux-iscsi.org>
         <1496527808-28789-3-git-send-email-nab@linux-iscsi.org>
In-Reply-To: <1496527808-28789-3-git-send-email-nab@linux-iscsi.org>
Accept-Language: en-US
Content-Language: en-US
wdcipoutbound: EOP-TRUE
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
Content-Type: text/plain; charset="iso-8859-1"
Content-ID: <062FB2E66570AE488DA94364C5B48DDE@namprd04.prod.outlook.com>
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-originalarrivaltime: 05 Jun 2017 15:57:30.0800
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: b61c8803-16f3-4c35-9b17-6f65f441df86
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY1PR0401MB1535
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Transfer-Encoding: 8bit
X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id v55Fvi2Q002066
Content-Length: 2219
Lines: 67

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.