Hi Michael, Thanks for your reply.
On Tue, Apr 9, 2024 at 4:46 PM Michael Ellerman <[email protected]> wrote: > Hi Lidong, > > Thanks for the patch. > > I'm not an expert on udev etc. so apologies if any of these questions > are stupid. > > Lidong Zhong <[email protected]> writes: > > We have noticed the following nuisance messages during boot > > > > [ 7.120610][ T1060] vio vio: uevent: failed to send synthetic uevent > > [ 7.122281][ T1060] vio 4000: uevent: failed to send synthetic uevent > > [ 7.122304][ T1060] vio 4001: uevent: failed to send synthetic uevent > > [ 7.122324][ T1060] vio 4002: uevent: failed to send synthetic uevent > > [ 7.122345][ T1060] vio 4004: uevent: failed to send synthetic uevent > > > > It's caused by either vio_register_device_node() failed to set > dev->of_node or > > the missing "compatible" property. Try return as much information as > possible > > instead of a failure. > > Does udev etc. cope with that? Can we just change the content of the > MODALIAS value like that? > > With this patch we'll start emitting uevents for devices we previously > didn't. I guess that's OK because nothing is expecting them? > > Can you include a log of udev showing the event firing and that nothing > breaks. > > On my system here I see nothing matches the devices except for libvpd, > which seems to match lots of things. > It's an issue reported by our customer. I am sorry I can't provide more information because I don't have the environment to reproduce this issue. The only related log I got is shown below: Feb 07 14:08:03 rb3i0060 udevadm[623]: vio: Failed to write 'add' to '/sys/devices/vio/uevent', ignoring: No such device Feb 07 14:08:03 rb3i0060 kernel: synth uevent: /devices/vio: failed to send uevent Feb 07 14:08:03 rb3i0060 kernel: vio vio: uevent: failed to send synthetic uevent Feb 07 14:08:03 rb3i0060 kernel: synth uevent: /devices/vio/4000: failed to send uevent Feb 07 14:08:03 rb3i0060 kernel: vio 4000: uevent: failed to send synthetic uevent Feb 07 14:08:03 rb3i0060 kernel: synth uevent: /devices/vio/4001: failed to send uevent Feb 07 14:08:03 rb3i0060 kernel: vio 4001: uevent: failed to send synthetic uevent Feb 07 14:08:03 rb3i0060 kernel: synth uevent: /devices/vio/4002: failed to send uevent Feb 07 14:08:03 rb3i0060 kernel: vio 4002: uevent: failed to send synthetic uevent Feb 07 14:08:03 rb3i0060 kernel: synth uevent: /devices/vio/4004: failed to send uevent Feb 07 14:08:03 rb3i0060 kernel: vio 4004: uevent: failed to send synthetic uevent Feb 07 14:08:03 rb3i0060 udevadm[623]: 4000: Failed to write 'add' to '/sys/devices/vio/4000/uevent', ignoring: No such device Feb 07 14:08:03 rb3i0060 udevadm[623]: 4001: Failed to write 'add' to '/sys/devices/vio/4001/uevent', ignoring: No such device Feb 07 14:08:03 rb3i0060 udevadm[623]: 4002: Failed to write 'add' to '/sys/devices/vio/4002/uevent', ignoring: No such device Feb 07 14:08:03 rb3i0060 udevadm[623]: 4004: Failed to write 'add' to '/sys/devices/vio/4004/uevent', ignoring: No such device systemd-udev-trigger service calls 'udevadm trigger --type=devices --action=add' and kernel returns -ENODEV because either dev->of_node is NULL or 'compatible' property is not present. Similar cases were already reported after some search, for example https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1827162 https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1845319 I don't think it causes real problems but confusion to users. > > diff --git a/arch/powerpc/platforms/pseries/vio.c > b/arch/powerpc/platforms/pseries/vio.c > > index 90ff85c879bf..62961715ca24 100644 > > --- a/arch/powerpc/platforms/pseries/vio.c > > +++ b/arch/powerpc/platforms/pseries/vio.c > > @@ -1593,12 +1593,13 @@ static int vio_hotplug(const struct device *dev, > struct kobj_uevent_env *env) > > > > dn = dev->of_node; > > if (!dn) > > - return -ENODEV; > > + goto out; > > cp = of_get_property(dn, "compatible", NULL); > > if (!cp) > > - return -ENODEV; > > - > > - add_uevent_var(env, "MODALIAS=vio:T%sS%s", vio_dev->type, cp); > > + add_uevent_var(env, "MODALIAS=vio:T%s", vio_dev->type); > > If it's OK to skip the compatible property then we don't need the > of_node at all, and we could always emit this, even when of_node is not > available. > You mean something like this? @@ -1592,13 +1592,10 @@ static int vio_hotplug(const struct device *dev, struct kobj_uevent_env *env) const char *cp; dn = dev->of_node; - if (!dn) - return -ENODEV; - cp = of_get_property(dn, "compatible", NULL); - if (!cp) - return -ENODEV; - - add_uevent_var(env, "MODALIAS=vio:T%sS%s", vio_dev->type, cp); + if (dn && (cp = of_get_property(dn, "compatible", NULL)) + add_uevent_var(env, "MODALIAS=vio:T%sS%s", vio_dev->type, cp); + else + add_uevent_var(env, "MODALIAS=vio:T%s", vio_dev->type); return 0; > > > + else > > + add_uevent_var(env, "MODALIAS=vio:T%sS%s", vio_dev->type, > cp); > > +out: > > return 0; > > } > > I think we also should update the vio modalias_show() to follow the same > logic, otherwise the uevent MODALIAS value and the modalias file won't > match which is confusing. > > Preferably vio_hotplug() and modalias_show() would just call a common > helper. > > cheers > Thanks for the suggestion. I'll send a v2 patch. -- Regards, Lidong Zhong
