Re: [PATCH v3] ata: libata-scsi: Use correct device no in ata_find_dev()

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

 



On 2023/5/29 14:45, Damien Le Moal wrote:
On 5/23/23 18:23, Jason Yan wrote:
On 2023/5/23 16:29, John Garry wrote:
On 23/05/2023 09:04, Damien Le Moal wrote:
For devices not attached to a port multiplier and managed directly by
libata, the device number passed to ata_find_dev() must always be lower
than the maximum number of devices returned by ata_link_max_devices().
That is 1 for SATA devices or 2 for an IDE link with master+slave
devices. This device number is the scsi device ID which matches these
constraint as the ID are generated per port and so never exceed the
link maximum.

However, for libsas managed devices, scsi device IDs are assigned per
scsi host, leading to device IDs for SATA devices that can be well in
excess of libata per-link maximum number of devices. This results in
ata_find_dev() always returning NULL for libsas managed devices except
for the first device of the host with ID (device number) 0. This issue
is visible by executing hdparm command, which fails:

hdparm -i /dev/sdX
/dev/sdX:
    HDIO_GET_IDENTITY failed: No message of desired type

Fix this by rewriting ata_find_dev() to ignore the device number for
non-pmp attached devices with a link with at most 1 device, that is SATA
devices on SATA ports. For these, device number 0 is always used to
return the correct ata_device struct of the port link. This change
excludes IDE master/slave setups (maximum number of devices per link
is 2) and port-multiplier attached devices. Also, to be consistant with
the fact that scsi device IDs and channel numbers used as device numbers
are both unsigned int, change the devno argument of ata_find_dev() to
unsinged int.

Reported-by: Xingui Yang <yangxingui@xxxxxxxxxx>
Fixes: 41bda9c98035 ("libata-link: update hotplug to handle PMP links")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Damien Le Moal <dlemoal@xxxxxxxxxx>

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

Hi Damien & John,

I think we may missed something. What about if we do this:

echo "scsi add-single-device 0 0 100 0" >/proc/scsi/scsi

Then in ata_scsi_user_scan() we will return device "0 0 0 0 " and rescan
this device, which is wrong?

I did some more digging into this. And I do not think there are any
issues (I tested and it works). The reason is that the "shost" passed to
ata_scsi_user_scan() corresponds to the scsi host for the ata_port of
the ata_device in libsas (dev->sata_dev.ap). It is not the scsi_host
representing the HBA itself (which has multiple devices).

So changing the devno to 0 has no effect and deleting+rescanning
particular devices leads to the correct IDs being used. E.g., on my
system, I have 4 drives connected to the pm80xx:

# lsscsi -g
[0:0:0:0]    disk    ATA      WDC  WUH721818AL W232  /dev/sdc   /dev/sg3
[0:0:1:0]    disk    ATA      WDC  WUH721818AL WTW2  /dev/sdb   /dev/sg1
[0:0:2:0]    disk    ATA      WDC  WUH722222AL Wf86  /dev/sdd   /dev/sg2
[0:0:3:0]    zbc     ATA      WDC  WSH722020AL W803  /dev/sda   /dev/sg0

If I remove the first 0 and 2:

# echo 1 > /sys/class/scsi_device/0:0:0:0/device/delete
# echo 1 > /sys/class/scsi_device/0:0:2:0/device/delete
# lsscsi -g
[0:0:1:0]    disk    ATA      WDC  WUH721818AL WTW2  /dev/sdb   /dev/sg1
[0:0:3:0]    zbc     ATA      WDC  WSH722020AL W803  /dev/sda   /dev/sg0

And then manually rescan in reverse order of the removal:

# echo "scsi add-single-device 0 0 2 0" > /proc/scsi/scsi
# lsscsi -g
[0:0:1:0]    disk    ATA      WDC  WUH721818AL WTW2  /dev/sdb   /dev/sg1
[0:0:2:0]    disk    ATA      WDC  WUH722222AL Wf86  /dev/sdc   /dev/sg2
[0:0:3:0]    zbc     ATA      WDC  WSH722020AL W803  /dev/sda   /dev/sg0

# echo "scsi add-single-device 0 0 0 0" > /proc/scsi/scsi
# lsscsi -g
[0:0:0:0]    disk    ATA      WDC  WUH721818AL W232  /dev/sdd   /dev/sg3
[0:0:1:0]    disk    ATA      WDC  WUH721818AL WTW2  /dev/sdb   /dev/sg1
[0:0:2:0]    disk    ATA      WDC  WUH722222AL Wf86  /dev/sdc   /dev/sg2
[0:0:3:0]    zbc     ATA      WDC  WSH722020AL W803  /dev/sda   /dev/sg0

I get back all devices with the correct IDs.

Note that I tried John's suggestion as well using an ata_for_each_dev()
loop. That does work well for libsas as we do have ata_dev->sdev set to
the scsi device already when scanning, but does not work for AHCI as we
do not. I will keep doing some more tests with v3 (and correct typos and
suggested) but I think this is all good.

Thanks to clarify this:

Reviewed-by: Jason Yan <yanaijie@xxxxxxxxxx>



[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