On 7/1/20 3:55 PM, Lee Duncan wrote:
> On 7/1/20 12:47 PM, Mike Christie wrote:
>> There is no need for one session to flush the entire
>> iscsi_eh_timer_workq when removing/unblocking a session. During removal
>> we need to make sure our works are not running anymore. And
>> iscsi_unblock_session only needs to make sure it's work is done. The
>> unblock work function will flush/cancel the works it has conflicts with.
>>
>> Signed-off-by: Mike Christie <michael.christie@xxxxxxxxxx>
>> ---
>> drivers/scsi/scsi_transport_iscsi.c | 15 +++++++--------
>> 1 file changed, 7 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c
>> index 2cd2610ecfaf..80b442a2b4c8 100644
>> --- a/drivers/scsi/scsi_transport_iscsi.c
>> +++ b/drivers/scsi/scsi_transport_iscsi.c
>> @@ -1978,10 +1978,11 @@ void iscsi_unblock_session(struct iscsi_cls_session *session)
>> {
>> queue_work(iscsi_eh_timer_workq, &session->unblock_work);
>> /*
>> - * make sure all the events have completed before tell the driver
>> - * it is safe
>> + * Blocking the session can be done from any context so we only
>> + * queue the block work. Make sure the unblock work has completed
>> + * because it flushes/cancels the other works and updates the state.
>> */
>> - flush_workqueue(iscsi_eh_timer_workq);
>> + flush_work(&session->unblock_work);
>> }
>> EXPORT_SYMBOL_GPL(iscsi_unblock_session);
>>
>> @@ -2205,11 +2206,9 @@ void iscsi_remove_session(struct iscsi_cls_session *session)
>> list_del(&session->sess_list);
>> spin_unlock_irqrestore(&sesslock, flags);
>>
>> - /* make sure there are no blocks/unblocks queued */
>> - flush_workqueue(iscsi_eh_timer_workq);
>> - /* make sure the timedout callout is not running */
>> - if (!cancel_delayed_work(&session->recovery_work))
>> - flush_workqueue(iscsi_eh_timer_workq);
>> + flush_work(&session->block_work);
>> + flush_work(&session->unblock_work);
>> + cancel_delayed_work_sync(&session->recovery_work);
>> /*
>> * If we are blocked let commands flow again. The lld or iscsi
>> * layer should set up the queuecommand to fail commands.
>>
>
> Is the error handler timer work queue still get flushed? (Sorry if this
> is a dumb question), or is that covered in the other flush()es?
No. We don't flush the entire workqueue anymore with this patch. Everyone flushes their specific work on the work queue.
On rmmod the destroy call does a flush, but there shouldn't be any thing on there.
