Re: [PATCH] iscsi: Add support for asynchronous iSCSI session destruction
On Friday, January 17, 2020 at 3:33:35 PM UTC-8, Gabriel Krisman Bertazi
wrote:
>
> From: Frank Mayhar
>
> iSCSI session destruction can be arbitrarily slow, since it might
> require network operations and serialization inside the scsi layer.
> This patch adds a new user event to trigger the destruction work
> asynchronously, releasing the rx_queue_mutex as soon as the operation is
> queued and before it is performed. This change allow other operations
> to run in other sessions in the meantime, removing one of the major
> iSCSI bottlenecks for us.
>
> To prevent the session from being used after the destruction request, we
> remove it immediately from the sesslist. This simplifies the locking
> required during the asynchronous removal.
>
> Co-developed-by: Khazhismel Kumykov
> Signed-off-by: Khazhismel Kumykov
> Signed-off-by: Frank Mayhar
> Co-developed-by: Gabriel Krisman Bertazi
> Signed-off-by: Gabriel Krisman Bertazi
> ---
>
> This patch requires a patch that just went upstream to apply cleanly.
> it is ("iscsi: Don't destroy session if there are outstanding
> connections"), which was just merged by Martin into 5.6/scsi-queue.
> Please make sure you have it in your tree, otherwise this one won't
> apply.
>
> drivers/scsi/scsi_transport_iscsi.c | 36 +
> include/scsi/iscsi_if.h | 1 +
> include/scsi/scsi_transport_iscsi.h | 1 +
> 3 files changed, 38 insertions(+)
>
> diff --git a/drivers/scsi/scsi_transport_iscsi.c
> b/drivers/scsi/scsi_transport_iscsi.c
> index ba6cfaf71aef..e9a8e0317b0d 100644
> --- a/drivers/scsi/scsi_transport_iscsi.c
> +++ b/drivers/scsi/scsi_transport_iscsi.c
> @@ -95,6 +95,8 @@ static DECLARE_WORK(stop_conn_work, stop_conn_work_fn);
> static atomic_t iscsi_session_nr; /* sysfs session id for next new
> session */
> static struct workqueue_struct *iscsi_eh_timer_workq;
>
> +static struct workqueue_struct *iscsi_destroy_workq;
> +
> static DEFINE_IDA(iscsi_sess_ida);
> /*
> * list of registered transports and lock that must
> @@ -1615,6 +1617,7 @@ static struct sock *nls;
> static DEFINE_MUTEX(rx_queue_mutex);
>
> static LIST_HEAD(sesslist);
> +static LIST_HEAD(sessdestroylist);
> static DEFINE_SPINLOCK(sesslock);
> static LIST_HEAD(connlist);
> static LIST_HEAD(connlist_err);
> @@ -2035,6 +2038,14 @@ static void __iscsi_unbind_session(struct
> work_struct *work)
> ISCSI_DBG_TRANS_SESSION(session, "Completed target removal\n");
> }
>
> +static void __iscsi_destroy_session(struct work_struct *work)
> +{
> +struct iscsi_cls_session *session =
> +container_of(work, struct iscsi_cls_session,
> destroy_work);
> +
> +session->transport->destroy_session(session);
> +}
> +
> struct iscsi_cls_session *
> iscsi_alloc_session(struct Scsi_Host *shost, struct iscsi_transport
> *transport,
> int dd_size)
> @@ -2057,6 +2068,7 @@ iscsi_alloc_session(struct Scsi_Host *shost, struct
> iscsi_transport *transport,
> INIT_WORK(>block_work, __iscsi_block_session);
> INIT_WORK(>unbind_work, __iscsi_unbind_session);
> INIT_WORK(>scan_work, iscsi_scan_session);
> +INIT_WORK(>destroy_work, __iscsi_destroy_session);
> spin_lock_init(>lock);
>
> /* this is released in the dev's release function */
> @@ -3617,6 +3629,23 @@ iscsi_if_recv_msg(struct sk_buff *skb, struct
> nlmsghdr *nlh, uint32_t *group)
> else
> transport->destroy_session(session);
> break;
> +case ISCSI_UEVENT_DESTROY_SESSION_ASYNC:
> +session = iscsi_session_lookup(ev->u.d_session.sid);
> +if (!session)
> +err = -EINVAL;
> +else if (iscsi_session_has_conns(ev->u.d_session.sid))
> +err = -EBUSY;
> +else {
> +unsigned long flags;
> +
> +/* Prevent this session from being found again */
> +spin_lock_irqsave(, flags);
> +list_move(>sess_list, );
> +spin_unlock_irqrestore(, flags);
> +
> +queue_work(iscsi_destroy_workq,
> >destroy_work);
> +}
> +break;
> case ISCSI_UEVENT_UNBIND_SESSION:
> session = iscsi_session_lookup(ev->u.d_session.sid);
> if (session)
> @@ -4662,8 +4691,14 @@ static __init int iscsi_transport_init(void)
> goto release_nls;
> }
>
> +iscsi_destroy_workq =
> create_singlethread_workqueue("iscsi_destroy");
> +if (!iscsi_destroy_workq)
> +goto destroy_wq;
> +
> return 0;
>
> +destroy_wq:
> +destroy_workqueue(iscsi_eh_timer_workq);
> release_nls:
>
[PATCH] iscsi: Add support for asynchronous iSCSI session destruction
From: Frank Mayhar
iSCSI session destruction can be arbitrarily slow, since it might
require network operations and serialization inside the scsi layer.
This patch adds a new user event to trigger the destruction work
asynchronously, releasing the rx_queue_mutex as soon as the operation is
queued and before it is performed. This change allow other operations
to run in other sessions in the meantime, removing one of the major
iSCSI bottlenecks for us.
To prevent the session from being used after the destruction request, we
remove it immediately from the sesslist. This simplifies the locking
required during the asynchronous removal.
Co-developed-by: Khazhismel Kumykov
Signed-off-by: Khazhismel Kumykov
Signed-off-by: Frank Mayhar
Co-developed-by: Gabriel Krisman Bertazi
Signed-off-by: Gabriel Krisman Bertazi
---
This patch requires a patch that just went upstream to apply cleanly.
it is ("iscsi: Don't destroy session if there are outstanding
connections"), which was just merged by Martin into 5.6/scsi-queue.
Please make sure you have it in your tree, otherwise this one won't
apply.
drivers/scsi/scsi_transport_iscsi.c | 36 +
include/scsi/iscsi_if.h | 1 +
include/scsi/scsi_transport_iscsi.h | 1 +
3 files changed, 38 insertions(+)
diff --git a/drivers/scsi/scsi_transport_iscsi.c
b/drivers/scsi/scsi_transport_iscsi.c
index ba6cfaf71aef..e9a8e0317b0d 100644
--- a/drivers/scsi/scsi_transport_iscsi.c
+++ b/drivers/scsi/scsi_transport_iscsi.c
@@ -95,6 +95,8 @@ static DECLARE_WORK(stop_conn_work, stop_conn_work_fn);
static atomic_t iscsi_session_nr; /* sysfs session id for next new session */
static struct workqueue_struct *iscsi_eh_timer_workq;
+static struct workqueue_struct *iscsi_destroy_workq;
+
static DEFINE_IDA(iscsi_sess_ida);
/*
* list of registered transports and lock that must
@@ -1615,6 +1617,7 @@ static struct sock *nls;
static DEFINE_MUTEX(rx_queue_mutex);
static LIST_HEAD(sesslist);
+static LIST_HEAD(sessdestroylist);
static DEFINE_SPINLOCK(sesslock);
static LIST_HEAD(connlist);
static LIST_HEAD(connlist_err);
@@ -2035,6 +2038,14 @@ static void __iscsi_unbind_session(struct work_struct
*work)
ISCSI_DBG_TRANS_SESSION(session, "Completed target removal\n");
}
+static void __iscsi_destroy_session(struct work_struct *work)
+{
+ struct iscsi_cls_session *session =
+ container_of(work, struct iscsi_cls_session, destroy_work);
+
+ session->transport->destroy_session(session);
+}
+
struct iscsi_cls_session *
iscsi_alloc_session(struct Scsi_Host *shost, struct iscsi_transport *transport,
int dd_size)
@@ -2057,6 +2068,7 @@ iscsi_alloc_session(struct Scsi_Host *shost, struct
iscsi_transport *transport,
INIT_WORK(>block_work, __iscsi_block_session);
INIT_WORK(>unbind_work, __iscsi_unbind_session);
INIT_WORK(>scan_work, iscsi_scan_session);
+ INIT_WORK(>destroy_work, __iscsi_destroy_session);
spin_lock_init(>lock);
/* this is released in the dev's release function */
@@ -3617,6 +3629,23 @@ iscsi_if_recv_msg(struct sk_buff *skb, struct nlmsghdr
*nlh, uint32_t *group)
else
transport->destroy_session(session);
break;
+ case ISCSI_UEVENT_DESTROY_SESSION_ASYNC:
+ session = iscsi_session_lookup(ev->u.d_session.sid);
+ if (!session)
+ err = -EINVAL;
+ else if (iscsi_session_has_conns(ev->u.d_session.sid))
+ err = -EBUSY;
+ else {
+ unsigned long flags;
+
+ /* Prevent this session from being found again */
+ spin_lock_irqsave(, flags);
+ list_move(>sess_list, );
+ spin_unlock_irqrestore(, flags);
+
+ queue_work(iscsi_destroy_workq, >destroy_work);
+ }
+ break;
case ISCSI_UEVENT_UNBIND_SESSION:
session = iscsi_session_lookup(ev->u.d_session.sid);
if (session)
@@ -4662,8 +4691,14 @@ static __init int iscsi_transport_init(void)
goto release_nls;
}
+ iscsi_destroy_workq = create_singlethread_workqueue("iscsi_destroy");
+ if (!iscsi_destroy_workq)
+ goto destroy_wq;
+
return 0;
+destroy_wq:
+ destroy_workqueue(iscsi_eh_timer_workq);
release_nls:
netlink_kernel_release(nls);
unregister_flashnode_bus:
@@ -4685,6 +4720,7 @@ static __init int iscsi_transport_init(void)
static void __exit iscsi_transport_exit(void)
{
+ destroy_workqueue(iscsi_destroy_workq);
destroy_workqueue(iscsi_eh_timer_workq);
netlink_kernel_release(nls);
bus_unregister(_flashnode_bus);
diff --git a/include/scsi/iscsi_if.h b/include/scsi/iscsi_if.h
index 92b11c7e0b4f..deacaee53e61 100644
---
