2021-01-22 12:58:20

by Kai-Heng Feng

[permalink] [raw]
Subject: [PATCH] ACPI / device_sysfs: Prefer "compatible" modalias

Commit 8765c5ba1949 ("ACPI / scan: Rework modalias creation when
"compatible" is present") may create two "MODALIAS=" in uevent file if
conditions are met.

This breaks systemd-udevd, which assumes each "key" in uevent file is
unique. The internal implementation of systemd-udevd overwrites the
first MODALIAS with the second one, so its kmod rule doesn't load driver
for the first MODALIAS.

So if both ACPI modalias and OF modalias are present, use the latter
one to ensure there's only one MODALIAS.

Reference: https://github.com/systemd/systemd/pull/18163
Cc: AceLan Kao <[email protected]>
Cc: "Rafael J. Wysocki" <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Andy Shevchenko <[email protected]>
Suggested-by: Mika Westerberg <[email protected]>
Fixes: 8765c5ba1949 ("ACPI / scan: Rework modalias creation when "compatible" is present")
Signed-off-by: Kai-Heng Feng <[email protected]>
---
drivers/acpi/device_sysfs.c | 20 ++++++--------------
1 file changed, 6 insertions(+), 14 deletions(-)

diff --git a/drivers/acpi/device_sysfs.c b/drivers/acpi/device_sysfs.c
index 96869f1538b9..bfca116482b8 100644
--- a/drivers/acpi/device_sysfs.c
+++ b/drivers/acpi/device_sysfs.c
@@ -251,20 +251,12 @@ int __acpi_device_uevent_modalias(struct acpi_device *adev,
if (add_uevent_var(env, "MODALIAS="))
return -ENOMEM;

- len = create_pnp_modalias(adev, &env->buf[env->buflen - 1],
- sizeof(env->buf) - env->buflen);
- if (len < 0)
- return len;
-
- env->buflen += len;
- if (!adev->data.of_compatible)
- return 0;
-
- if (len > 0 && add_uevent_var(env, "MODALIAS="))
- return -ENOMEM;
-
- len = create_of_modalias(adev, &env->buf[env->buflen - 1],
- sizeof(env->buf) - env->buflen);
+ if (adev->data.of_compatible)
+ len = create_of_modalias(adev, &env->buf[env->buflen - 1],
+ sizeof(env->buf) - env->buflen);
+ else
+ len = create_pnp_modalias(adev, &env->buf[env->buflen - 1],
+ sizeof(env->buf) - env->buflen);
if (len < 0)
return len;

--
2.29.2


2021-01-22 13:09:17

by Mika Westerberg

[permalink] [raw]
Subject: Re: [PATCH] ACPI / device_sysfs: Prefer "compatible" modalias

On Fri, Jan 22, 2021 at 08:53:02PM +0800, Kai-Heng Feng wrote:
> Commit 8765c5ba1949 ("ACPI / scan: Rework modalias creation when
> "compatible" is present") may create two "MODALIAS=" in uevent file if
> conditions are met.
>
> This breaks systemd-udevd, which assumes each "key" in uevent file is
> unique. The internal implementation of systemd-udevd overwrites the
> first MODALIAS with the second one, so its kmod rule doesn't load driver
> for the first MODALIAS.
>
> So if both ACPI modalias and OF modalias are present, use the latter
> one to ensure there's only one MODALIAS.
>
> Reference: https://github.com/systemd/systemd/pull/18163
> Cc: AceLan Kao <[email protected]>
> Cc: "Rafael J. Wysocki" <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Andy Shevchenko <[email protected]>
> Suggested-by: Mika Westerberg <[email protected]>
> Fixes: 8765c5ba1949 ("ACPI / scan: Rework modalias creation when "compatible" is present")
> Signed-off-by: Kai-Heng Feng <[email protected]>

Reviewed-by: Mika Westerberg <[email protected]>

2021-01-22 13:10:26

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] ACPI / device_sysfs: Prefer "compatible" modalias

On Fri, Jan 22, 2021 at 08:53:02PM +0800, Kai-Heng Feng wrote:
> Commit 8765c5ba1949 ("ACPI / scan: Rework modalias creation when
> "compatible" is present") may create two "MODALIAS=" in uevent file if
> conditions are met.
>
> This breaks systemd-udevd, which assumes each "key" in uevent file is
> unique. The internal implementation of systemd-udevd overwrites the
> first MODALIAS with the second one, so its kmod rule doesn't load driver
> for the first MODALIAS.
>
> So if both ACPI modalias and OF modalias are present, use the latter
> one to ensure there's only one MODALIAS.
>
> Reference: https://github.com/systemd/systemd/pull/18163
> Cc: AceLan Kao <[email protected]>
> Cc: "Rafael J. Wysocki" <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Andy Shevchenko <[email protected]>
> Suggested-by: Mika Westerberg <[email protected]>
> Fixes: 8765c5ba1949 ("ACPI / scan: Rework modalias creation when "compatible" is present")
> Signed-off-by: Kai-Heng Feng <[email protected]>
> ---
> drivers/acpi/device_sysfs.c | 20 ++++++--------------
> 1 file changed, 6 insertions(+), 14 deletions(-)

Thanks for fixing this up!

Reviewed-by: Greg Kroah-Hartman <[email protected]>

2021-01-26 01:37:19

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH] ACPI / device_sysfs: Prefer "compatible" modalias

On Fri, Jan 22, 2021 at 2:06 PM Greg Kroah-Hartman
<[email protected]> wrote:
>
> On Fri, Jan 22, 2021 at 08:53:02PM +0800, Kai-Heng Feng wrote:
> > Commit 8765c5ba1949 ("ACPI / scan: Rework modalias creation when
> > "compatible" is present") may create two "MODALIAS=" in uevent file if
> > conditions are met.
> >
> > This breaks systemd-udevd, which assumes each "key" in uevent file is
> > unique. The internal implementation of systemd-udevd overwrites the
> > first MODALIAS with the second one, so its kmod rule doesn't load driver
> > for the first MODALIAS.
> >
> > So if both ACPI modalias and OF modalias are present, use the latter
> > one to ensure there's only one MODALIAS.
> >
> > Reference: https://github.com/systemd/systemd/pull/18163
> > Cc: AceLan Kao <[email protected]>
> > Cc: "Rafael J. Wysocki" <[email protected]>
> > Cc: Greg Kroah-Hartman <[email protected]>
> > Cc: Andy Shevchenko <[email protected]>
> > Suggested-by: Mika Westerberg <[email protected]>
> > Fixes: 8765c5ba1949 ("ACPI / scan: Rework modalias creation when "compatible" is present")
> > Signed-off-by: Kai-Heng Feng <[email protected]>
> > ---
> > drivers/acpi/device_sysfs.c | 20 ++++++--------------
> > 1 file changed, 6 insertions(+), 14 deletions(-)
>
> Thanks for fixing this up!
>
> Reviewed-by: Greg Kroah-Hartman <[email protected]>

Applied as 5.11-rc material with some minor edits in the subject and
changelog and with the tags.

Thanks everyone!