diff mbox

[v6,15/33] target: Avoid circular waits between LUN resets

Message ID 20170215002612.14566-16-bart.vanassche@sandisk.com (mailing list archive)
State Superseded
Headers show

Commit Message

Bart Van Assche Feb. 15, 2017, 12:25 a.m. UTC
If an initiator submits a LUN reset while a previous LUN reset is
still being processed, both LUN resets will be added to dev_tmr_list.
Avoid that this results in a circular wait between the two LUN resets
by removing a LUN reset from dev_tmr_list before scanning that list
for TMFs to wait for.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Fixes: c66ac9db8d4a ("target: Add LIO target core to kernel v2.6.38")
Cc: Hannes Reinecke <hare@suse.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: David Disseldorp <ddiss@suse.de>
Cc: <stable@vger.kernel.org>
---
 drivers/target/target_core_tmr.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

Comments

Nicholas A. Bellinger Feb. 20, 2017, 10:32 p.m. UTC | #1
On Tue, 2017-02-14 at 16:25 -0800, Bart Van Assche wrote:
> If an initiator submits a LUN reset while a previous LUN reset is
> still being processed, both LUN resets will be added to dev_tmr_list.
> Avoid that this results in a circular wait between the two LUN resets
> by removing a LUN reset from dev_tmr_list before scanning that list
> for TMFs to wait for.
> 
> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> Fixes: c66ac9db8d4a ("target: Add LIO target core to kernel v2.6.38")
> Cc: Hannes Reinecke <hare@suse.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: David Disseldorp <ddiss@suse.de>
> Cc: <stable@vger.kernel.org>
> ---
>  drivers/target/target_core_tmr.c | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/drivers/target/target_core_tmr.c b/drivers/target/target_core_tmr.c
> index 5eb164dac5cc..5c671fa67339 100644
> --- a/drivers/target/target_core_tmr.c
> +++ b/drivers/target/target_core_tmr.c
> @@ -230,13 +230,8 @@ static void core_tmr_drain_tmr_list(
>  	 * LUN_RESET tmr..
>  	 */
>  	spin_lock_irqsave(&dev->se_tmr_lock, flags);
> +	list_del_init(&tmr->tmr_list);
>  	list_for_each_entry_safe(tmr_p, tmr_pp, &dev->dev_tmr_list, tmr_list) {
> -		/*
> -		 * Allow the received TMR to return with FUNCTION_COMPLETE.
> -		 */
> -		if (tmr_p == tmr)
> -			continue;
> -
>  		cmd = tmr_p->task_cmd;
>  		if (!cmd) {
>  			pr_err("Unable to locate struct se_cmd for TMR\n");

As-is I don't think this is a stand-alone upstream bug-fix without patch
#7, that allows multiple target_tmr_work() execute concurrently.

That is because target_tmr_work() only executes one context at a time
since se_device->tmr_wq is using WQ_UNBOUND w/ max_active = 1, so this
is not an issue.

However, I think it's a reasonable improvement on it's own, so I'll
apply it to target-pending/for-next with a slightly different
description and without the stable CC.



--
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
diff mbox

Patch

diff --git a/drivers/target/target_core_tmr.c b/drivers/target/target_core_tmr.c
index 5eb164dac5cc..5c671fa67339 100644
--- a/drivers/target/target_core_tmr.c
+++ b/drivers/target/target_core_tmr.c
@@ -230,13 +230,8 @@  static void core_tmr_drain_tmr_list(
 	 * LUN_RESET tmr..
 	 */
 	spin_lock_irqsave(&dev->se_tmr_lock, flags);
+	list_del_init(&tmr->tmr_list);
 	list_for_each_entry_safe(tmr_p, tmr_pp, &dev->dev_tmr_list, tmr_list) {
-		/*
-		 * Allow the received TMR to return with FUNCTION_COMPLETE.
-		 */
-		if (tmr_p == tmr)
-			continue;
-
 		cmd = tmr_p->task_cmd;
 		if (!cmd) {
 			pr_err("Unable to locate struct se_cmd for TMR\n");