On 11.06.2019 1:48, James Bottomley wrote:
On Mon, 2019-06-10 at 10:49 +0300, Konstantin Khlebnikov wrote:
On 10.06.2019 0:37, James Bottomley wrote:
On Sat, 2019-06-08 at 17:13 +0300, Konstantin Khlebnikov wrote:
On 08.06.2019 11:25, Christoph Hellwig wrote:> On Fri, Jun 07,
2019
at 10:34:39AM +0300, Konstantin Khlebnikov wrote:
>
> Do we really need to spam dmesg with even more ATA
crap? What
about
> a sysfs file that can be read on demand instead?
>
Makes sense.
Trim state is exposed for ata_device:
/sys/class/ata_device/devX.Y/trim
but there is no link from scsi device to ata device so they
hard to match.
I'll think about it.
Nope. There is no obvious way to link scsi device with
ata_device. ata_device is built on top of "transport_class" and
"attribute_container". This some extremely over engineered sysfs
framework used only in ata/scsi. I don't want to touch this.
You don't need to know any of that. The problem is actually when
the ata transport classes were first created, the devices weren't
properly parented. What should have happened, like every other
transport class, is that the devices should have descended down to
the scsi device as the leaf in an integrated fashion. Instead,
what we seem to have is three completely separate trees.
So if you look at a SAS device, you see from the pci device:
host2/port-2:0/end_device-2:0/target2:0:0/2:0:0:0/block/sdb/sdb1
But if you look at a SATA device, you see three separate paths:
ata3/host3/target3\:0\:0/3\:0\:0\:0/block/sda/sda1
ata3/link3/dev3.0/ata_device/dev3.0
ata3/ata_port/ata3
Instead of an integrated tree
Unfortunately, this whole thing is unfixable now. If I integrate
the tree properly, the separate port and link directories will get
subsumed and we won't be able to recover them with judicious
linking so scripts relying on them will break. The best we can
probably do is add additional links with what we have.
To follow the way we usually do it, there should be a link from the
ata device to the scsi target, but that wouldn't help you find the
"trim" files, so it sounds like you want a link from the scsi
device to the ata device, which would?
Yes, I'm talking about link from scsi device to leaf ata_device node.
In libata scsi_device has one to one relation with ata_device.
So making link like /sys/class/block/sda/device/ata_device should be
possible easy.
But I haven't found implicit reference from struct ata_device to
ata_device in sysfs.
If that's all you want, it is pretty simple modulo the fact we can only
get at the tdev, not the lower transport device, which is what you
want, but at least it's linear from the symlink.
The attached patch should do this.
Now I see this for my non-sas device:
# ls -l /sys/class/scsi_device/3\:0\:0\:0/device/ata_device
lrwxrwxrwx 1 root root 0 Jun 10 13:39
/sys/class/scsi_device/3:0:0:0/device/ata_device -> ../../../link3/dev3.0
I've tried this too. Such link is not very useful,
because attribute 'trim' is deeper and suffix path isn't constant:
/sys/class/block/sda/device/ata_device/ata_device/dev1.0/trim
while I expect something like
/sys/class/block/sda/device/ata_device/trim
James
---
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 391ac0503dc0..b644336a6d65 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -4576,7 +4576,20 @@ void ata_scsi_scan_host(struct ata_port *ap, int sync)
sdev = __scsi_add_device(ap->scsi_host, channel, id, 0,
NULL);
if (!IS_ERR(sdev)) {
+ int r;
+
dev->sdev = sdev;
+ /* this is a sysfs stupidity: we don't
+ * care if the link actually gets
+ * created: there's no error handling
+ * for failure and we continue on
+ * regardless, but we have to discard
+ * the return value like this to
+ * defeat unused result checking */
+ r = sysfs_create_link(&sdev->sdev_gendev.kobj,
+ &dev->tdev.kobj,
+ "ata_device");
+ (void)r;
scsi_device_put(sdev);
} else {
dev->sdev = NULL;
@@ -4703,6 +4716,7 @@ static void ata_scsi_remove_dev(struct ata_device *dev)
ata_dev_info(dev, "detaching (SCSI %s)\n",
dev_name(&sdev->sdev_gendev));
+ sysfs_remove_link(&sdev->sdev_gendev.kobj, "ata_device");
scsi_remove_device(sdev);
scsi_device_put(sdev);
}
