On Fri, 2017-06-02 at 22:35 -0700, Nicholas A. Bellinger wrote:
> On Fri, 2017-06-02 at 17:11 +0000, Bart Van Assche wrote:
> > On Thu, 2017-06-01 at 21:44 -0700, Nicholas A. Bellinger wrote:
> > > On Tue, 2017-05-23 at 16:48 -0700, Bart Van Assche wrote:
> > > > Introduce a function that sends the SCSI status "BUSY" back to the
> > > > initiator.
> > > >
> > > > Signed-off-by: Bart Van Assche <bart.vanassche@xxxxxxxxxxx>
> > > > Reviewed-by: Hannes Reinecke <hare@xxxxxxxx>
> > > > Cc: Christoph Hellwig <hch@xxxxxx>
> > > > Cc: Andy Grover <agrover@xxxxxxxxxx>
> > > > Cc: David Disseldorp <ddiss@xxxxxxx>
> > > > ---
> > > > drivers/target/target_core_transport.c | 10 ++++++++++
> > > > include/target/target_core_fabric.h | 1 +
> > > > 2 files changed, 11 insertions(+)
> > > >
> > > > diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
> > > > index cfe69c1fc879..c28e3b58150b 100644
> > > > --- a/drivers/target/target_core_transport.c
> > > > +++ b/drivers/target/target_core_transport.c
> > > > @@ -3130,6 +3130,16 @@ transport_send_check_condition_and_sense(struct se_cmd *cmd,
> > > > }
> > > > EXPORT_SYMBOL(transport_send_check_condition_and_sense);
> > > >
> > > > +int target_send_busy(struct se_cmd *cmd)
> > > > +{
> > > > + WARN_ON_ONCE(cmd->se_cmd_flags & SCF_SCSI_TMR_CDB);
> > > > +
> > > > + cmd->scsi_status = SAM_STAT_BUSY;
> > > > + trace_target_cmd_complete(cmd);
> > > > + return cmd->se_tfo->queue_status(cmd);
> > > > +}
> > > > +EXPORT_SYMBOL(target_send_busy);
> > > > +
> > >
> > > This doesn't handle queue-full, or any of the other interesting failure
> > > conditions.
> > >
> > > It really needs to be using the normal completion path that already
> > > handles all of those cases instead.
> >
> > Hello Nic,
> >
> > Have you noticed that this function is *only* used if target_submit_tmr()
> > fails and that the normal completion path is not triggered at all if
> > target_submit_tmr() fails?
>
> Doesn't matter. If common code is added to target-core it needs to work
> for all cases, and handle error conditions.
>
> >
> > > That said, MNC and I are working on a patch to allow this.
> >
> > Which patch are you referring to? Has it already been posted on the
> > target-devel mailing list?
> >
>
> Yes.
>
> The bottom part of http://www.spinics.net/lists/target-devel/msg11835.html
>
> diff --git a/drivers/target/target_core_transport.c b/drivers/target/target_core_transport.c
> index 6becc94..eb12ae2 100644
> --- a/drivers/target/target_core_transport.c
> +++ b/drivers/target/target_core_transport.c
> @@ -732,11 +732,20 @@ static unsigned char *transport_get_sense_buffer(struct se_cmd *cmd)
> void target_complete_cmd(struct se_cmd *cmd, u8 scsi_status)
> {
> struct se_device *dev = cmd->se_dev;
> - int success = scsi_status == GOOD;
> + int success;
> unsigned long flags;
>
> cmd->scsi_status = scsi_status;
> -
> + switch (cmd->scsi_status) {
> + case SAM_STAT_GOOD:
> + case SAM_STAT_BUSY:
> + case SAM_STAT_TASK_SET_FULL:
> + success = 1;
> + break;
> + default:
> + success = 0;
> + break;
> + }
>
> spin_lock_irqsave(&cmd->t_state_lock, flags);
> cmd->transport_state &= ~CMD_T_BUSY;
>
> And more recently MNC's version:
>
> http://www.spinics.net/lists/target-devel/msg15436.html
Sorry but you make me repeat my previous reply: both patches you referred to
are only relevant if target_submit_tmr() has *succeeded*. Patches 12 and 13
in this series are for the case when target_submit_tmr() *failed*. So please
reconsider these patches.
Bart.--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
