On Thu, May 30, 2019 at 10:57 PM Hannes Reinecke <h...@suse.de> wrote:
>
> On 5/30/19 8:41 AM, Ming Lei wrote:
> > On Wed, May 29, 2019 at 03:28:39PM +0200, Hannes Reinecke wrote:
> >> Add helper functions to retrieve SCSI commands from the reserved
> >> tag pool.
> >>
> >> Signed-off-by: Hannes Reinecke <h...@suse.com>
> >> ---
> >>   include/scsi/scsi_tcq.h | 22 ++++++++++++++++++++++
> >>   1 file changed, 22 insertions(+)
> >>
> >> diff --git a/include/scsi/scsi_tcq.h b/include/scsi/scsi_tcq.h
> >> index 6053d46e794e..227f3bd4e974 100644
> >> --- a/include/scsi/scsi_tcq.h
> >> +++ b/include/scsi/scsi_tcq.h
> >> @@ -39,5 +39,27 @@ static inline struct scsi_cmnd 
> >> *scsi_host_find_tag(struct Scsi_Host *shost,
> >>      return blk_mq_rq_to_pdu(req);
> >>   }
> >>
> >> +static inline struct scsi_cmnd *scsi_get_reserved_cmd(struct scsi_device 
> >> *sdev)
> >> +{
> >> +    struct request *rq;
> >> +    struct scsi_cmnd *scmd;
> >> +
> >> +    rq = blk_mq_alloc_request(sdev->request_queue,
> >> +                              REQ_OP_SCSI_OUT | REQ_NOWAIT,
> >> +                              BLK_MQ_REQ_RESERVED);
> >> +    if (IS_ERR(rq))
> >> +            return NULL;
> >> +    scmd = blk_mq_rq_to_pdu(rq);
> >> +    scmd->request = rq;
> >> +    return scmd;
> >> +}
> >
> > Now all these internal commands won't share tags with IO requests,
> > however, your patch switches to reserve slots for internal
> > commands.
> >
> Yes.
>
> > This way may have performance effect on IO workloads given the
> > reserved tags can't be used by IO at all.
> >
> Not really. Basically all drivers which have to use tags to send
> internal commands already set some tags aside to handle internal
> commands. So all this patchset does is to formalize this behaviour by
> using private tags.
> Some drivers (like fnic or snic) does _not_ do this currently; for those
> I've set one command aside to handle command abort etc.

>From hardware view, you might be right, however, the implementation
isn't correct:

set->queue_depth means number of the total tags, set->reserved_tags is just
part of the total tags, see blk_mq_init_bitmap_tags().

So any driver sets .reserved_tags > 0, tags available for IO is reduced by
same amount, isn't it?  Cause .can_queue isn't increased.


Thanks,
Ming Lei

Reply via email to