[regression] uas no longer queues commands since v4.5.2

[regression] uas no longer queues commands since v4.5.2

[Tom Yan]

With commit 198de51dbc3454d95b015ca0a055b673f85f01bb, uas no longer
set `queue_depth` with scsi_change_queue_depth(), so now `queue_depth`
of UAS drives are 1. Even though `can_queue` is set to
`devinfo->qdepth - 2`, but apparently that does not help, since I can
see performance impact.

Suppose limiting `can_queue` is the right way to fix this bug
(https://bugzilla.redhat.com/show_bug.cgi?id=1315013), do we really
need to eliminate scsi_change_queue_depth() at the same time though?
If so, how should we deal with the regression?

Re: [regression] uas no longer queues commands since v4.5.2

[Tom Yan]

Btw, I suppose `.can_queue = MAX_CMNDS` in the host template is
unnecessary (unless we are going to revert `shost->can_queue =
devinfo->qdepth - 2`)?

On 22 May 2016 at 18:39, Tom Yan <tom.ty89-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> With commit 198de51dbc3454d95b015ca0a055b673f85f01bb, uas no longer
> set `queue_depth` with scsi_change_queue_depth(), so now `queue_depth`
> of UAS drives are 1. Even though `can_queue` is set to
> `devinfo->qdepth - 2`, but apparently that does not help, since I can
> see performance impact.
>
> Suppose limiting `can_queue` is the right way to fix this bug
> (https://bugzilla.redhat.com/show_bug.cgi?id=1315013), do we really
> need to eliminate scsi_change_queue_depth() at the same time though?
> If so, how should we deal with the regression?
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [regression] uas no longer queues commands since v4.5.2

[Hans de Goede]

Hi,

On 22-05-16 12:39, Tom Yan wrote:
> With commit 198de51dbc3454d95b015ca0a055b673f85f01bb, uas no longer
> set `queue_depth` with scsi_change_queue_depth(), so now `queue_depth`
> of UAS drives are 1. Even though `can_queue` is set to
> `devinfo->qdepth - 2`, but apparently that does not help, since I can
> see performance impact.
>
> Suppose limiting `can_queue` is the right way to fix this bug
> (https://bugzilla.redhat.com/show_bug.cgi?id=1315013), do we really
> need to eliminate scsi_change_queue_depth() at the same time though?

No, I thought it was no longer needed, assuming the slave would
inherit the host's value, but I was mistaken, the slave will default
to a queue_depth of 1 if not specified.

I'll send out a patch fixing this right away.

Regards,

Hans

Re: [regression] uas no longer queues commands since v4.5.2

[James Bottomley]

On Mon, 2016-05-23 at 13:48 +0200, Hans de Goede wrote:
> Hi,
> 
> On 22-05-16 12:39, Tom Yan wrote:
> > With commit 198de51dbc3454d95b015ca0a055b673f85f01bb, uas no longer
> > set `queue_depth` with scsi_change_queue_depth(), so now
> > `queue_depth`
> > of UAS drives are 1. Even though `can_queue` is set to
> > `devinfo->qdepth - 2`, but apparently that does not help, since I
> > can
> > see performance impact.
> > 
> > Suppose limiting `can_queue` is the right way to fix this bug
> > (https://bugzilla.redhat.com/show_bug.cgi?id=1315013), do we really
> > need to eliminate scsi_change_queue_depth() at the same time
> > though?
> 
> No, I thought it was no longer needed, assuming the slave would
> inherit the host's value, but I was mistaken, the slave will default
> to a queue_depth of 1 if not specified.

Just to improve understanding: shost->can_queue is the host limit: it's
the maximum number of commands that can be outstanding on the host at
any one time, which is why it's used as the count for the shared tag
map.  It's also a fixed limit and can't be altered after the host is
instantiated.  sdev->queue_depth is the device queue limit, it can be
altered dynamically during device operation.  There's no relation
between them except that if you make queue_depth larger than can_queue,
the latter rules.

James