On 2022/4/14 23:22, Mike Christie wrote:
> On 4/13/22 8:49 PM, Wenchao Hao wrote:
>> Fix the issue of kernel send multiple ISCSI_KEVENT_UNBIND_SESSION event.
>> If session is in UNBOUND state, do not perform unbind operations anymore,
>> else unbind session and set session to UNBOUND state.
>>
> 
> I don't think we want this to be a state because you can have a session
> with no target or it could be partially deleted and it could be in the
> logged in or failed state. If scsi-ml is sending SYNC_CACHEs as part of
> the target/device removal operation, and we lose the session then we could
> go through recovery and the state will go from failed to logged in, and
> your new unbound state will have been overwritten.
> 
> I think it might be better to have a new sysfs file, target_state, for
> this where you could have values like scanning, bound, unbinding, and
> unbound, or just a sysfs file, target_bound, that is bool.
> 

Thanks for your review, I would modify and send another patch.

>> Reference:https://github.com/open-iscsi/open-iscsi/issues/338
>>
> 
> You should add a description of the problem in the commit, because that
> link might be gone one day.
> 
> 
>> Signed-off-by: Wenchao Hao <haowenc...@huawei.com>
>> ---
>>  drivers/scsi/scsi_transport_iscsi.c | 19 +++++++++++++++++--
>>  include/scsi/scsi_transport_iscsi.h |  1 +
>>  2 files changed, 18 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/scsi/scsi_transport_iscsi.c 
>> b/drivers/scsi/scsi_transport_iscsi.c
>> index 27951ea05dd4..97a9fee02efa 100644
>> --- a/drivers/scsi/scsi_transport_iscsi.c
>> +++ b/drivers/scsi/scsi_transport_iscsi.c
>> @@ -1656,6 +1656,7 @@ static struct {
>>      { ISCSI_SESSION_LOGGED_IN,      "LOGGED_IN" },
>>      { ISCSI_SESSION_FAILED,         "FAILED" },
>>      { ISCSI_SESSION_FREE,           "FREE" },
>> +    { ISCSI_SESSION_UNBOUND,        "UNBOUND" },
>>  };
>>  
>>  static const char *iscsi_session_state_name(int state)
>> @@ -1686,6 +1687,9 @@ int iscsi_session_chkready(struct iscsi_cls_session 
>> *session)
>>      case ISCSI_SESSION_FREE:
>>              err = DID_TRANSPORT_FAILFAST << 16;
>>              break;
>> +    case ISCSI_SESSION_UNBOUND:
>> +            err = DID_NO_CONNECT << 16;
>> +            break;
>>      default:
>>              err = DID_NO_CONNECT << 16;
>>              break;
>> @@ -1838,7 +1842,8 @@ int iscsi_block_scsi_eh(struct scsi_cmnd *cmd)
>>  
>>      spin_lock_irqsave(&session->lock, flags);
>>      while (session->state != ISCSI_SESSION_LOGGED_IN) {
>> -            if (session->state == ISCSI_SESSION_FREE) {
>> +            if ((session->state == ISCSI_SESSION_FREE) ||
>> +                (session->state == ISCSI_SESSION_UNBOUND)) {
>>                      ret = FAST_IO_FAIL;
>>                      break;
>>              }
>> @@ -1869,6 +1874,7 @@ static void session_recovery_timedout(struct 
>> work_struct *work)
>>              break;
>>      case ISCSI_SESSION_LOGGED_IN:
>>      case ISCSI_SESSION_FREE:
>> +    case ISCSI_SESSION_UNBOUND:
>>              /* we raced with the unblock's flush */
>>              spin_unlock_irqrestore(&session->lock, flags);
>>              return;
>> @@ -1957,6 +1963,14 @@ static void __iscsi_unbind_session(struct work_struct 
>> *work)
>>      unsigned long flags;
>>      unsigned int target_id;
>>  
>> +    spin_lock_irqsave(&session->lock, flags);
>> +    if (session->state == ISCSI_SESSION_UNBOUND) {
>> +            spin_unlock_irqrestore(&session->lock, flags);
>> +            return;
>> +    }
>> +    session->state = ISCSI_SESSION_UNBOUND;
>> +    spin_unlock_irqrestore(&session->lock, flags);
>> +
>>      ISCSI_DBG_TRANS_SESSION(session, "Unbinding session\n");
>>  
>>      /* Prevent new scans and make sure scanning is not in progress */
>> @@ -4329,7 +4343,8 @@ store_priv_session_##field(struct device *dev,         
>>                 \
>>      struct iscsi_cls_session *session =                             \
>>              iscsi_dev_to_session(dev->parent);                      \
>>      if ((session->state == ISCSI_SESSION_FREE) ||                   \
>> -        (session->state == ISCSI_SESSION_FAILED))                   \
>> +        (session->state == ISCSI_SESSION_FAILED) ||                 \
>> +        (session->state == ISCSI_SESSION_UNBOUND))                  \
>>              return -EBUSY;                                          \
>>      if (strncmp(buf, "off", 3) == 0) {                              \
>>              session->field = -1;                                    \
>> diff --git a/include/scsi/scsi_transport_iscsi.h 
>> b/include/scsi/scsi_transport_iscsi.h
>> index 38e4a67f5922..80149643cbcd 100644
>> --- a/include/scsi/scsi_transport_iscsi.h
>> +++ b/include/scsi/scsi_transport_iscsi.h
>> @@ -232,6 +232,7 @@ enum {
>>      ISCSI_SESSION_LOGGED_IN,
>>      ISCSI_SESSION_FAILED,
>>      ISCSI_SESSION_FREE,
>> +    ISCSI_SESSION_UNBOUND,
>>  };
>>  
>>  #define ISCSI_MAX_TARGET -1
> 
> .

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to open-iscsi+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/open-iscsi/1e4b47e5-2bd3-2766-7d47-d14d3a4fbd65%40huawei.com.

Reply via email to