target: Invoke release_cmd() callback without holding a spinlock

Bart Van Assche · Nicholas Bellinger · commit 9ff9d15eddd1 · 2015-11-28T19:33:23.000-08:00
This patch fixes the following kernel warning because it avoids that
IRQs are disabled while ft_release_cmd() is invoked (fc_seq_set_resp()
invokes spin_unlock_bh()):

WARNING: CPU: 3 PID: 117 at kernel/softirq.c:150 __local_bh_enable_ip+0xaa/0x110()
Call Trace:
 [&lt;ffffffff814f71eb&gt;] dump_stack+0x4f/0x7b
 [&lt;ffffffff8105e56a&gt;] warn_slowpath_common+0x8a/0xc0
 [&lt;ffffffff8105e65a&gt;] warn_slowpath_null+0x1a/0x20
 [&lt;ffffffff81062b2a&gt;] __local_bh_enable_ip+0xaa/0x110
 [&lt;ffffffff814ff229&gt;] _raw_spin_unlock_bh+0x39/0x40
 [&lt;ffffffffa03a7f94&gt;] fc_seq_set_resp+0xe4/0x100 [libfc]
 [&lt;ffffffffa02e604a&gt;] ft_free_cmd+0x4a/0x90 [tcm_fc]
 [&lt;ffffffffa02e6972&gt;] ft_release_cmd+0x12/0x20 [tcm_fc]
 [&lt;ffffffffa042bd66&gt;] target_release_cmd_kref+0x56/0x90 [target_core_mod]
 [&lt;ffffffffa042caf0&gt;] target_put_sess_cmd+0xc0/0x110 [target_core_mod]
 [&lt;ffffffffa042cb81&gt;] transport_release_cmd+0x41/0x70 [target_core_mod]
 [&lt;ffffffffa042d975&gt;] transport_generic_free_cmd+0x35/0x420 [target_core_mod]

Signed-off-by: Bart Van Assche &lt;bart.vanassche@sandisk.com&gt;
Acked-by: Joern Engel &lt;joern@logfs.org&gt;
Reviewed-by: Andy Grover &lt;agrover@redhat.com&gt;
Cc: Christoph Hellwig &lt;hch@lst.de&gt;
Cc: Hannes Reinecke &lt;hare@suse.de&gt;
Cc: Sagi Grimberg &lt;sagig@mellanox.com&gt;
Signed-off-by: Nicholas Bellinger &lt;nab@linux-iscsi.org&gt;
diff --git a/drivers/target/target_core_tmr.c b/drivers/target/target_core_tmr.c
@@ -130,6 +130,9 @@ void core_tmr_abort_task(
 		if (tmr->ref_task_tag != ref_tag)
 			continue;
 
+		if (!kref_get_unless_zero(&se_cmd->cmd_kref))
+			continue;
+
 		printk("ABORT_TASK: Found referenced %s task_tag: %llu\n",
 			se_cmd->se_tfo->get_fabric_name(), ref_tag);
 
@@ -139,13 +142,15 @@ void core_tmr_abort_task(
 			       " skipping\n", ref_tag);
 			spin_unlock(&se_cmd->t_state_lock);
 			spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags);
+
+			target_put_sess_cmd(se_cmd);
+
 			goto out;
 		}
 		se_cmd->transport_state |= CMD_T_ABORTED;
 		spin_unlock(&se_cmd->t_state_lock);
 
 		list_del_init(&se_cmd->se_cmd_list);
-		kref_get(&se_cmd->cmd_kref);
 		spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags);
 
 		cancel_work_sync(&se_cmd->work);
diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
@@ -2509,23 +2509,24 @@ int target_get_sess_cmd(struct se_cmd *se_cmd, bool ack_kref)
 EXPORT_SYMBOL(target_get_sess_cmd);
 
 static void target_release_cmd_kref(struct kref *kref)
-		__releases(&se_cmd->se_sess->sess_cmd_lock)
 {
 	struct se_cmd *se_cmd = container_of(kref, struct se_cmd, cmd_kref);
 	struct se_session *se_sess = se_cmd->se_sess;
+	unsigned long flags;
 
+	spin_lock_irqsave(&se_sess->sess_cmd_lock, flags);
 	if (list_empty(&se_cmd->se_cmd_list)) {
-		spin_unlock(&se_sess->sess_cmd_lock);
+		spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags);
 		se_cmd->se_tfo->release_cmd(se_cmd);
 		return;
 	}
 	if (se_sess->sess_tearing_down && se_cmd->cmd_wait_set) {
-		spin_unlock(&se_sess->sess_cmd_lock);
+		spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags);
 		complete(&se_cmd->cmd_wait_comp);
 		return;
 	}
 	list_del(&se_cmd->se_cmd_list);
-	spin_unlock(&se_sess->sess_cmd_lock);
+	spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags);
 
 	se_cmd->se_tfo->release_cmd(se_cmd);
 }
@@ -2541,8 +2542,7 @@ int target_put_sess_cmd(struct se_cmd *se_cmd)
 		se_cmd->se_tfo->release_cmd(se_cmd);
 		return 1;
 	}
-	return kref_put_spinlock_irqsave(&se_cmd->cmd_kref, target_release_cmd_kref,
-			&se_sess->sess_cmd_lock);
+	return kref_put(&se_cmd->cmd_kref, target_release_cmd_kref);
 }
 EXPORT_SYMBOL(target_put_sess_cmd);
 

Original file line number	Diff line number	Diff line change
`@@ -2509,23 +2509,24 @@ int target_get_sess_cmd(struct se_cmd *se_cmd, bool ack_kref)`
`2509`	`2509`	`EXPORT_SYMBOL(target_get_sess_cmd);`
`2510`	`2510`
`2511`	`2511`	`static void target_release_cmd_kref(struct kref *kref)`
`2512`		`- __releases(&se_cmd->se_sess->sess_cmd_lock)`
`2513`	`2512`	`{`
`2514`	`2513`	`struct se_cmd *se_cmd = container_of(kref, struct se_cmd, cmd_kref);`
`2515`	`2514`	`struct se_session *se_sess = se_cmd->se_sess;`
	`2515`	`+ unsigned long flags;`
`2516`	`2516`
	`2517`	`+ spin_lock_irqsave(&se_sess->sess_cmd_lock, flags);`
`2517`	`2518`	`if (list_empty(&se_cmd->se_cmd_list)) {`
`2518`		`- spin_unlock(&se_sess->sess_cmd_lock);`
	`2519`	`+ spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags);`
`2519`	`2520`	`se_cmd->se_tfo->release_cmd(se_cmd);`
`2520`	`2521`	`return;`
`2521`	`2522`	`}`
`2522`	`2523`	`if (se_sess->sess_tearing_down && se_cmd->cmd_wait_set) {`
`2523`		`- spin_unlock(&se_sess->sess_cmd_lock);`
	`2524`	`+ spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags);`
`2524`	`2525`	`complete(&se_cmd->cmd_wait_comp);`
`2525`	`2526`	`return;`
`2526`	`2527`	`}`
`2527`	`2528`	`list_del(&se_cmd->se_cmd_list);`
`2528`		`- spin_unlock(&se_sess->sess_cmd_lock);`
	`2529`	`+ spin_unlock_irqrestore(&se_sess->sess_cmd_lock, flags);`
`2529`	`2530`
`2530`	`2531`	`se_cmd->se_tfo->release_cmd(se_cmd);`
`2531`	`2532`	`}`
`@@ -2541,8 +2542,7 @@ int target_put_sess_cmd(struct se_cmd *se_cmd)`
`2541`	`2542`	`se_cmd->se_tfo->release_cmd(se_cmd);`
`2542`	`2543`	`return 1;`
`2543`	`2544`	`}`
`2544`		`- return kref_put_spinlock_irqsave(&se_cmd->cmd_kref, target_release_cmd_kref,`
`2545`		`- &se_sess->sess_cmd_lock);`
	`2545`	`+ return kref_put(&se_cmd->cmd_kref, target_release_cmd_kref);`
`2546`	`2546`	`}`
`2547`	`2547`	`EXPORT_SYMBOL(target_put_sess_cmd);`
`2548`	`2548`