Re: [PATCH 5/6] scsi: iscsi: Use the session workqueue for recovery.

[Lee Duncan <lduncan@xxxxxxxx>]

On 2/26/22 15:04, Mike Christie wrote:
> Use the session workqueue for recovery and unbinding. If there are delays
> during device blocking/cleanup then it will no longer affect other
> sessions.
>
> Signed-off-by: Mike Christie <michael.christie@xxxxxxxxxx>
> ---
>   drivers/scsi/scsi_transport_iscsi.c | 20 ++++----------------
>   1 file changed, 4 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c
> index ecb592a70e03..754277bec63a 100644
> --- a/drivers/scsi/scsi_transport_iscsi.c
> +++ b/drivers/scsi/scsi_transport_iscsi.c
> @@ -87,7 +87,6 @@ struct iscsi_internal {
>   };
>   
>   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_conn_cleanup_workq;
>   
> @@ -1913,7 +1912,7 @@ void iscsi_unblock_session(struct iscsi_cls_session *session)
>   	if (!cancel_work_sync(&session->block_work))
>   		cancel_delayed_work_sync(&session->recovery_work);
>   
> -	queue_work(iscsi_eh_timer_workq, &session->unblock_work);
> +	queue_work(session->workq, &session->unblock_work);
>   	/*
>   	 * Blocking the session can be done from any context so we only
>   	 * queue the block work. Make sure the unblock work has completed
> @@ -1937,14 +1936,14 @@ static void __iscsi_block_session(struct work_struct *work)
>   	scsi_target_block(&session->dev);
>   	ISCSI_DBG_TRANS_SESSION(session, "Completed SCSI target blocking\n");
>   	if (session->recovery_tmo >= 0)
> -		queue_delayed_work(iscsi_eh_timer_workq,
> +		queue_delayed_work(session->workq,
>   				   &session->recovery_work,
>   				   session->recovery_tmo * HZ);
>   }
>   
>   void iscsi_block_session(struct iscsi_cls_session *session)
>   {
> -	queue_work(iscsi_eh_timer_workq, &session->block_work);
> +	queue_work(session->workq, &session->block_work);
>   }
>   EXPORT_SYMBOL_GPL(iscsi_block_session);
>   
> @@ -4851,26 +4850,16 @@ static __init int iscsi_transport_init(void)
>   		goto unregister_flashnode_bus;
>   	}
>   
> -	iscsi_eh_timer_workq = alloc_workqueue("%s",
> -			WQ_SYSFS | __WQ_LEGACY | WQ_MEM_RECLAIM | WQ_UNBOUND,
> -			1, "iscsi_eh");
> -	if (!iscsi_eh_timer_workq) {
> -		err = -ENOMEM;
> -		goto release_nls;
> -	}
> -
>   	iscsi_conn_cleanup_workq = alloc_workqueue("%s",
>   			WQ_SYSFS | WQ_MEM_RECLAIM | WQ_UNBOUND, 0,
>   			"iscsi_conn_cleanup");
>   	if (!iscsi_conn_cleanup_workq) {
>   		err = -ENOMEM;
> -		goto destroy_wq;
> +		goto release_nls;
>   	}
>   
>   	return 0;
>   
> -destroy_wq:
> -	destroy_workqueue(iscsi_eh_timer_workq);
>   release_nls:
>   	netlink_kernel_release(nls);
>   unregister_flashnode_bus:
> @@ -4893,7 +4882,6 @@ static __init int iscsi_transport_init(void)
>   static void __exit iscsi_transport_exit(void)
>   {
>   	destroy_workqueue(iscsi_conn_cleanup_workq);
> -	destroy_workqueue(iscsi_eh_timer_workq);
>   	netlink_kernel_release(nls);
>   	bus_unregister(&iscsi_flashnode_bus);
>   	transport_class_unregister(&iscsi_connection_class);

Reviewed-by: Lee Duncan <lduncan@xxxxxxxx>