| Message ID | 20190313165933.6961-1-andriy.shevchenko@linux.intel.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | [v1] ACPI / platform: Unbind, trim and unregister staled devices | expand |
On Wed, Mar 13, 2019 at 5:59 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > When the commit 68bdb6773289 > > ("ACPI: add support for ACPI reconfiguration notifiers") > > introduced reconfiguration notifiers it misses the point that the ACPI table, > which may be loaded and then unloded via ConfigFS, can contain devices that are > not enumerated by their parents. > > In such case the stale platform device is dangling in the system while the rest > of the devices from the same table are already gone. > > Introduce acpi_trim_stale_platform_devices() function that iterates over staled > platform devices, unbinds and trims its ACPI companions and then unregisters > the physical device itself. > > Cc: Mika Westerberg <mika.westerberg@linux.intel.com> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > drivers/acpi/acpi_platform.c | 47 ++++++++++++++++++++++++++++++++++++ > drivers/acpi/scan.c | 7 +++--- > include/linux/acpi.h | 2 ++ > 3 files changed, 53 insertions(+), 3 deletions(-) > > diff --git a/drivers/acpi/acpi_platform.c b/drivers/acpi/acpi_platform.c > index 1f32caa87686..529ef9621e4a 100644 > --- a/drivers/acpi/acpi_platform.c > +++ b/drivers/acpi/acpi_platform.c > @@ -133,3 +133,50 @@ struct platform_device *acpi_create_platform_device(struct acpi_device *adev, > return pdev; > } > EXPORT_SYMBOL_GPL(acpi_create_platform_device); > + > +static int trim_stale_platform_device(struct device *dev, void *context) > +{ > + struct acpi_device *adev = ACPI_COMPANION(dev); > + int ret; > + > + /* Skip devices enumerated by parent */ > + if (!adev || adev->flags.enumeration_by_parent) > + return 0; > + > + /* Skip devices that are sharing ACPI companion except primary one */ > + if (acpi_get_first_physical_node(adev) != dev) > + return 0; > + > + /* Skip alive devices */ > + ret = acpi_bus_get_status(adev); > + if (!ret) > + return 0; > + > + /* > + * We have to call acpi_unbind_one() explicitly to avoid asynchronous > + * call in device_platform_notify() against staled data. > + */ I'm not sure if I understand the comment. What's the exact scenario you are concerned about? Also, wouldn't platform device unregistration before calling acpi_bus_trim() work? > + acpi_unbind_one(dev); > + acpi_bus_trim(adev); > + > + dev_dbg(&adev->dev, "trimming platform device %s\n", dev_name(dev)); > + platform_device_unregister(to_platform_device(dev)); > + return 0; > +} > + > +/** > + * acpi_trim_stale_platform_devices - remove platform devices that are gone > + * @pdev: platform device to start walking the hierarchy from > + * > + * Iterate over platform devices that have ACPI companion and represent > + * physical node and check if the device is gone. In such case unbind its > + * ACPI companion and trim, then unregister physical device itself. > + */ > +int acpi_trim_stale_platform_devices(struct platform_device *pdev) > +{ > + struct device *dev = pdev ? &pdev->dev : NULL; > + struct bus_type *bus = &platform_bus_type; > + > + return bus_for_each_dev(bus, dev, NULL, trim_stale_platform_device); > +} > +EXPORT_SYMBOL_GPL(acpi_trim_stale_platform_devices); > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c > index 5efd4219f112..4151181676dd 100644 > --- a/drivers/acpi/scan.c > +++ b/drivers/acpi/scan.c > @@ -2291,6 +2291,10 @@ static void acpi_table_events_fn(struct work_struct *work) > acpi_scan_lock_acquire(); > acpi_bus_scan(ACPI_ROOT_OBJECT); > acpi_scan_lock_release(); > + } else if (tew->event == ACPI_TABLE_EVENT_UNLOAD) { > + acpi_scan_lock_acquire(); > + acpi_trim_stale_platform_devices(NULL); > + acpi_scan_lock_release(); > } The lock can be acquired around the entire if () statement. Also, it might be slightly cleaner to use a switch () statement here. > > kfree(tew); > @@ -2303,9 +2307,6 @@ void acpi_scan_table_handler(u32 event, void *table, void *context) > if (!acpi_scan_initialized) > return; > > - if (event != ACPI_TABLE_EVENT_LOAD) > - return; > - > tew = kmalloc(sizeof(*tew), GFP_KERNEL); > if (!tew) > return; > diff --git a/include/linux/acpi.h b/include/linux/acpi.h > index 87715f20b69a..98a02f770d49 100644 > --- a/include/linux/acpi.h > +++ b/include/linux/acpi.h > @@ -608,6 +608,8 @@ void acpi_walk_dep_device_list(acpi_handle handle); > > struct platform_device *acpi_create_platform_device(struct acpi_device *, > struct property_entry *); > +int acpi_trim_stale_platform_devices(struct platform_device *pdev); > + > #define ACPI_PTR(_ptr) (_ptr) > > static inline void acpi_device_set_enumerated(struct acpi_device *adev) > -- > 2.20.1 >
diff --git a/drivers/acpi/acpi_platform.c b/drivers/acpi/acpi_platform.c index 1f32caa87686..529ef9621e4a 100644 --- a/drivers/acpi/acpi_platform.c +++ b/drivers/acpi/acpi_platform.c @@ -133,3 +133,50 @@ struct platform_device *acpi_create_platform_device(struct acpi_device *adev, return pdev; } EXPORT_SYMBOL_GPL(acpi_create_platform_device); + +static int trim_stale_platform_device(struct device *dev, void *context) +{ + struct acpi_device *adev = ACPI_COMPANION(dev); + int ret; + + /* Skip devices enumerated by parent */ + if (!adev || adev->flags.enumeration_by_parent) + return 0; + + /* Skip devices that are sharing ACPI companion except primary one */ + if (acpi_get_first_physical_node(adev) != dev) + return 0; + + /* Skip alive devices */ + ret = acpi_bus_get_status(adev); + if (!ret) + return 0; + + /* + * We have to call acpi_unbind_one() explicitly to avoid asynchronous + * call in device_platform_notify() against staled data. + */ + acpi_unbind_one(dev); + acpi_bus_trim(adev); + + dev_dbg(&adev->dev, "trimming platform device %s\n", dev_name(dev)); + platform_device_unregister(to_platform_device(dev)); + return 0; +} + +/** + * acpi_trim_stale_platform_devices - remove platform devices that are gone + * @pdev: platform device to start walking the hierarchy from + * + * Iterate over platform devices that have ACPI companion and represent + * physical node and check if the device is gone. In such case unbind its + * ACPI companion and trim, then unregister physical device itself. + */ +int acpi_trim_stale_platform_devices(struct platform_device *pdev) +{ + struct device *dev = pdev ? &pdev->dev : NULL; + struct bus_type *bus = &platform_bus_type; + + return bus_for_each_dev(bus, dev, NULL, trim_stale_platform_device); +} +EXPORT_SYMBOL_GPL(acpi_trim_stale_platform_devices); diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index 5efd4219f112..4151181676dd 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -2291,6 +2291,10 @@ static void acpi_table_events_fn(struct work_struct *work) acpi_scan_lock_acquire(); acpi_bus_scan(ACPI_ROOT_OBJECT); acpi_scan_lock_release(); + } else if (tew->event == ACPI_TABLE_EVENT_UNLOAD) { + acpi_scan_lock_acquire(); + acpi_trim_stale_platform_devices(NULL); + acpi_scan_lock_release(); } kfree(tew); @@ -2303,9 +2307,6 @@ void acpi_scan_table_handler(u32 event, void *table, void *context) if (!acpi_scan_initialized) return; - if (event != ACPI_TABLE_EVENT_LOAD) - return; - tew = kmalloc(sizeof(*tew), GFP_KERNEL); if (!tew) return; diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 87715f20b69a..98a02f770d49 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -608,6 +608,8 @@ void acpi_walk_dep_device_list(acpi_handle handle); struct platform_device *acpi_create_platform_device(struct acpi_device *, struct property_entry *); +int acpi_trim_stale_platform_devices(struct platform_device *pdev); + #define ACPI_PTR(_ptr) (_ptr) static inline void acpi_device_set_enumerated(struct acpi_device *adev)
When the commit 68bdb6773289 ("ACPI: add support for ACPI reconfiguration notifiers") introduced reconfiguration notifiers it misses the point that the ACPI table, which may be loaded and then unloded via ConfigFS, can contain devices that are not enumerated by their parents. In such case the stale platform device is dangling in the system while the rest of the devices from the same table are already gone. Introduce acpi_trim_stale_platform_devices() function that iterates over staled platform devices, unbinds and trims its ACPI companions and then unregisters the physical device itself. Cc: Mika Westerberg <mika.westerberg@linux.intel.com> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/acpi/acpi_platform.c | 47 ++++++++++++++++++++++++++++++++++++ drivers/acpi/scan.c | 7 +++--- include/linux/acpi.h | 2 ++ 3 files changed, 53 insertions(+), 3 deletions(-)