Re: [PATCH v2 2/2] ata: libata-sata: Fix device queue depth control

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 27/09/2022 16:03, Damien Le Moal wrote:
On 9/27/22 20:51, John Garry wrote:
On 26/09/2022 00:08, Damien Le Moal wrote:
The function __ata_change_queue_depth() uses the helper
ata_scsi_find_dev() to get the ata_device structure of a scsi device and
set that device maximum queue depth. However, when the ata device is
managed by libsas, ata_scsi_find_dev() returns NULL, turning
__ata_change_queue_depth() into a nop, which prevents the user from
setting the maximum queue depth of ATA devices used with libsas based
HBAs.

Fix this by renaming __ata_change_queue_depth() to
ata_change_queue_depth() and adding a pointer to the ata_device
structure of the target device as argument. This pointer is provided by
ata_scsi_change_queue_depth() using ata_scsi_find_dev() in the case of
a libata managed device and by sas_change_queue_depth() using
sas_to_ata_dev() in the case of a libsas managed ata device.

Signed-off-by: Damien Le Moal <damien.lemoal@xxxxxxxxxxxxxxxxxx>

Tested-by: John Garry <john.garry@xxxxxxxxxx>

However - a big however - I will note that this following behaviour is
strange for a SATA device for libsas:

root@(none)$ echo 33 > 0:0:2:0/device/queue_depth
root@(none)$ echo 33 > 0:0:2:0/device/queue_depth
sh: echo: write error: Invalid argument
root@(none)$

Weird. I am not getting any error (pm80xx driver). The qd gets capped at
32 as expected. Is it something that changes per libsas driver ?

That is with my pm8001 card.

What happens here is that the first store of 33 gets through to ata_change_queue_depth() as it does not exceed the SAS shost can_queue, which is >> 32, and then we cap this to 32 and store it in sdev->queue_depth. And then the 2nd store of 33 also gets through, but this following expression not evaluate true in ata_change_queue_depth():

queue_depth < 1 || queue_depth == sdev->queue_depth

So we don't return. However the following subsequent test does evaluate true in ata_change_queue_depth():

if (sdev->queue_depth == queue_depth)
	return -EINVAL;

And we error.


I also note that setting a value out of range is just rejected for a SAS
device, and not capped to the max range (like it is for SATA).

Not sure where that come from. A quick look does not reveal anything
obvious. Need to dig into that one.

AHCI rejects out of range values it as it exceeds the shost can_queue in
sdev_store_queue_depth().

Indeed:

echo 1 > /sys/block/sdk/device/queue_depth
echo 33 > /sys/block/sdk/device/queue_depth

Hmmmm... why no error message? is the printk silenced?

cat /sys/block/sdk/device/queue_depth
1

But for the libsas SATA device:

echo 1 > /sys/block/sdd/device/queue_depth
cat /sys/block/sdd/device/queue_depth
1
echo 33 > /sys/block/sdd/device/queue_depth
cat /sys/block/sdd/device/queue_depth
32

As one would expect... > Need to dig into that one.


thanks,
John




[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux