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