update the fc_transport_class to use a workqueue instead of a timeout
PATCH 1/1 update the fc_transport_class to use a workqueue instead of a timeout
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 =====
drivers/scsi/scsi_transport_fc.c +17 -17
@@ -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 =====
include/scsi/scsi_transport_fc.h +6 -6
@@ -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 */