[PATCH] update the fc_transport_class to use a workqueue instead of a timeout

[PATCH] update the fc_transport_class to use a workqueue instead of a timeout

[James Bottomley]

The amount of work that has to be done in the timeout routines is really
a bit much if there's a large number of LUNS.  Plus the
device_for_each_child needs user context to get the bus semaphore, so
the solution is to migrate them from a timer to delayed work.

There's still a race here in that the timer may still be ticking when
the device is destroyed ... To fix this, I think we may need to
introduce a destroy callback to the transport class.

James

===== drivers/scsi/scsi_transport_fc.c 1.6 vs edited =====
--- 1.6/drivers/scsi/scsi_transport_fc.c	2004-10-11 10:03:45 -05:00
+++ edited/drivers/scsi/scsi_transport_fc.c	2004-10-26 17:12:02 -05:00
@@ -29,6 +29,8 @@
 
 static void transport_class_release(struct class_device *class_dev);
 static void host_class_release(struct class_device *class_dev);
+static void fc_timeout_blocked_host(void *data);
+static void fc_timeout_blocked_tgt(void *data);
 
 #define FC_STARGET_NUM_ATTRS 	4	/* increase this if you add attributes */
 #define FC_STARGET_OTHER_ATTRS 	0	/* increase this if you add "always on"
@@ -87,7 +89,8 @@
 	fc_starget_port_name(starget) = -1;
 	fc_starget_port_id(starget) = -1;
 	fc_starget_dev_loss_tmo(starget) = -1;
-	init_timer(&fc_starget_dev_loss_timer(starget));
+	INIT_WORK(&fc_starget_dev_loss_work(starget),
+		  fc_timeout_blocked_tgt, starget);
 	return 0;
 }
 
@@ -99,7 +102,8 @@
 	 * all transport attributes to valid values per host.
 	 */
 	fc_host_link_down_tmo(shost) = -1;
-	init_timer(&fc_host_link_down_timer(shost));
+	INIT_WORK(&fc_host_link_down_work(shost),
+		  fc_timeout_blocked_host, shost);
 	return 0;
 }
 
@@ -353,7 +357,7 @@
  *			 that fail to recover in the alloted time.
  * @data:	scsi target that failed to reappear in the alloted time.
  **/
-static void fc_timeout_blocked_tgt(unsigned long data)
+static void fc_timeout_blocked_tgt(void  *data)
 {
 	struct scsi_target *starget = (struct scsi_target *)data;
 
@@ -388,7 +392,7 @@
 fc_target_block(struct scsi_target *starget)
 {
 	int timeout = fc_starget_dev_loss_tmo(starget);
-	struct timer_list *timer = &fc_starget_dev_loss_timer(starget);
+	struct work_struct *work = &fc_starget_dev_loss_work(starget);
 
 	if (timeout < 0 || timeout > SCSI_DEVICE_BLOCK_MAX_TIMEOUT)
 		return -EINVAL;
@@ -396,10 +400,7 @@
 	device_for_each_child(&starget->dev, NULL, fc_device_block);
 
 	/* The scsi lld blocks this target for the timeout period only. */
-	timer->data = (unsigned long)starget;
-	timer->expires = jiffies + timeout * HZ;
-	timer->function = fc_timeout_blocked_tgt;
-	add_timer(timer);
+	schedule_delayed_work(work, timeout * HZ);
 
 	return 0;
 }
@@ -424,7 +425,8 @@
 	 * failure as the state machine state change will validate the
 	 * transaction. 
 	 */
-	del_timer_sync(&fc_starget_dev_loss_timer(starget));
+	if (cancel_delayed_work(&fc_starget_dev_loss_work(starget)))
+		flush_scheduled_work();
 
 	device_for_each_child(&starget->dev, NULL, fc_device_unblock);
 }
@@ -436,7 +438,7 @@
  * @data:	scsi host that failed to recover its devices in the alloted
  *		time.
  **/
-static void fc_timeout_blocked_host(unsigned long data)
+static void fc_timeout_blocked_host(void  *data)
 {
 	struct Scsi_Host *shost = (struct Scsi_Host *)data;
 	struct scsi_device *sdev;
@@ -475,7 +477,7 @@
 {
 	struct scsi_device *sdev;
 	int timeout = fc_host_link_down_tmo(shost);
-	struct timer_list *timer = &fc_host_link_down_timer(shost);
+	struct work_struct *work = &fc_host_link_down_work(shost);
 
 	if (timeout < 0 || timeout > SCSI_DEVICE_BLOCK_MAX_TIMEOUT)
 		return -EINVAL;
@@ -484,11 +486,7 @@
 		scsi_internal_device_block(sdev);
 	}
 
-	/* The scsi lld blocks this host for the timeout period only. */
-	timer->data = (unsigned long)shost;
-	timer->expires = jiffies + timeout * HZ;
-	timer->function = fc_timeout_blocked_host;
-	add_timer(timer);
+	schedule_delayed_work(work, timeout * HZ);
 
 	return 0;
 }
@@ -516,7 +514,9 @@
 	 * failure as the state machine state change will validate the
 	 * transaction.
 	 */
-	del_timer_sync(&fc_host_link_down_timer(shost));
+	if (cancel_delayed_work(&fc_host_link_down_work(shost)))
+		flush_scheduled_work();
+
 	shost_for_each_device(sdev, shost) {
 		scsi_internal_device_unblock(sdev);
 	}
===== include/scsi/scsi_transport_fc.h 1.5 vs edited =====
--- 1.5/include/scsi/scsi_transport_fc.h	2004-10-11 10:03:45 -05:00
+++ edited/include/scsi/scsi_transport_fc.h	2004-10-26 17:12:10 -05:00
@@ -29,7 +29,7 @@
 	uint64_t node_name;
 	uint64_t port_name;
 	uint32_t dev_loss_tmo;	/* Remote Port loss timeout in seconds. */
-	struct timer_list dev_loss_timer;
+	struct work_struct dev_loss_work;
 };
 
 #define fc_starget_port_id(x) \
@@ -40,18 +40,18 @@
 	(((struct fc_starget_attrs *)&(x)->starget_data)->port_name)
 #define fc_starget_dev_loss_tmo(x) \
 	(((struct fc_starget_attrs *)&(x)->starget_data)->dev_loss_tmo)
-#define fc_starget_dev_loss_timer(x) \
-	(((struct fc_starget_attrs *)&(x)->starget_data)->dev_loss_timer)
+#define fc_starget_dev_loss_work(x) \
+	(((struct fc_starget_attrs *)&(x)->starget_data)->dev_loss_work)
 
 struct fc_host_attrs {
 	uint32_t link_down_tmo;	/* Link Down timeout in seconds. */
-	struct timer_list link_down_timer;
+	struct work_struct link_down_work;
 };
 
 #define fc_host_link_down_tmo(x) \
 	(((struct fc_host_attrs *)(x)->shost_data)->link_down_tmo)
-#define fc_host_link_down_timer(x) \
-	(((struct fc_host_attrs *)(x)->shost_data)->link_down_timer)
+#define fc_host_link_down_work(x) \
+	(((struct fc_host_attrs *)(x)->shost_data)->link_down_work)
 

 /* The functions by which the transport class and the driver communicate */