Skip to content

Commit

Permalink
scsi: iscsi: Fix multiple iSCSI session unbind events sent to userspace
Browse files Browse the repository at this point in the history 
It was observed that the kernel would potentially send
ISCSI_KEVENT_UNBIND_SESSION multiple times. Introduce 'target_state' in
iscsi_cls_session() to make sure session will send only one unbind session
event.

This introduces a regression wrt. the issue fixed in commit 13e60d3
("scsi: iscsi: Report unbind session event when the target has been
removed"). If iscsid dies for any reason after sending an unbind session to
kernel, once iscsid is restarted, the kernel's ISCSI_KEVENT_UNBIND_SESSION
event is lost and userspace is then unable to logout. However, the session
is actually in invalid state (its target_id is INVALID) so iscsid should
not sync this session during restart.

Consequently we need to check the session's target state during iscsid
restart.  If session is in unbound state, do not sync this session and
perform session teardown. This is OK because once a session is unbound, we
can not recover it any more (mainly because its target id is INVALID).

Signed-off-by: Wenchao Hao <haowenchao@huawei.com>
Link: https://lore.kernel.org/r/20221126010752.231917-1-haowenchao@huawei.com
Reviewed-by: Mike Christie <michael.christie@oracle.com>
Reviewed-by: Wu Bo <wubo40@huawei.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
  • Loading branch information
Wenchao Hao authored and Martin K. Petersen committed Dec 14, 2022
1 parent 68ad831 commit a3be19b
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 5 deletions.
50 changes: 45 additions & 5 deletions drivers/scsi/scsi_transport_iscsi.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1676,6 +1676,13 @@ static const char *iscsi_session_state_name(int state)
return name;
}

static char *iscsi_session_target_state_name[] = {
[ISCSI_SESSION_TARGET_UNBOUND] = "UNBOUND",
[ISCSI_SESSION_TARGET_ALLOCATED] = "ALLOCATED",
[ISCSI_SESSION_TARGET_SCANNED] = "SCANNED",
[ISCSI_SESSION_TARGET_UNBINDING] = "UNBINDING",
};

int iscsi_session_chkready(struct iscsi_cls_session *session)
{
int err;
Expand Down Expand Up @@ -1785,9 +1792,13 @@ static int iscsi_user_scan_session(struct device *dev, void *data)
if ((scan_data->channel == SCAN_WILD_CARD ||
scan_data->channel == 0) &&
(scan_data->id == SCAN_WILD_CARD ||
scan_data->id == id))
scan_data->id == id)) {
scsi_scan_target(&session->dev, 0, id,
scan_data->lun, scan_data->rescan);
spin_lock_irqsave(&session->lock, flags);
session->target_state = ISCSI_SESSION_TARGET_SCANNED;
spin_unlock_irqrestore(&session->lock, flags);
}
}

user_scan_exit:
Expand Down Expand Up @@ -1960,31 +1971,41 @@ static void __iscsi_unbind_session(struct work_struct *work)
struct iscsi_cls_host *ihost = shost->shost_data;
unsigned long flags;
unsigned int target_id;
bool remove_target = true;

ISCSI_DBG_TRANS_SESSION(session, "Unbinding session\n");

/* Prevent new scans and make sure scanning is not in progress */
mutex_lock(&ihost->mutex);
spin_lock_irqsave(&session->lock, flags);
if (session->target_id == ISCSI_MAX_TARGET) {
if (session->target_state == ISCSI_SESSION_TARGET_ALLOCATED) {
remove_target = false;
} else if (session->target_state != ISCSI_SESSION_TARGET_SCANNED) {
spin_unlock_irqrestore(&session->lock, flags);
mutex_unlock(&ihost->mutex);
goto unbind_session_exit;
ISCSI_DBG_TRANS_SESSION(session,
"Skipping target unbinding: Session is unbound/unbinding.\n");
return;
}

session->target_state = ISCSI_SESSION_TARGET_UNBINDING;
target_id = session->target_id;
session->target_id = ISCSI_MAX_TARGET;
spin_unlock_irqrestore(&session->lock, flags);
mutex_unlock(&ihost->mutex);

scsi_remove_target(&session->dev);
if (remove_target)
scsi_remove_target(&session->dev);

if (session->ida_used)
ida_free(&iscsi_sess_ida, target_id);

unbind_session_exit:
iscsi_session_event(session, ISCSI_KEVENT_UNBIND_SESSION);
ISCSI_DBG_TRANS_SESSION(session, "Completed target removal\n");

spin_lock_irqsave(&session->lock, flags);
session->target_state = ISCSI_SESSION_TARGET_UNBOUND;
spin_unlock_irqrestore(&session->lock, flags);
}

static void __iscsi_destroy_session(struct work_struct *work)
Expand Down Expand Up @@ -2061,6 +2082,9 @@ int iscsi_add_session(struct iscsi_cls_session *session, unsigned int target_id)
session->ida_used = true;
} else
session->target_id = target_id;
spin_lock_irqsave(&session->lock, flags);
session->target_state = ISCSI_SESSION_TARGET_ALLOCATED;
spin_unlock_irqrestore(&session->lock, flags);

dev_set_name(&session->dev, "session%u", session->sid);
err = device_add(&session->dev);
Expand Down Expand Up @@ -4368,6 +4392,19 @@ iscsi_session_attr(def_taskmgmt_tmo, ISCSI_PARAM_DEF_TASKMGMT_TMO, 0);
iscsi_session_attr(discovery_parent_idx, ISCSI_PARAM_DISCOVERY_PARENT_IDX, 0);
iscsi_session_attr(discovery_parent_type, ISCSI_PARAM_DISCOVERY_PARENT_TYPE, 0);

static ssize_t
show_priv_session_target_state(struct device *dev, struct device_attribute *attr,
char *buf)
{
struct iscsi_cls_session *session = iscsi_dev_to_session(dev->parent);

return sysfs_emit(buf, "%s\n",
iscsi_session_target_state_name[session->target_state]);
}

static ISCSI_CLASS_ATTR(priv_sess, target_state, S_IRUGO,
show_priv_session_target_state, NULL);

static ssize_t
show_priv_session_state(struct device *dev, struct device_attribute *attr,
char *buf)
Expand Down Expand Up @@ -4470,6 +4507,7 @@ static struct attribute *iscsi_session_attrs[] = {
&dev_attr_sess_boot_target.attr,
&dev_attr_priv_sess_recovery_tmo.attr,
&dev_attr_priv_sess_state.attr,
&dev_attr_priv_sess_target_state.attr,
&dev_attr_priv_sess_creator.attr,
&dev_attr_sess_chap_out_idx.attr,
&dev_attr_sess_chap_in_idx.attr,
Expand Down Expand Up @@ -4583,6 +4621,8 @@ static umode_t iscsi_session_attr_is_visible(struct kobject *kobj,
return S_IRUGO | S_IWUSR;
else if (attr == &dev_attr_priv_sess_state.attr)
return S_IRUGO;
else if (attr == &dev_attr_priv_sess_target_state.attr)
return S_IRUGO;
else if (attr == &dev_attr_priv_sess_creator.attr)
return S_IRUGO;
else if (attr == &dev_attr_priv_sess_target_id.attr)
Expand Down
9 changes: 9 additions & 0 deletions include/scsi/scsi_transport_iscsi.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,14 @@ enum {
ISCSI_SESSION_FREE,
};

enum {
ISCSI_SESSION_TARGET_UNBOUND,
ISCSI_SESSION_TARGET_ALLOCATED,
ISCSI_SESSION_TARGET_SCANNED,
ISCSI_SESSION_TARGET_UNBINDING,
ISCSI_SESSION_TARGET_MAX,
};

#define ISCSI_MAX_TARGET -1

struct iscsi_cls_session {
Expand Down Expand Up @@ -264,6 +272,7 @@ struct iscsi_cls_session {
*/
pid_t creator;
int state;
int target_state; /* session target bind state */
int sid; /* session id */
void *dd_data; /* LLD private data */
struct device dev; /* sysfs transport/container device */
Expand Down

0 comments on commit a3be19b

Please sign in to comment.