aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorRoland Dreier <roland@purestorage.com>2011-08-28 00:33:16 -0400
committerNicholas A. Bellinger <nab@linux-iscsi.org>2011-09-16 05:29:20 -0400
commitbcac364a24c894c4cf8cf219b7863c192cd34079 (patch)
tree121757157452bf6e546c53c0efd2a3d463f4aa2e
parenta7f934d4f16144cb9521b62e9b8c9ac0118097da (diff)
target: Fix race between multiple invocations of target_qf_do_work()
When work is scheduled with schedule_work(), the work can end up running on multiple CPUs at the same time -- this happens if the work is already running on one CPU and schedule_work() is called on another CPU. This leads to list corruption with target_qf_do_work(), which is roughly doing: spin_lock(...); list_for_each_entry_safe(...) { list_del(...); spin_unlock(...); // do stuff spin_lock(...); } With multiple CPUs running this code, one CPU can end up deleting the list entry that the other CPU is about to work on. Fix this by splicing the list entries onto a local list and then operating on that in the work function. This way, each invocation of target_qf_do_work() operates on its own local list and so multiple invocations don't corrupt each other's list. This also avoids dropping and reacquiring the lock for each list entry. Signed-off-by: Roland Dreier <roland@purestorage.com> Signed-off-by: Nicholas Bellinger <nab@linux-iscsi.org>
-rw-r--r--drivers/target/target_core_transport.c9
1 files changed, 4 insertions, 5 deletions
diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
index 8d0c58ea6316..a4b0a8d27f25 100644
--- a/drivers/target/target_core_transport.c
+++ b/drivers/target/target_core_transport.c
@@ -977,15 +977,17 @@ static void target_qf_do_work(struct work_struct *work)
977{ 977{
978 struct se_device *dev = container_of(work, struct se_device, 978 struct se_device *dev = container_of(work, struct se_device,
979 qf_work_queue); 979 qf_work_queue);
980 LIST_HEAD(qf_cmd_list);
980 struct se_cmd *cmd, *cmd_tmp; 981 struct se_cmd *cmd, *cmd_tmp;
981 982
982 spin_lock_irq(&dev->qf_cmd_lock); 983 spin_lock_irq(&dev->qf_cmd_lock);
983 list_for_each_entry_safe(cmd, cmd_tmp, &dev->qf_cmd_list, se_qf_node) { 984 list_splice_init(&dev->qf_cmd_list, &qf_cmd_list);
985 spin_unlock_irq(&dev->qf_cmd_lock);
984 986
987 list_for_each_entry_safe(cmd, cmd_tmp, &qf_cmd_list, se_qf_node) {
985 list_del(&cmd->se_qf_node); 988 list_del(&cmd->se_qf_node);
986 atomic_dec(&dev->dev_qf_count); 989 atomic_dec(&dev->dev_qf_count);
987 smp_mb__after_atomic_dec(); 990 smp_mb__after_atomic_dec();
988 spin_unlock_irq(&dev->qf_cmd_lock);
989 991
990 pr_debug("Processing %s cmd: %p QUEUE_FULL in work queue" 992 pr_debug("Processing %s cmd: %p QUEUE_FULL in work queue"
991 " context: %s\n", cmd->se_tfo->get_fabric_name(), cmd, 993 " context: %s\n", cmd->se_tfo->get_fabric_name(), cmd,
@@ -997,10 +999,7 @@ static void target_qf_do_work(struct work_struct *work)
997 * has been added to head of queue 999 * has been added to head of queue
998 */ 1000 */
999 transport_add_cmd_to_queue(cmd, cmd->t_state); 1001 transport_add_cmd_to_queue(cmd, cmd->t_state);
1000
1001 spin_lock_irq(&dev->qf_cmd_lock);
1002 } 1002 }
1003 spin_unlock_irq(&dev->qf_cmd_lock);
1004} 1003}
1005 1004
1006unsigned char *transport_dump_cmd_direction(struct se_cmd *cmd) 1005unsigned char *transport_dump_cmd_direction(struct se_cmd *cmd)