| Message ID | 20220226230435.38733-2-michael.christie@oracle.com |
|---|---|
| State | New |
| Headers | show |
| Series | iscsi: Speed up failover with lots of devices. | expand |
On 2/26/22 15:04, Mike Christie wrote: > If the user sets the iscsi_eh_timer_workq/iscsi_eh workqueue's max_active > to greater than 1, the recovery_work could be running when > __iscsi_unblock_session runs. The cancel_delayed_work will then not wait > for the running work and we can race where we end up with the wrong > session state and scsi_device state set. > > This replaces the cancel_delayed_work with the sync version. > > Signed-off-by: Mike Christie <michael.christie@oracle.com> > --- > drivers/scsi/scsi_transport_iscsi.c | 7 ++----- > 1 file changed, 2 insertions(+), 5 deletions(-) > > diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c > index 554b6f784223..c58126e8cd88 100644 > --- a/drivers/scsi/scsi_transport_iscsi.c > +++ b/drivers/scsi/scsi_transport_iscsi.c > @@ -1917,11 +1917,8 @@ static void __iscsi_unblock_session(struct work_struct *work) > unsigned long flags; > > ISCSI_DBG_TRANS_SESSION(session, "Unblocking session\n"); > - /* > - * The recovery and unblock work get run from the same workqueue, > - * so try to cancel it if it was going to run after this unblock. > - */ > - cancel_delayed_work(&session->recovery_work); > + > + cancel_delayed_work_sync(&session->recovery_work); > spin_lock_irqsave(&session->lock, flags); > session->state = ISCSI_SESSION_LOGGED_IN; > spin_unlock_irqrestore(&session->lock, flags); Reviewed-by: Lee Duncan <lduncan@suse.com>
On 2/26/22 3:04 PM, Mike Christie wrote: > If the user sets the iscsi_eh_timer_workq/iscsi_eh workqueue's max_active > to greater than 1, the recovery_work could be running when > __iscsi_unblock_session runs. The cancel_delayed_work will then not wait > for the running work and we can race where we end up with the wrong > session state and scsi_device state set. > > This replaces the cancel_delayed_work with the sync version. > > Signed-off-by: Mike Christie <michael.christie@oracle.com> > --- > drivers/scsi/scsi_transport_iscsi.c | 7 ++----- > 1 file changed, 2 insertions(+), 5 deletions(-) > > diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c > index 554b6f784223..c58126e8cd88 100644 > --- a/drivers/scsi/scsi_transport_iscsi.c > +++ b/drivers/scsi/scsi_transport_iscsi.c > @@ -1917,11 +1917,8 @@ static void __iscsi_unblock_session(struct work_struct *work) > unsigned long flags; > > ISCSI_DBG_TRANS_SESSION(session, "Unblocking session\n"); > - /* > - * The recovery and unblock work get run from the same workqueue, > - * so try to cancel it if it was going to run after this unblock. > - */ > - cancel_delayed_work(&session->recovery_work); > + > + cancel_delayed_work_sync(&session->recovery_work); > spin_lock_irqsave(&session->lock, flags); > session->state = ISCSI_SESSION_LOGGED_IN; > spin_unlock_irqrestore(&session->lock, flags); Looks good to me, Reviewed-by: Chris Leech <cleech@redhat.com>
diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c index 554b6f784223..c58126e8cd88 100644 --- a/drivers/scsi/scsi_transport_iscsi.c +++ b/drivers/scsi/scsi_transport_iscsi.c @@ -1917,11 +1917,8 @@ static void __iscsi_unblock_session(struct work_struct *work) unsigned long flags; ISCSI_DBG_TRANS_SESSION(session, "Unblocking session\n"); - /* - * The recovery and unblock work get run from the same workqueue, - * so try to cancel it if it was going to run after this unblock. - */ - cancel_delayed_work(&session->recovery_work); + + cancel_delayed_work_sync(&session->recovery_work); spin_lock_irqsave(&session->lock, flags); session->state = ISCSI_SESSION_LOGGED_IN; spin_unlock_irqrestore(&session->lock, flags);
If the user sets the iscsi_eh_timer_workq/iscsi_eh workqueue's max_active to greater than 1, the recovery_work could be running when __iscsi_unblock_session runs. The cancel_delayed_work will then not wait for the running work and we can race where we end up with the wrong session state and scsi_device state set. This replaces the cancel_delayed_work with the sync version. Signed-off-by: Mike Christie <michael.christie@oracle.com> --- drivers/scsi/scsi_transport_iscsi.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)