Re: [PATCH] powerpc/pseries: remove returning ENODEV when uevent is triggered
Hi Michael, After checking the definition of modalias in modalias_show(), I think it's better to keep the same logic in vio_hotplug(), that's removing the else part in my original patch shown below. + 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); I think we can avoid some possible regression then. I'll make the change in my v2 patch. -- Regards, Lidong Zhong On Wed, Apr 10, 2024 at 9:25 AM Lidong Zhong wrote: > Hi Michael, > > Thanks for your reply. > > On Tue, Apr 9, 2024 at 4:46 PM Michael Ellerman > 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 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
Re: [PATCH] powerpc/pseries: remove returning ENODEV when uevent is triggered
Hi Michael, Thanks for your reply. On Tue, Apr 9, 2024 at 4:46 PM Michael Ellerman 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 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;
Re: [PATCH] powerpc/pseries: remove returning ENODEV when uevent is triggered
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 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. > 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. > +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
Re: [PATCH] powerpc/pseries: remove returning ENODEV when uevent is triggered
Hi Michael, Could you share your opinion about this patch please? Thanks, Lidong On Sat, Mar 23, 2024 at 4:47 PM Lidong Zhong wrote: > 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. The above annoying errors can also be removed > after the patch applied. > > Signed-off-by: Lidong Zhong > --- > arch/powerpc/platforms/pseries/vio.c | 9 + > 1 file changed, 5 insertions(+), 4 deletions(-) > > 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); > +else > + add_uevent_var(env, "MODALIAS=vio:T%sS%s", vio_dev->type, > cp); > +out: > return 0; > } > > -- > 2.35.3 > > -- Regards, Lidong Zhong
[PATCH] powerpc/pseries: remove returning ENODEV when uevent is triggered
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. The above annoying errors can also be removed after the patch applied. Signed-off-by: Lidong Zhong --- arch/powerpc/platforms/pseries/vio.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) 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); +else + add_uevent_var(env, "MODALIAS=vio:T%sS%s", vio_dev->type, cp); +out: return 0; } -- 2.35.3
