| Message ID | 20200710104817.19462-3-bstroesser@ts.fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | scsi: target: tcmu: Add TMR notification for tcmu | expand |
On 7/10/20 5:48 AM, Bodo Stroesser wrote: > Target core is modified to call an optional backend > callback function if a TMR is received or commands > are aborted implicitly after a PR command was received. > The backend function takes as parameters the se_dev, the > type of the TMR, and the list of aborted commands. > If no commands were aborted, an empty list is supplied. > > Signed-off-by: Bodo Stroesser <bstroesser@ts.fujitsu.com> > --- > drivers/target/target_core_tmr.c | 16 +++++++++++++++- > drivers/target/target_core_transport.c | 1 + > include/target/target_core_backend.h | 2 ++ > include/target/target_core_base.h | 1 + > 4 files changed, 19 insertions(+), 1 deletion(-) > > diff --git a/drivers/target/target_core_tmr.c b/drivers/target/target_core_tmr.c > index b65d7a0a5df1..39d93357db65 100644 > --- a/drivers/target/target_core_tmr.c > +++ b/drivers/target/target_core_tmr.c > @@ -116,6 +116,7 @@ void core_tmr_abort_task( > struct se_tmr_req *tmr, > struct se_session *se_sess) > { > + LIST_HEAD(aborted_list); > struct se_cmd *se_cmd, *next; > unsigned long flags; > bool rc; > @@ -144,7 +145,7 @@ void core_tmr_abort_task( > if (!rc) > continue; > > - list_del_init(&se_cmd->state_list); > + list_move_tail(&se_cmd->state_list, &aborted_list); > se_cmd->state_active = false; > > spin_unlock_irqrestore(&dev->execute_task_lock, flags); > @@ -157,6 +158,11 @@ void core_tmr_abort_task( > WARN_ON_ONCE(transport_lookup_tmr_lun(tmr->task_cmd) < > 0); > > + if (dev->transport->tmr_notify) > + dev->transport->tmr_notify(dev, TMR_ABORT_TASK, > + &aborted_list); > + > + list_del_init(&se_cmd->state_list); > target_put_cmd_and_wait(se_cmd); > > printk("ABORT_TASK: Sending TMR_FUNCTION_COMPLETE for" > @@ -167,6 +173,9 @@ void core_tmr_abort_task( > } > spin_unlock_irqrestore(&dev->execute_task_lock, flags); > > + if (dev->transport->tmr_notify) > + dev->transport->tmr_notify(dev, TMR_ABORT_TASK, &aborted_list); Is this needed? It seems like the backend can't do anything because there isn't enough info. I saw in tcmu_tmr_notify it looks when then happens we can still do queue_tmr_ring, but there would be no commands. Was that intentional? > + > printk("ABORT_TASK: Sending TMR_TASK_DOES_NOT_EXIST for ref_tag: %lld\n", > tmr->ref_task_tag); > tmr->response = TMR_TASK_DOES_NOT_EXIST; > @@ -318,6 +327,11 @@ static void core_tmr_drain_state_list( > } > spin_unlock_irqrestore(&dev->execute_task_lock, flags); > > + if (dev->transport->tmr_notify) > + dev->transport->tmr_notify(dev, preempt_and_abort_list ? > + TMR_LUN_RESET_PRO : TMR_LUN_RESET, > + &drain_task_list); > + > while (!list_empty(&drain_task_list)) { > cmd = list_entry(drain_task_list.next, struct se_cmd, state_list); > list_del_init(&cmd->state_list); > diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c > index e6e1fa68de54..9fb0be0aa620 100644 > --- a/drivers/target/target_core_transport.c > +++ b/drivers/target/target_core_transport.c > @@ -2946,6 +2946,7 @@ static const char *target_tmf_name(enum tcm_tmreq_table tmf) > case TMR_LUN_RESET: return "LUN_RESET"; > case TMR_TARGET_WARM_RESET: return "TARGET_WARM_RESET"; > case TMR_TARGET_COLD_RESET: return "TARGET_COLD_RESET"; > + case TMR_LUN_RESET_PRO: return "LUN_RESET_PRO"; > case TMR_UNKNOWN: break; > } > return "(?)"; > diff --git a/include/target/target_core_backend.h b/include/target/target_core_backend.h > index f51452e3b984..6336780d83a7 100644 > --- a/include/target/target_core_backend.h > +++ b/include/target/target_core_backend.h > @@ -40,6 +40,8 @@ struct target_backend_ops { > ssize_t (*show_configfs_dev_params)(struct se_device *, char *); > > sense_reason_t (*parse_cdb)(struct se_cmd *cmd); > + void (*tmr_notify)(struct se_device *se_dev, enum tcm_tmreq_table, > + struct list_head *aborted_cmds); > u32 (*get_device_type)(struct se_device *); > sector_t (*get_blocks)(struct se_device *); > sector_t (*get_alignment_offset_lbas)(struct se_device *); > diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h > index 18c3f277b770..549947d407cf 100644 > --- a/include/target/target_core_base.h > +++ b/include/target/target_core_base.h > @@ -207,6 +207,7 @@ enum tcm_tmreq_table { > TMR_LUN_RESET = 5, > TMR_TARGET_WARM_RESET = 6, > TMR_TARGET_COLD_RESET = 7, > + TMR_LUN_RESET_PRO = 0x80, > TMR_UNKNOWN = 0xff, > }; > >
On 2020-07-12 01:27, Mike Christie wrote: > On 7/10/20 5:48 AM, Bodo Stroesser wrote: >> Target core is modified to call an optional backend >> callback function if a TMR is received or commands >> are aborted implicitly after a PR command was received. >> The backend function takes as parameters the se_dev, the >> type of the TMR, and the list of aborted commands. >> If no commands were aborted, an empty list is supplied. >> >> Signed-off-by: Bodo Stroesser <bstroesser@ts.fujitsu.com> >> --- >> drivers/target/target_core_tmr.c | 16 +++++++++++++++- >> drivers/target/target_core_transport.c | 1 + >> include/target/target_core_backend.h | 2 ++ >> include/target/target_core_base.h | 1 + >> 4 files changed, 19 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/target/target_core_tmr.c >> b/drivers/target/target_core_tmr.c >> index b65d7a0a5df1..39d93357db65 100644 >> --- a/drivers/target/target_core_tmr.c >> +++ b/drivers/target/target_core_tmr.c >> @@ -116,6 +116,7 @@ void core_tmr_abort_task( >> struct se_tmr_req *tmr, >> struct se_session *se_sess) >> { >> + LIST_HEAD(aborted_list); >> struct se_cmd *se_cmd, *next; >> unsigned long flags; >> bool rc; >> @@ -144,7 +145,7 @@ void core_tmr_abort_task( >> if (!rc) >> continue; >> - list_del_init(&se_cmd->state_list); >> + list_move_tail(&se_cmd->state_list, &aborted_list); >> se_cmd->state_active = false; >> spin_unlock_irqrestore(&dev->execute_task_lock, flags); >> @@ -157,6 +158,11 @@ void core_tmr_abort_task( >> WARN_ON_ONCE(transport_lookup_tmr_lun(tmr->task_cmd) < >> 0); >> + if (dev->transport->tmr_notify) >> + dev->transport->tmr_notify(dev, TMR_ABORT_TASK, >> + &aborted_list); >> + >> + list_del_init(&se_cmd->state_list); >> target_put_cmd_and_wait(se_cmd); >> printk("ABORT_TASK: Sending TMR_FUNCTION_COMPLETE for" >> @@ -167,6 +173,9 @@ void core_tmr_abort_task( >> } >> spin_unlock_irqrestore(&dev->execute_task_lock, flags); >> + if (dev->transport->tmr_notify) >> + dev->transport->tmr_notify(dev, TMR_ABORT_TASK, &aborted_list); > > Is this needed? It seems like the backend can't do anything because > there isn't enough info. > > I saw in tcmu_tmr_notify it looks when then happens we can still do > queue_tmr_ring, but there would be no commands. Was that intentional? Yes, it is intentional. I see two purposes for backend tmr notification: 1) Allow backend (userspace) to cancel long running command execution if possible, which then makes the core more 'responsive' to TMRs 2) Tracing. If a userspace device emulation does tracing, it would be good to see an ABORT_TASK in trace even if core does not find the aborted cmd. The reason for this e.g. can be, that userspace and tcmu complete a command while initiator times out and sends ABORT_TASK. In that case ABORT_TASK in trace is helpful, because initiator will not accept the command completion but start some error handling. Without the ABORT_TASK entry the trace would not show the reason for error handling.
diff --git a/drivers/target/target_core_tmr.c b/drivers/target/target_core_tmr.c index b65d7a0a5df1..39d93357db65 100644 --- a/drivers/target/target_core_tmr.c +++ b/drivers/target/target_core_tmr.c @@ -116,6 +116,7 @@ void core_tmr_abort_task( struct se_tmr_req *tmr, struct se_session *se_sess) { + LIST_HEAD(aborted_list); struct se_cmd *se_cmd, *next; unsigned long flags; bool rc; @@ -144,7 +145,7 @@ void core_tmr_abort_task( if (!rc) continue; - list_del_init(&se_cmd->state_list); + list_move_tail(&se_cmd->state_list, &aborted_list); se_cmd->state_active = false; spin_unlock_irqrestore(&dev->execute_task_lock, flags); @@ -157,6 +158,11 @@ void core_tmr_abort_task( WARN_ON_ONCE(transport_lookup_tmr_lun(tmr->task_cmd) < 0); + if (dev->transport->tmr_notify) + dev->transport->tmr_notify(dev, TMR_ABORT_TASK, + &aborted_list); + + list_del_init(&se_cmd->state_list); target_put_cmd_and_wait(se_cmd); printk("ABORT_TASK: Sending TMR_FUNCTION_COMPLETE for" @@ -167,6 +173,9 @@ void core_tmr_abort_task( } spin_unlock_irqrestore(&dev->execute_task_lock, flags); + if (dev->transport->tmr_notify) + dev->transport->tmr_notify(dev, TMR_ABORT_TASK, &aborted_list); + printk("ABORT_TASK: Sending TMR_TASK_DOES_NOT_EXIST for ref_tag: %lld\n", tmr->ref_task_tag); tmr->response = TMR_TASK_DOES_NOT_EXIST; @@ -318,6 +327,11 @@ static void core_tmr_drain_state_list( } spin_unlock_irqrestore(&dev->execute_task_lock, flags); + if (dev->transport->tmr_notify) + dev->transport->tmr_notify(dev, preempt_and_abort_list ? + TMR_LUN_RESET_PRO : TMR_LUN_RESET, + &drain_task_list); + while (!list_empty(&drain_task_list)) { cmd = list_entry(drain_task_list.next, struct se_cmd, state_list); list_del_init(&cmd->state_list); diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c index e6e1fa68de54..9fb0be0aa620 100644 --- a/drivers/target/target_core_transport.c +++ b/drivers/target/target_core_transport.c @@ -2946,6 +2946,7 @@ static const char *target_tmf_name(enum tcm_tmreq_table tmf) case TMR_LUN_RESET: return "LUN_RESET"; case TMR_TARGET_WARM_RESET: return "TARGET_WARM_RESET"; case TMR_TARGET_COLD_RESET: return "TARGET_COLD_RESET"; + case TMR_LUN_RESET_PRO: return "LUN_RESET_PRO"; case TMR_UNKNOWN: break; } return "(?)"; diff --git a/include/target/target_core_backend.h b/include/target/target_core_backend.h index f51452e3b984..6336780d83a7 100644 --- a/include/target/target_core_backend.h +++ b/include/target/target_core_backend.h @@ -40,6 +40,8 @@ struct target_backend_ops { ssize_t (*show_configfs_dev_params)(struct se_device *, char *); sense_reason_t (*parse_cdb)(struct se_cmd *cmd); + void (*tmr_notify)(struct se_device *se_dev, enum tcm_tmreq_table, + struct list_head *aborted_cmds); u32 (*get_device_type)(struct se_device *); sector_t (*get_blocks)(struct se_device *); sector_t (*get_alignment_offset_lbas)(struct se_device *); diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h index 18c3f277b770..549947d407cf 100644 --- a/include/target/target_core_base.h +++ b/include/target/target_core_base.h @@ -207,6 +207,7 @@ enum tcm_tmreq_table { TMR_LUN_RESET = 5, TMR_TARGET_WARM_RESET = 6, TMR_TARGET_COLD_RESET = 7, + TMR_LUN_RESET_PRO = 0x80, TMR_UNKNOWN = 0xff, };
Target core is modified to call an optional backend callback function if a TMR is received or commands are aborted implicitly after a PR command was received. The backend function takes as parameters the se_dev, the type of the TMR, and the list of aborted commands. If no commands were aborted, an empty list is supplied. Signed-off-by: Bodo Stroesser <bstroesser@ts.fujitsu.com> --- drivers/target/target_core_tmr.c | 16 +++++++++++++++- drivers/target/target_core_transport.c | 1 + include/target/target_core_backend.h | 2 ++ include/target/target_core_base.h | 1 + 4 files changed, 19 insertions(+), 1 deletion(-)