| Message ID | 20200623191749.115200-6-tasleson@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add persistent durable identifier to storage log messages | expand |
Hi Tony, On 7/9/20 11:18 PM, Tony Asleson wrote: > Hi Bartlomiej, > > On 6/24/20 5:35 AM, Bartlomiej Zolnierkiewicz wrote: >> The root source of problem is that libata transport uses different >> naming scheme for ->tdev devices (please see dev_set_name() in >> ata_t{dev,link,port}_add()) than libata core for its logging >> functionality (ata_{dev,link,port}_printk()). >> >> Since libata transport is part of sysfs ABI we should be careful >> to not break it so one idea for solving the issue is to convert >> ata_t{dev,link,port}_add() to use libata logging naming scheme and >> at the same time add sysfs symlinks for the old libata transport >> naming scheme. > > I tried doing as you suggested. I've included what I've done so far. I > haven't been able to get all the needed parts for the symlinks to > maintain compatibility. > > The /sys/class/.. seems OK, eg. > > $ ls -x -w 70 /sys/class/ata_[dl]* > /sys/class/ata_device: > ata1.00 ata2.00 ata3.00 ata4.00 ata5.00 ata6.00 ata7.00 > ata7.01 ata8.00 ata8.01 dev1.0 dev2.0 dev3.0 dev4.0 > dev5.0 dev6.0 dev7.0 dev7.1 dev8.0 dev8.1 > > /sys/class/ata_link: > ata1 ata2 ata3 ata4 ata5 ata6 ata7 ata8 link1 link2 > link3 link4 link5 link6 link7 link8 > > but the implementation is a hack, see device.h, core.c changes. There > must be a better way? > > Also I'm missing part of the full path, eg. > > /sys/devices/pci0000:00/0000:00:01.1/ata7/link7/dev7.0/ata_device/dev7.0/gscr > > becomes > > /sys/devices/pci0000:00/0000:00:01.1/ata7/ata7/ata7.01/ata_device/ata7.01/gscr > > but the compatibility symlinks added only get me to > > /sys/devices/pci0000:00/0000:00:01.1/ata7/link7/dev7.0/ata_device/ > > I haven't found the right spot to get the last symlink included. > > If you or anyone else has suggestions to correct the incomplete symlink > and/or correct the implementation to set the > /sys/class/ata_device it would be greatly appreciated. Unfortunately I know too little about sysfs class-es handling to help (I've added linux-{scsi,block} MLs & Greg to Cc:). PS If libata support turns out to be blocking the patchset progress you may consider concentrating on getting SCSI & NVME support merged first.. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics > Thanks, > -Tony > > diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c > index beca5f91bb4c..2aa230ad813e 100644 > --- a/drivers/ata/libata-core.c > +++ b/drivers/ata/libata-core.c > @@ -6444,7 +6444,7 @@ void ata_port_printk(const struct ata_port *ap, > const char *level, > vaf.fmt = fmt; > vaf.va = &args; > > - printk("%sata%u: %pV", level, ap->print_id, &vaf); > + dev_printk(level, &ap->tdev, "%pV", &vaf); > > va_end(args); > } > @@ -6461,12 +6461,7 @@ void ata_link_printk(const struct ata_link *link, > const char *level, > vaf.fmt = fmt; > vaf.va = &args; > > - if (sata_pmp_attached(link->ap) || link->ap->slave_link) > - printk("%sata%u.%02u: %pV", > - level, link->ap->print_id, link->pmp, &vaf); > - else > - printk("%sata%u: %pV", > - level, link->ap->print_id, &vaf); > + dev_printk(level, &link->tdev, "%pV", &vaf); > > va_end(args); > } > @@ -6483,9 +6478,7 @@ void ata_dev_printk(const struct ata_device *dev, > const char *level, > vaf.fmt = fmt; > vaf.va = &args; > > - printk("%sata%u.%02u: %pV", > - level, dev->link->ap->print_id, dev->link->pmp + dev->devno, > - &vaf); > + dev_printk(level, &dev->tdev,"%pV", &vaf); > > va_end(args); > } > diff --git a/drivers/ata/libata-transport.c b/drivers/ata/libata-transport.c > index 6a40e3c6cf49..e5870f9b2b21 100644 > --- a/drivers/ata/libata-transport.c > +++ b/drivers/ata/libata-transport.c > @@ -288,7 +288,7 @@ int ata_tport_add(struct device *parent, > dev->parent = parent; > ata_host_get(ap->host); > dev->release = ata_tport_release; > - dev_set_name(dev, "ata%d", ap->print_id); > + dev_set_name(dev, "ata%u", ap->print_id); > transport_setup_device(dev); > ata_acpi_bind_port(ap); > error = device_add(dev); > @@ -374,6 +374,46 @@ static int ata_tlink_match(struct > attribute_container *cont, > return &i->link_attr_cont.ac == cont; > } > > +static struct device* tlink_to_symlink(struct device *dev) { > + struct ata_internal* i = > to_ata_internal(ata_scsi_transport_template); > + return > attribute_container_find_class_device(&i->link_attr_cont.ac, dev); > +} > + > +void ata_tlink_symlink_add_del(struct ata_link *link, int add) { > + struct device *dev = &link->tdev; > + struct ata_port *ap = link->ap; > + char lname[64]; > + struct device *devp = tlink_to_symlink(dev); > + > + if (ata_is_host_link(link)) { > + snprintf(lname, sizeof(lname), > + "link%d", ap->print_id); > + } else { > + snprintf(lname, sizeof(lname), > + "link%d.%d", ap->print_id, link->pmp); > + } > + > + if (add) { > + int e; > + > + e = device_add_class_symlinks_additional(devp, lname); > + if (e) { > + dev_warn(devp, "Error %d tlink symlink > class\n", e); > + } > + > + e = sysfs_create_link(&dev->parent->kobj, &dev->kobj, > lname); > + if (e) { > + dev_warn(dev, "Error %d tlink symlink\n", e); > + } > + } else { > + sysfs_delete_link(&dev->parent->kobj, &dev->kobj, lname); > + device_delete_class_symlinks_additional(devp, lname); > + } > +} > + > +#define ata_tlink_symlink_add(x) ata_tlink_symlink_add_del(x, 1) > +#define ata_tlink_symlink_del(x) ata_tlink_symlink_add_del(x, 0) > + > /** > * ata_tlink_delete -- remove ATA LINK > * @port: ATA LINK to remove > @@ -389,6 +429,7 @@ void ata_tlink_delete(struct ata_link *link) > ata_tdev_delete(ata_dev); > } > > + ata_tlink_symlink_del(link); > transport_remove_device(dev); > device_del(dev); > transport_destroy_device(dev); > @@ -415,9 +456,9 @@ int ata_tlink_add(struct ata_link *link) > dev->parent = &ap->tdev; > dev->release = ata_tlink_release; > if (ata_is_host_link(link)) > - dev_set_name(dev, "link%d", ap->print_id); > - else > - dev_set_name(dev, "link%d.%d", ap->print_id, link->pmp); > + dev_set_name(dev, "ata%u", ap->print_id); > + else > + dev_set_name(dev, "ata%u.%02u", ap->print_id, link->pmp); > > transport_setup_device(dev); > > @@ -429,6 +470,8 @@ int ata_tlink_add(struct ata_link *link) > transport_add_device(dev); > transport_configure_device(dev); > > + ata_tlink_symlink_add(link); > + > ata_for_each_dev(ata_dev, link, ALL) { > error = ata_tdev_add(ata_dev); > if (error) { > @@ -440,6 +483,7 @@ int ata_tlink_add(struct ata_link *link) > while (--ata_dev >= link->device) { > ata_tdev_delete(ata_dev); > } > + ata_tlink_symlink_del(link); > transport_remove_device(dev); > device_del(dev); > tlink_err: > @@ -630,6 +674,44 @@ static void ata_tdev_free(struct ata_device *dev) > put_device(&dev->tdev); > } > > +static struct device* tdev_to_symlink(struct device *dev) { > + struct ata_internal* i = > to_ata_internal(ata_scsi_transport_template); > + return > attribute_container_find_class_device(&i->dev_attr_cont.ac, dev); > +} > + > +void ata_tdev_symlink_add_del(struct ata_device *ata_dev, int add) { > + struct device *dev = &ata_dev->tdev; > + struct ata_link *link = ata_dev->link; > + struct ata_port *ap = link->ap; > + char dname[64]; > + > + struct device *devp = tdev_to_symlink(dev); > + > + if (ata_is_host_link(link)) > + snprintf(dname, sizeof(dname), "dev%d.%d", > ap->print_id,ata_dev->devno); > + else > + snprintf(dname, sizeof(dname), "dev%d.%d.0", > ap->print_id, link->pmp); > + > + if (add) { > + int e; > + e = device_add_class_symlinks_additional(devp, dname); > + if (e) { > + dev_warn(devp, "Error %d tdev symlink class\n", e); > + } > + > + e = sysfs_create_link(&dev->parent->kobj, &dev->kobj, > dname); > + if (e) { > + dev_warn(dev, "Error %d tdev symlink\n", e); > + } > + } else { > + sysfs_delete_link(&dev->parent->kobj, &dev->kobj, dname); > + device_delete_class_symlinks_additional(devp, dname); > + } > +} > + > +#define ata_tdev_symlink_add(x) ata_tdev_symlink_add_del(x, 1) > +#define ata_tdev_symlink_del(x) ata_tdev_symlink_add_del(x, 0) > + > /** > * ata_tdev_delete -- remove ATA device > * @port: ATA PORT to remove > @@ -640,6 +722,7 @@ static void ata_tdev_delete(struct ata_device *ata_dev) > { > struct device *dev = &ata_dev->tdev; > > + ata_tdev_symlink_del(ata_dev); > transport_remove_device(dev); > device_del(dev); > ata_tdev_free(ata_dev); > @@ -665,10 +748,8 @@ static int ata_tdev_add(struct ata_device *ata_dev) > device_initialize(dev); > dev->parent = &link->tdev; > dev->release = ata_tdev_release; > - if (ata_is_host_link(link)) > - dev_set_name(dev, "dev%d.%d", ap->print_id,ata_dev->devno); > - else > - dev_set_name(dev, "dev%d.%d.0", ap->print_id, link->pmp); > + > + dev_set_name(dev, "ata%u.%02u", ap->print_id, link->pmp + > ata_dev->devno); > > transport_setup_device(dev); > ata_acpi_bind_dev(ata_dev); > @@ -680,6 +761,8 @@ static int ata_tdev_add(struct ata_device *ata_dev) > > transport_add_device(dev); > transport_configure_device(dev); > + > + ata_tdev_symlink_add(ata_dev); > return 0; > } > > diff --git a/drivers/base/core.c b/drivers/base/core.c > index c2439d12608d..bc060000837c 100644 > --- a/drivers/base/core.c > +++ b/drivers/base/core.c > @@ -2269,6 +2269,16 @@ static int device_add_class_symlinks(struct > device *dev) > return error; > } > > +int device_add_class_symlinks_additional(struct device *dev, char *name) { > + return sysfs_create_link(&dev->class->p->subsys.kobj, > &dev->kobj, name); > +} > +EXPORT_SYMBOL_GPL(device_add_class_symlinks_additional); > + > +void device_delete_class_symlinks_additional(struct device *dev, char > *name) { > + sysfs_delete_link(&dev->class->p->subsys.kobj, &dev->kobj, name); > +} > +EXPORT_SYMBOL_GPL(device_delete_class_symlinks_additional); > + > static void device_remove_class_symlinks(struct device *dev) > { > if (dev_of_node(dev)) > diff --git a/include/linux/device.h b/include/linux/device.h > index 281755404c21..4827d86116ab 100644 > --- a/include/linux/device.h > +++ b/include/linux/device.h > @@ -656,6 +656,10 @@ static inline const char *dev_name(const struct > device *dev) > extern __printf(2, 3) > int dev_set_name(struct device *dev, const char *name, ...); > > + > +int device_add_class_symlinks_additional(struct device *dev, char *name); > +void device_delete_class_symlinks_additional(struct device *dev, char > *name); > + > int dev_durable_name(const struct device *d, char *buffer, size_t len); > > #ifdef CONFIG_NUMA >
On Tue, Jul 14, 2020 at 10:06:05AM +0200, Bartlomiej Zolnierkiewicz wrote: > > Hi Tony, > > On 7/9/20 11:18 PM, Tony Asleson wrote: > > Hi Bartlomiej, > > > > On 6/24/20 5:35 AM, Bartlomiej Zolnierkiewicz wrote: > >> The root source of problem is that libata transport uses different > >> naming scheme for ->tdev devices (please see dev_set_name() in > >> ata_t{dev,link,port}_add()) than libata core for its logging > >> functionality (ata_{dev,link,port}_printk()). > >> > >> Since libata transport is part of sysfs ABI we should be careful > >> to not break it so one idea for solving the issue is to convert > >> ata_t{dev,link,port}_add() to use libata logging naming scheme and > >> at the same time add sysfs symlinks for the old libata transport > >> naming scheme. Given the age of the current implementation, what suddenly broke that requires this to change at this point in time? > > > > I tried doing as you suggested. I've included what I've done so far. I > > haven't been able to get all the needed parts for the symlinks to > > maintain compatibility. > > > > The /sys/class/.. seems OK, eg. > > > > $ ls -x -w 70 /sys/class/ata_[dl]* > > /sys/class/ata_device: > > ata1.00 ata2.00 ata3.00 ata4.00 ata5.00 ata6.00 ata7.00 > > ata7.01 ata8.00 ata8.01 dev1.0 dev2.0 dev3.0 dev4.0 > > dev5.0 dev6.0 dev7.0 dev7.1 dev8.0 dev8.1 > > > > /sys/class/ata_link: > > ata1 ata2 ata3 ata4 ata5 ata6 ata7 ata8 link1 link2 > > link3 link4 link5 link6 link7 link8 A link class? Ick ick ick. > > but the implementation is a hack, see device.h, core.c changes. There > > must be a better way? > > > > Also I'm missing part of the full path, eg. > > > > /sys/devices/pci0000:00/0000:00:01.1/ata7/link7/dev7.0/ata_device/dev7.0/gscr > > > > becomes > > > > /sys/devices/pci0000:00/0000:00:01.1/ata7/ata7/ata7.01/ata_device/ata7.01/gscr > > > > but the compatibility symlinks added only get me to > > > > /sys/devices/pci0000:00/0000:00:01.1/ata7/link7/dev7.0/ata_device/ > > > > I haven't found the right spot to get the last symlink included. > > > > If you or anyone else has suggestions to correct the incomplete symlink > > and/or correct the implementation to set the > > /sys/class/ata_device it would be greatly appreciated. I can't understand what you are trying to do here. What do you want to represent in sysfs with a symlink that you can't just have in a single sysfs file like "name" or "new_name" or "name_because_we_didnt_think_about_this_10_years_ago" that shows you the other "name" that you are trying to look up here? Why abuse symlinks like this at all? And no, the device.h and core.c changes aren't ok :) thanks, greg k-h
On 7/14/20 10:17 AM, Greg Kroah-Hartman wrote: > On Tue, Jul 14, 2020 at 10:06:05AM +0200, Bartlomiej Zolnierkiewicz wrote: >> >> Hi Tony, >> >> On 7/9/20 11:18 PM, Tony Asleson wrote: >>> Hi Bartlomiej, >>> >>> On 6/24/20 5:35 AM, Bartlomiej Zolnierkiewicz wrote: >>>> The root source of problem is that libata transport uses different >>>> naming scheme for ->tdev devices (please see dev_set_name() in >>>> ata_t{dev,link,port}_add()) than libata core for its logging >>>> functionality (ata_{dev,link,port}_printk()). >>>> >>>> Since libata transport is part of sysfs ABI we should be careful >>>> to not break it so one idea for solving the issue is to convert >>>> ata_t{dev,link,port}_add() to use libata logging naming scheme and >>>> at the same time add sysfs symlinks for the old libata transport >>>> naming scheme. > > Given the age of the current implementation, what suddenly broke that > requires this to change at this point in time? Unfortunately when adding libata transport classes (+ at the same time embedding struct device-s in libata dev/link/port objects) in the past someone has decided to use different naming scheme than the one used for standard libata log messages (which use printk() without any reference to struct device-s in libata dev/link/port objects). Now we would like to use dev_printk() for standard libata logging functionality as this is required for 2 pending patchsets: - move DPRINTK to dynamic debugging (from Hannes Reinecke) - add persistent durable identifier storage log messages (from Tony) but we don't want to change standard libata log messages and confuse users.. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics
On Tue, Jul 14, 2020 at 10:50:39AM +0200, Bartlomiej Zolnierkiewicz wrote: > > On 7/14/20 10:17 AM, Greg Kroah-Hartman wrote: > > On Tue, Jul 14, 2020 at 10:06:05AM +0200, Bartlomiej Zolnierkiewicz wrote: > >> > >> Hi Tony, > >> > >> On 7/9/20 11:18 PM, Tony Asleson wrote: > >>> Hi Bartlomiej, > >>> > >>> On 6/24/20 5:35 AM, Bartlomiej Zolnierkiewicz wrote: > >>>> The root source of problem is that libata transport uses different > >>>> naming scheme for ->tdev devices (please see dev_set_name() in > >>>> ata_t{dev,link,port}_add()) than libata core for its logging > >>>> functionality (ata_{dev,link,port}_printk()). > >>>> > >>>> Since libata transport is part of sysfs ABI we should be careful > >>>> to not break it so one idea for solving the issue is to convert > >>>> ata_t{dev,link,port}_add() to use libata logging naming scheme and > >>>> at the same time add sysfs symlinks for the old libata transport > >>>> naming scheme. > > > > Given the age of the current implementation, what suddenly broke that > > requires this to change at this point in time? > > Unfortunately when adding libata transport classes (+ at the same > time embedding struct device-s in libata dev/link/port objects) in > the past someone has decided to use different naming scheme than > the one used for standard libata log messages (which use printk() > without any reference to struct device-s in libata dev/link/port > objects). > > Now we would like to use dev_printk() for standard libata logging > functionality as this is required for 2 pending patchsets: > > - move DPRINTK to dynamic debugging (from Hannes Reinecke) > > - add persistent durable identifier storage log messages (from Tony) > > but we don't want to change standard libata log messages and > confuse users.. All of that mess with symlinks just for a common debug printk? That seems excessive :) Just use the device name and don't worry about it, I doubt anyone will notice, unless the name is _really_ different. thanks, greg k-h
On 7/17/20 12:06 PM, Greg Kroah-Hartman wrote: > On Tue, Jul 14, 2020 at 10:50:39AM +0200, Bartlomiej Zolnierkiewicz wrote: >> >> On 7/14/20 10:17 AM, Greg Kroah-Hartman wrote: >>> On Tue, Jul 14, 2020 at 10:06:05AM +0200, Bartlomiej Zolnierkiewicz wrote: >>>> >>>> Hi Tony, >>>> >>>> On 7/9/20 11:18 PM, Tony Asleson wrote: >>>>> Hi Bartlomiej, >>>>> >>>>> On 6/24/20 5:35 AM, Bartlomiej Zolnierkiewicz wrote: >>>>>> The root source of problem is that libata transport uses different >>>>>> naming scheme for ->tdev devices (please see dev_set_name() in >>>>>> ata_t{dev,link,port}_add()) than libata core for its logging >>>>>> functionality (ata_{dev,link,port}_printk()). >>>>>> >>>>>> Since libata transport is part of sysfs ABI we should be careful >>>>>> to not break it so one idea for solving the issue is to convert >>>>>> ata_t{dev,link,port}_add() to use libata logging naming scheme and >>>>>> at the same time add sysfs symlinks for the old libata transport >>>>>> naming scheme. >>> >>> Given the age of the current implementation, what suddenly broke that >>> requires this to change at this point in time? >> >> Unfortunately when adding libata transport classes (+ at the same >> time embedding struct device-s in libata dev/link/port objects) in >> the past someone has decided to use different naming scheme than >> the one used for standard libata log messages (which use printk() >> without any reference to struct device-s in libata dev/link/port >> objects). >> >> Now we would like to use dev_printk() for standard libata logging >> functionality as this is required for 2 pending patchsets: >> >> - move DPRINTK to dynamic debugging (from Hannes Reinecke) >> >> - add persistent durable identifier storage log messages (from Tony) >> >> but we don't want to change standard libata log messages and >> confuse users.. > > All of that mess with symlinks just for a common debug printk? That > seems excessive :) > > Just use the device name and don't worry about it, I doubt anyone will > notice, unless the name is _really_ different. > Good luck. I tried (cf patchset 'ata: kill ATA_DEBUG') but got rejected for exactly this reason. Cheers, Hannes -- Dr. Hannes Reinecke Kernel Storage Architect hare@suse.de +49 911 74053 688 SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg HRB 36809 (AG Nürnberg), GF: Felix Imendörffer
On 7/17/20 12:06 PM, Greg Kroah-Hartman wrote: > On Tue, Jul 14, 2020 at 10:50:39AM +0200, Bartlomiej Zolnierkiewicz wrote: >> >> On 7/14/20 10:17 AM, Greg Kroah-Hartman wrote: >>> On Tue, Jul 14, 2020 at 10:06:05AM +0200, Bartlomiej Zolnierkiewicz wrote: >>>> >>>> Hi Tony, >>>> >>>> On 7/9/20 11:18 PM, Tony Asleson wrote: >>>>> Hi Bartlomiej, >>>>> >>>>> On 6/24/20 5:35 AM, Bartlomiej Zolnierkiewicz wrote: >>>>>> The root source of problem is that libata transport uses different >>>>>> naming scheme for ->tdev devices (please see dev_set_name() in >>>>>> ata_t{dev,link,port}_add()) than libata core for its logging >>>>>> functionality (ata_{dev,link,port}_printk()). >>>>>> >>>>>> Since libata transport is part of sysfs ABI we should be careful >>>>>> to not break it so one idea for solving the issue is to convert >>>>>> ata_t{dev,link,port}_add() to use libata logging naming scheme and >>>>>> at the same time add sysfs symlinks for the old libata transport >>>>>> naming scheme. >>> >>> Given the age of the current implementation, what suddenly broke that >>> requires this to change at this point in time? >> >> Unfortunately when adding libata transport classes (+ at the same >> time embedding struct device-s in libata dev/link/port objects) in >> the past someone has decided to use different naming scheme than >> the one used for standard libata log messages (which use printk() >> without any reference to struct device-s in libata dev/link/port >> objects). >> >> Now we would like to use dev_printk() for standard libata logging >> functionality as this is required for 2 pending patchsets: >> >> - move DPRINTK to dynamic debugging (from Hannes Reinecke) >> >> - add persistent durable identifier storage log messages (from Tony) >> >> but we don't want to change standard libata log messages and >> confuse users.. > > All of that mess with symlinks just for a common debug printk? That > seems excessive :) Unfortunately "a common debug printk" means all log messages generated by libata core.. > Just use the device name and don't worry about it, I doubt anyone will > notice, unless the name is _really_ different. Well, Geert has noticed and complained pretty quickly: https://lore.kernel.org/linux-ide/alpine.DEB.2.21.2003241414490.21582@ramsan.of.borg/ Anyway, I don't insist that hard on keeping the old names and I won't be the one handling potential bug-reports.. (added Jens to Cc:). Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics
On 7/17/20 5:27 AM, Bartlomiej Zolnierkiewicz wrote: > > On 7/17/20 12:06 PM, Greg Kroah-Hartman wrote: > >> Just use the device name and don't worry about it, I doubt anyone will >> notice, unless the name is _really_ different. > > Well, Geert has noticed and complained pretty quickly: > > https://lore.kernel.org/linux-ide/alpine.DEB.2.21.2003241414490.21582@ramsan.of.borg/ > > Anyway, I don't insist that hard on keeping the old names and > I won't be the one handling potential bug-reports.. (added Jens to Cc:). I would think having sysfs use one naming convention and the logging using another would be confusing for users, but apparently they've managed this long with that. It appears changes are being rejected because of logging content differences, implying we shouldn't be changing printk usage to dev_printk. Should I re-work my changes to support dev_printk path in addition to utilizing printk_emit functionality so that we can avoid user space visible log changes? I don't see a way to make the transition from printk to dev_printk without having user visible changes to message content. Thanks -Tony
On 7/17/20 9:47 PM, Tony Asleson wrote: > On 7/17/20 5:27 AM, Bartlomiej Zolnierkiewicz wrote: >> >> On 7/17/20 12:06 PM, Greg Kroah-Hartman wrote: >> >>> Just use the device name and don't worry about it, I doubt anyone will >>> notice, unless the name is _really_ different. >> >> Well, Geert has noticed and complained pretty quickly: >> >> https://lore.kernel.org/linux-ide/alpine.DEB.2.21.2003241414490.21582@ramsan.of.borg/ >> >> Anyway, I don't insist that hard on keeping the old names and >> I won't be the one handling potential bug-reports.. (added Jens to Cc:). > > I would think having sysfs use one naming convention and the logging > using another would be confusing for users, but apparently they've > managed this long with that. > > > It appears changes are being rejected because of logging content > differences, implying we shouldn't be changing printk usage to dev_printk. > > Should I re-work my changes to support dev_printk path in addition to > utilizing printk_emit functionality so that we can avoid user space Unfortunately this won't fix the issue for Hannes' patchset. > visible log changes? I don't see a way to make the transition from > printk to dev_printk without having user visible changes to message content. The usage of sysfs symlinks for fixing the naming issue turned out to not be (easily) possible so we should consider other options (or just go forward with the original dev_printk() conversion).. Jens? Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index beca5f91bb4c..44c874367fe3 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -6475,6 +6475,7 @@ EXPORT_SYMBOL(ata_link_printk); void ata_dev_printk(const struct ata_device *dev, const char *level, const char *fmt, ...) { + const struct device *gendev; struct va_format vaf; va_list args; @@ -6483,9 +6484,12 @@ void ata_dev_printk(const struct ata_device *dev, const char *level, vaf.fmt = fmt; vaf.va = &args; - printk("%sata%u.%02u: %pV", - level, dev->link->ap->print_id, dev->link->pmp + dev->devno, - &vaf); + gendev = (dev->sdev) ? &dev->sdev->sdev_gendev : &dev->tdev; + + dev_printk(level, gendev, "ata%u.%02u: %pV", + dev->link->ap->print_id, + dev->link->pmp + dev->devno, + &vaf); va_end(args); }
Utilize the dev_printk function which will add structured data to the log message. Signed-off-by: Tony Asleson <tasleson@redhat.com> --- drivers/ata/libata-core.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)