| Message ID | 20210523175739.360324-18-michael.christie@oracle.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [01/28] scsi: iscsi: Add task completion helper | expand |
On 5/23/21 10:57 AM, Mike Christie wrote: > For aborts, qedi needs to cleanup the FW then send the TMF from a worker > thread. While it's doing these the cmd could complete normally and the TMF > could time out. libiscsi would then complete the iscsi_task which will > call into the driver to cleanup the driver level resources while it still > might be accessig them for the cleanup/abort. > > This has iscsi_eh_abort keep the iscsi_task ref if the TMF times out, so > qedi does not have to worry about if the task been freed while in use typo: "been" -> "being" > and does not need to get its own ref. > > Signed-off-by: Mike Christie <michael.christie@oracle.com> > ---iscsi_stop_conn > drivers/scsi/libiscsi.c | 15 ++++++++++++++- > include/scsi/libiscsi.h | 1 + > 2 files changed, 15 insertions(+), 1 deletion(-) > > diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c > index 8222db4f8fef..9247f70d2daa 100644 > --- a/drivers/scsi/libiscsi.c > +++ b/drivers/scsi/libiscsi.c > @@ -573,6 +573,11 @@ static bool cleanup_queued_task(struct iscsi_task *task) > __iscsi_put_task(task); > } > > + if (conn->session->running_aborted_task == task) { > + conn->session->running_aborted_task = NULL; > + __iscsi_put_task(task); > + } > + > if (conn->task == task) {iscsi_stop_conn > conn->task = NULL; > __iscsi_put_task(task); > @@ -2334,6 +2339,7 @@ int iscsi_eh_abort(struct scsi_cmnd *sc) > iscsi_start_tx(conn); > goto success_unlocked; > case TMF_TIMEDOUT: > + session->running_aborted_task = task; > spin_unlock_bh(&session->frwd_lock); > iscsi_conn_failure(conn, ISCSI_ERR_SCSI_EH_SESSION_RST); > goto failed_unlocked; > @@ -2367,7 +2373,14 @@ int iscsi_eh_abort(struct scsi_cmnd *sc) > failed_unlocked: > ISCSI_DBG_EH(session, "abort failed [sc %p itt 0x%x]\n", sc, > task ? task->itt : 0); > - iscsi_put_task(task); > + /* > + * The driver might be accessing the task so hold the ref. The conn > + * stop cleanup will drop the ref after ep_disconnect so we know the > + * driver's no longer touching the task. > + */ > + if (!session->running_aborted_task) > + iscsi_put_task(task); > + > iscsi_put_conn(conn->cls_conn); > mutex_unlock(&session->eh_mutex); > return FAILED; > diff --git a/include/scsi/libiscsi.h b/include/scsi/libiscsi.h > index 9d7908265afe..4ee233e5a6ff 100644 > --- a/include/scsi/libiscsi.h > +++ b/include/scsi/libiscsi.h > @@ -276,6 +276,7 @@ struct iscsi_session { > struct iscsi_tm tmhdr; > struct timer_list tmf_timer; > int tmf_state; /* see TMF_INITIAL, etc.*/ > + struct iscsi_task *running_aborted_task; > > /* iSCSI session-wide sequencing */ > uint32_t cmdsn; > Reviewed-by: Lee Duncan <lduncan@suse.com>
diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c index 8222db4f8fef..9247f70d2daa 100644 --- a/drivers/scsi/libiscsi.c +++ b/drivers/scsi/libiscsi.c @@ -573,6 +573,11 @@ static bool cleanup_queued_task(struct iscsi_task *task) __iscsi_put_task(task); } + if (conn->session->running_aborted_task == task) { + conn->session->running_aborted_task = NULL; + __iscsi_put_task(task); + } + if (conn->task == task) { conn->task = NULL; __iscsi_put_task(task); @@ -2334,6 +2339,7 @@ int iscsi_eh_abort(struct scsi_cmnd *sc) iscsi_start_tx(conn); goto success_unlocked; case TMF_TIMEDOUT: + session->running_aborted_task = task; spin_unlock_bh(&session->frwd_lock); iscsi_conn_failure(conn, ISCSI_ERR_SCSI_EH_SESSION_RST); goto failed_unlocked; @@ -2367,7 +2373,14 @@ int iscsi_eh_abort(struct scsi_cmnd *sc) failed_unlocked: ISCSI_DBG_EH(session, "abort failed [sc %p itt 0x%x]\n", sc, task ? task->itt : 0); - iscsi_put_task(task); + /* + * The driver might be accessing the task so hold the ref. The conn + * stop cleanup will drop the ref after ep_disconnect so we know the + * driver's no longer touching the task. + */ + if (!session->running_aborted_task) + iscsi_put_task(task); + iscsi_put_conn(conn->cls_conn); mutex_unlock(&session->eh_mutex); return FAILED; diff --git a/include/scsi/libiscsi.h b/include/scsi/libiscsi.h index 9d7908265afe..4ee233e5a6ff 100644 --- a/include/scsi/libiscsi.h +++ b/include/scsi/libiscsi.h @@ -276,6 +276,7 @@ struct iscsi_session { struct iscsi_tm tmhdr; struct timer_list tmf_timer; int tmf_state; /* see TMF_INITIAL, etc.*/ + struct iscsi_task *running_aborted_task; /* iSCSI session-wide sequencing */ uint32_t cmdsn;
For aborts, qedi needs to cleanup the FW then send the TMF from a worker thread. While it's doing these the cmd could complete normally and the TMF could time out. libiscsi would then complete the iscsi_task which will call into the driver to cleanup the driver level resources while it still might be accessig them for the cleanup/abort. This has iscsi_eh_abort keep the iscsi_task ref if the TMF times out, so qedi does not have to worry about if the task been freed while in use and does not need to get its own ref. Signed-off-by: Mike Christie <michael.christie@oracle.com> --- drivers/scsi/libiscsi.c | 15 ++++++++++++++- include/scsi/libiscsi.h | 1 + 2 files changed, 15 insertions(+), 1 deletion(-)