Skip to contentSkip to conversation

fix fc class work queue usage

PATCH New Mike Christie 1 participant 1 message Mar 26, 2005 SCSI

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 fix fc class work queue usage

According to this article http://lwn.net/Articles/125930/, "When cancel_delayed_work() returns zero, it means that the delayed work request was fired off before the call; it might, in fact, be running on another CPU when the cancel attempt is made". If it is successful, it returns a nonzero value. Tracing through cancel_delayed_work's timer usage would seem to confirm this. The fc class today though performs a flush_scheduled_work, when the return value is nonzero instead of zero.

Also it appears the fc class will use flush_scheduled_work to flush the work from the shost_work_q when it should be using flush_workqueue(shost->work_q) (flush_scheduled_work() only flushes the default, keventd_wq, work queue).

The attached patch adds a scsi_flush_work function for scsi_transport_fc to use and it fixes the cancel_delayed_work() test to detect when to flush the work queues correctly (it also only calls cancel_delayed_work when the work is queued as delayed (scan_work is not delayed). The patch has only been compile tested since I am away from any FC HW for a while.

drivers/scsi/hosts.c +17 -0
diff -aurp scsi-misc-2.6.orig/drivers/scsi/hosts.c scsi-misc-2.6.test/drivers/scsi/hosts.c
@@ -443,3 +443,20 @@ int scsi_queue_work(struct Scsi_Host *sh
 }
 EXPORT_SYMBOL_GPL(scsi_queue_work);
 
+/**
+ * scsi_flush_work - Flush a Scsi_Host's workqueue.
+ * @shost:	Pointer to Scsi_Host.
+ **/
+void scsi_flush_work(struct Scsi_Host *shost)
+{
+	if (!shost->work_q) {
+		printk(KERN_ERR
+			"ERROR: Scsi host '%s' attempted to flush scsi-work, "
+			"when no workqueue created.\n", shost->hostt->name);
+		dump_stack();
+		return;
+	}
+
+	flush_workqueue(shost->work_q);
+}
+EXPORT_SYMBOL_GPL(scsi_flush_work);
drivers/scsi/scsi_transport_fc.c +5 -3
diff -aurp scsi-misc-2.6.orig/drivers/scsi/scsi_transport_fc.c scsi-misc-2.6.test/drivers/scsi/scsi_transport_fc.c
@@ -1375,12 +1375,14 @@ EXPORT_SYMBOL(fc_remote_port_add);
 static void
 fc_rport_tgt_remove(struct fc_rport *rport)
 {
+	struct Scsi_Host *shost = rport_to_shost(rport);
+
 	scsi_target_unblock(&rport->dev);
 
 	/* Stop anything on the workq */
-	if (cancel_delayed_work(&rport->dev_loss_work) ||
-	    cancel_delayed_work(&rport->scan_work))
+	if (!cancel_delayed_work(&rport->dev_loss_work))
 		flush_scheduled_work();
+	scsi_flush_work(shost);
 
 	scsi_remove_target(&rport->dev);
 }
@@ -1625,7 +1627,7 @@ fc_remote_port_unblock(struct fc_rport *
 	 * failure as the state machine state change will validate the
 	 * transaction.
 	 */
-	if (cancel_delayed_work(work))
+	if (!cancel_delayed_work(work))
 		flush_scheduled_work();
 
 	if (rport->port_state == FC_PORTSTATE_OFFLINE)
include/scsi/scsi_host.h +1 -0
diff -aurp scsi-misc-2.6.orig/include/scsi/scsi_host.h scsi-misc-2.6.test/include/scsi/scsi_host.h
@@ -590,6 +590,7 @@ static inline struct Scsi_Host *dev_to_s
 }
 
 extern int scsi_queue_work(struct Scsi_Host *, struct work_struct *);
+extern void scsi_flush_work(struct Scsi_Host *);
 
 extern struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *, int);
 extern int __must_check scsi_add_host(struct Scsi_Host *, struct device *);