lkml.org 
[lkml]   [2022]   [Apr]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH 2/2] iscsi: set session to FREE state after unbind session in remove session
From
On 2022/4/14 23:30, Mike Christie wrote:
> On 4/13/22 8:49 PM, Wenchao Hao wrote:
>> __iscsi_unbind_session() set session state to ISCSI_SESSION_UNBOUND, which
>> would overwrite the ISCSI_SESSION_FREE state.
>>
>> Signed-off-by: Wenchao Hao <haowenchao@huawei.com>
>> ---
>> drivers/scsi/scsi_transport_iscsi.c | 26 ++++++++++++++++----------
>> 1 file changed, 16 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c
>> index 97a9fee02efa..d8dd9279cea8 100644
>> --- a/drivers/scsi/scsi_transport_iscsi.c
>> +++ b/drivers/scsi/scsi_transport_iscsi.c
>> @@ -2173,6 +2173,22 @@ void iscsi_remove_session(struct iscsi_cls_session *session)
>> if (!cancel_work_sync(&session->block_work))
>> cancel_delayed_work_sync(&session->recovery_work);
>> cancel_work_sync(&session->unblock_work);
>> +
>> + scsi_target_unblock(&session->dev, SDEV_TRANSPORT_OFFLINE);
>> + /*
>> + * qla4xxx can perform it's own scans when it runs in kernel only
>> + * mode. Make sure to flush those scans.
>> + */
>> + flush_work(&session->scan_work);
>> +
>> + /*
>> + * flush running unbind operations
>> + * if unbind work did not queued, call __iscsi_unbind_session
>> + * directly to perform target remove
>
> We probably don't need the flush_work test because we are going to
> normally call __iscsi_unbind_session.
>

I think we still need calling flush_work here. The introduce of flush_work
is to make sure sysfs objects are removed in an correct order. There is a
very low probability that __iscsi_unbind_session() triggered by queue_work()
has not been finished, and iscsi_remove_session() is called. So we need
flush_work() to make sure __iscsi_unbind_session() has done if it has been
activated by queue_work().

> If the unbind work had already run, which is the normal case, then
> flush_work returns false and we end up calling __iscsi_unbind_session
> like before. That function then checks if the target is really unbound.
> So the extra check doesn't normally buy us anything with your patches
> because in patch 1 you fixed it so __iscsi_unbind_session doesn't send
> the extra event.
>
>
>> + */
>> + if (!flush_work(&session->unbind_work))
>> + __iscsi_unbind_session(&session->unbind_work);
>> +
>> /*
>> * If we are blocked let commands flow again. The lld or iscsi
>> * layer should set up the queuecommand to fail commands.
>> @@ -2183,16 +2199,6 @@ void iscsi_remove_session(struct iscsi_cls_session *session)
>> session->state = ISCSI_SESSION_FREE;
>> spin_unlock_irqrestore(&session->lock, flags);
>>
>> - scsi_target_unblock(&session->dev, SDEV_TRANSPORT_OFFLINE);
>> - /*
>> - * qla4xxx can perform it's own scans when it runs in kernel only
>> - * mode. Make sure to flush those scans.
>> - */
>> - flush_work(&session->scan_work);
>> - /* flush running unbind operations */
>> - flush_work(&session->unbind_work);
>> - __iscsi_unbind_session(&session->unbind_work);
>> -
>> /* hw iscsi may not have removed all connections from session */
>> err = device_for_each_child(&session->dev, NULL,
>> iscsi_iter_destroy_conn_fn);
>
> .

\
 
 \ /
  Last update: 2022-04-15 11:41    [W:0.060 / U:0.216 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site