On Mon, Nov 16, 2020 at 10:44:54AM +0100, Hannes Reinecke wrote:
> On 11/16/20 10:07 AM, Ming Lei wrote:
> > Limit scsi device's queue depth is less than max(host->can_queue, 1024)
> > in scsi_change_queue_depth(), and 1024 is big enough for saturating
> > current fast SCSI LUN(SSD, or raid volume on multiple SSDs).
> >
> > We need this patch for replacing sdev->device_busy with sbitmap which
> > has to be pre-allocated with reasonable max depth.
> >
> > Cc: Omar Sandoval <osandov@xxxxxx>
> > Cc: Kashyap Desai <kashyap.desai@xxxxxxxxxxxx>
> > Cc: Sumanesh Samanta <sumanesh.samanta@xxxxxxxxxxxx>
> > Cc: Ewan D. Milne <emilne@xxxxxxxxxx>
> > Tested-by: Sumanesh Samanta <sumanesh.samanta@xxxxxxxxxxxx>
> > Signed-off-by: Ming Lei <ming.lei@xxxxxxxxxx>
> > ---
> > drivers/scsi/scsi.c | 11 +++++++++++
> > 1 file changed, 11 insertions(+)
> >
> > diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
> > index 24619c3bebd5..a28d48c850cf 100644
> > --- a/drivers/scsi/scsi.c
> > +++ b/drivers/scsi/scsi.c
> > @@ -214,6 +214,15 @@ void scsi_finish_command(struct scsi_cmnd *cmd)
> > scsi_io_completion(cmd, good_bytes);
> > }
> > +
> > +/*
> > + * 1024 is big enough for saturating the fast scsi LUN now
> > + */
> > +static int scsi_device_max_queue_depth(struct scsi_device *sdev)
> > +{
> > + return max_t(int, sdev->host->can_queue, 1024);
> > +}
> > +
>
> Shouldn't this rather be initialized with scsi_host->can_queue?
Multiple queues may be used for one single LUN, so in theory we should
return max queue depth as host->can_queue * host->nr_hw_queues, but
this number can be too big for the sbitmap's pre-allocation.
That is why this patch introduces one reasonable limit on this value
of max(sdev->host->can_queue, 1024). Suppose single SSD can be saturated
by ~128 requests, we still can saturate one LUN with 8 SSDs behind if
the hw queue depth is set as too low.
> These 'should be enough' settings inevitable turn out to be not enough in
> the long run ...
I have provided the theory behind this idea, not just simple 'should be
enough'.
Thanks,
Ming
