Re: [Regression] acpi: laptop panics early in boot

["Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>]

[-- Attachment #1: Type: text/plain, Size: 3315 bytes --]

Hello Rafael,

On Fri, Mar 24, 2023 at 01:12:35PM +0100, Rafael J. Wysocki wrote:
> On Thu, Mar 23, 2023 at 10:22 PM Uwe Kleine-König
> <u.kleine-koenig@pengutronix.de> wrote:
> >
> > Hello Pierre,
> >
> > On Tue, Mar 07, 2023 at 09:08:55PM +0100, Uwe Kleine-König wrote:
> > > On Tue, Mar 07, 2023 at 02:31:49PM -0500, Pierre Asselin wrote:
> > > > > Maybe the following patch helps (on top of v6.3-rc1):
> > > > >
> > > > > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> > > > > index 9531dd0fef50..a5a8f82981ce 100644
> > > > > --- a/drivers/acpi/bus.c
> > > > > +++ b/drivers/acpi/bus.c
> > > > > @@ -518,7 +518,7 @@ static void acpi_bus_notify(acpi_handle handle, u32
> > > > > type, void *data)
> > > > >   if (!adev)
> > > > >           goto err;
> > > > >
> > > > > - if (adev->dev.driver) {
> > > > > + if (device_is_bound(&adev->dev)) {
> > > > >           struct acpi_driver *driver = to_acpi_driver(adev->dev.driver);
> > > > >
> > > > >           if (driver && driver->ops.notify &&
> > > > >
> > > >
> > > > It does indeed "fix" 6.3-rc1.  Modulo locking issues, which I am
> > > > not qualified to evaluate.
> > >
> > > Thanks for your prompt test feedback.
> > >
> > > The locked variant could look as follows:
> > >
> > > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> > > index 9531dd0fef50..fddca263ac40 100644
> > > --- a/drivers/acpi/bus.c
> > > +++ b/drivers/acpi/bus.c
> > > @@ -518,13 +518,15 @@ static void acpi_bus_notify(acpi_handle handle, u32 type, void *data)
> > >       if (!adev)
> > >               goto err;
> > >
> > > -     if (adev->dev.driver) {
> > > +     device_lock(&adev->dev);
> > > +     if (device_is_bound(&adev->dev)) {
> > >               struct acpi_driver *driver = to_acpi_driver(adev->dev.driver);
> > >
> > >               if (driver && driver->ops.notify &&
> > >                   (driver->flags & ACPI_DRIVER_ALL_NOTIFY_EVENTS))
> > >                       driver->ops.notify(adev, type);
> > >       }
> > > +     device_unlock(&adev->dev);
> > >
> > >       if (!hotplug_event) {
> > >               acpi_put_acpi_dev(adev);
> > >
> > > Pierre: If you want to test that, I suggest to also enable
> > > PROVE_LOCKING.
> > >
> > > Rafael: I don't know if this would work and I hope you're able to judge
> > > this better than I do. The change without the lock is for sure better
> > > than the status quo.
> > >
> > > I did a similar conversion as the blamed commit for pci that got
> > > reverted for similiar reasons. See
> > > 68da4e0eaaab421f228eac57cbe7505b136464af. (Added Bjorn and Robert to
> > > Cc:) I think the pci code suffers from a similar race even after
> > > reverting my change. If someone is able to find the right fix for one of
> > > them, that might be transferable to the other?!
> >
> > This mail didn't make it into the archive on lore.kernel.org. Did
> > someone receive it?
> 
> I have received it.

I bounced it to the list and Pierre. This was enough to make it appear
in the archive on lore.kernel.org. If you got it yesterday, that was my
bounce.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]