Re: [PATCH] dax: include bus.h for definition of run_dax()
Ben Dooks wrote: > The run_dax() prototype is defined in "bus.h" but drivers/dax/super.c > does not include this. Include bus.h to silece the following sparse > warning: > > drivers/dax/super.c:337:6: warning: symbol 'run_dax' was not declared. Should > it be static? A different version of this fix has already been queued up. https://git.kernel.org/pub/scm/linux/kernel/git/nvdimm/nvdimm.git/commit/?h=libnvdimm-fixes=2d5153526f929838b0912ded26862840f72745f4 Thank you anyway, Ira
Re: [PATCH] nvdimm: make nd_class variable static
Ben Dooks wrote: > The nd_class is not used outside of drivers/nvdimm/bus.c and thus sparse > is generating the following warning. Remove this by making it static: > > drivers/nvdimm/bus.c:28:14: warning: symbol 'nd_class' was not declared. > Should it be static? Reviewed-by: Ira Weiny > Signed-off-by: Ben Dooks > --- > drivers/nvdimm/bus.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/nvdimm/bus.c b/drivers/nvdimm/bus.c > index 954dbc105fc8..5852fe290523 100644 > --- a/drivers/nvdimm/bus.c > +++ b/drivers/nvdimm/bus.c > @@ -25,7 +25,7 @@ > > int nvdimm_major; > static int nvdimm_bus_major; > -struct class *nd_class; > +static struct class *nd_class; > static DEFINE_IDA(nd_ida); > > static int to_nd_device_type(const struct device *dev) > -- > 2.39.2 >
[PATCH v5 10/10] acpi/thermal: Move handler installing logic to driver
Currently logic for installing notifications from ACPI devices is implemented using notify callback in struct acpi_driver. Preparations are being made to replace acpi_driver with more generic struct platform_driver, which doesn't contain notify callback. Furthermore as of now handlers are being called indirectly through acpi_notify_device(), which decreases performance. Call acpi_dev_install_notify_handler() at the end of .add() callback. Call acpi_dev_remove_notify_handler() at the beginning of .remove() callback. Change arguments passed to the notify function to match with what's required by acpi_install_notify_handler(). Remove .notify callback initialization in acpi_driver. While at it, fix whitespaces in .remove() callback and move tz assignment upwards. Suggested-by: Rafael J. Wysocki Signed-off-by: Michal Wilczynski --- drivers/acpi/thermal.c | 28 ++-- 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c index f9f6ebb08fdb..84716e4b967c 100644 --- a/drivers/acpi/thermal.c +++ b/drivers/acpi/thermal.c @@ -825,9 +825,12 @@ static void acpi_queue_thermal_check(struct acpi_thermal *tz) queue_work(acpi_thermal_pm_queue, >thermal_check_work); } -static void acpi_thermal_notify(struct acpi_device *device, u32 event) +static void acpi_thermal_notify(acpi_handle handle, u32 event, void *data) { - struct acpi_thermal *tz = acpi_driver_data(device); + struct acpi_device *device = data; + struct acpi_thermal *tz; + + tz = acpi_driver_data(device); if (!tz) return; @@ -997,11 +1000,20 @@ static int acpi_thermal_add(struct acpi_device *device) pr_info("%s [%s] (%ld C)\n", acpi_device_name(device), acpi_device_bid(device), deci_kelvin_to_celsius(tz->temperature)); - goto end; + result = acpi_dev_install_notify_handler(device, +ACPI_DEVICE_NOTIFY, +acpi_thermal_notify); + if (result) + goto flush_wq_and_unregister; + + return 0; + +flush_wq_and_unregister: + flush_workqueue(acpi_thermal_pm_queue); + acpi_thermal_unregister_thermal_zone(tz); free_memory: kfree(tz); -end: return result; } @@ -1012,10 +1024,15 @@ static void acpi_thermal_remove(struct acpi_device *device) if (!device || !acpi_driver_data(device)) return; - flush_workqueue(acpi_thermal_pm_queue); tz = acpi_driver_data(device); + acpi_dev_remove_notify_handler(device, + ACPI_DEVICE_NOTIFY, + acpi_thermal_notify); + + flush_workqueue(acpi_thermal_pm_queue); acpi_thermal_unregister_thermal_zone(tz); + kfree(tz); } @@ -1078,7 +1095,6 @@ static struct acpi_driver acpi_thermal_driver = { .ops = { .add = acpi_thermal_add, .remove = acpi_thermal_remove, - .notify = acpi_thermal_notify, }, .drv.pm = _thermal_pm, }; -- 2.41.0
[PATCH v5 09/10] acpi/nfit: Move handler installing logic to driver
Currently logic for installing notifications from ACPI devices is implemented using notify callback in struct acpi_driver. Preparations are being made to replace acpi_driver with more generic struct platform_driver, which doesn't contain notify callback. Furthermore as of now handlers are being called indirectly through acpi_notify_device(), which decreases performance. Call acpi_dev_install_notify_handler() at the end of .add() callback. Call acpi_dev_remove_notify_handler() at the beginning of .remove() callback. Change arguments passed to the notify function to match with what's required by acpi_install_notify_handler(). Remove .notify callback initialization in acpi_driver. Suggested-by: Rafael J. Wysocki Signed-off-by: Michal Wilczynski --- drivers/acpi/nfit/core.c | 24 ++-- 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c index 95930e9d776c..a281bdfee8a0 100644 --- a/drivers/acpi/nfit/core.c +++ b/drivers/acpi/nfit/core.c @@ -3312,11 +3312,13 @@ void acpi_nfit_shutdown(void *data) } EXPORT_SYMBOL_GPL(acpi_nfit_shutdown); -static void acpi_nfit_notify(struct acpi_device *adev, u32 event) +static void acpi_nfit_notify(acpi_handle handle, u32 event, void *data) { - device_lock(>dev); - __acpi_nfit_notify(>dev, adev->handle, event); - device_unlock(>dev); + struct acpi_device *device = data; + + device_lock(>dev); + __acpi_nfit_notify(>dev, handle, event); + device_unlock(>dev); } static int acpi_nfit_add(struct acpi_device *adev) @@ -3375,12 +3377,23 @@ static int acpi_nfit_add(struct acpi_device *adev) if (rc) return rc; - return devm_add_action_or_reset(dev, acpi_nfit_shutdown, acpi_desc); + + rc = devm_add_action_or_reset(dev, acpi_nfit_shutdown, acpi_desc); + if (rc) + return rc; + + return acpi_dev_install_notify_handler(adev, + ACPI_DEVICE_NOTIFY, + acpi_nfit_notify); } static void acpi_nfit_remove(struct acpi_device *adev) { /* see acpi_nfit_unregister */ + + acpi_dev_remove_notify_handler(adev, + ACPI_DEVICE_NOTIFY, + acpi_nfit_notify); } static void acpi_nfit_update_notify(struct device *dev, acpi_handle handle) @@ -3465,7 +3478,6 @@ static struct acpi_driver acpi_nfit_driver = { .ops = { .add = acpi_nfit_add, .remove = acpi_nfit_remove, - .notify = acpi_nfit_notify, }, }; -- 2.41.0
[PATCH v5 08/10] acpi/nfit: Improve terminator line in acpi_nfit_ids
Currently terminator line contains redunant characters. Remove them and also remove a comma at the end. Signed-off-by: Michal Wilczynski --- drivers/acpi/nfit/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c index aff79cbc2190..95930e9d776c 100644 --- a/drivers/acpi/nfit/core.c +++ b/drivers/acpi/nfit/core.c @@ -3455,7 +3455,7 @@ EXPORT_SYMBOL_GPL(__acpi_nfit_notify); static const struct acpi_device_id acpi_nfit_ids[] = { { "ACPI0012", 0 }, - { "", 0 }, + {} }; MODULE_DEVICE_TABLE(acpi, acpi_nfit_ids); -- 2.41.0
[PATCH v5 07/10] acpi/nfit: Move acpi_nfit_notify() before acpi_nfit_add()
To use new style of installing event handlers acpi_nfit_notify() needs to be known inside acpi_nfit_add(). Move acpi_nfit_notify() upwards in the file, so it can be used inside acpi_nfit_add(). Signed-off-by: Michal Wilczynski --- drivers/acpi/nfit/core.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c index 07204d482968..aff79cbc2190 100644 --- a/drivers/acpi/nfit/core.c +++ b/drivers/acpi/nfit/core.c @@ -3312,6 +3312,13 @@ void acpi_nfit_shutdown(void *data) } EXPORT_SYMBOL_GPL(acpi_nfit_shutdown); +static void acpi_nfit_notify(struct acpi_device *adev, u32 event) +{ + device_lock(>dev); + __acpi_nfit_notify(>dev, adev->handle, event); + device_unlock(>dev); +} + static int acpi_nfit_add(struct acpi_device *adev) { struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER, NULL }; @@ -3446,13 +3453,6 @@ void __acpi_nfit_notify(struct device *dev, acpi_handle handle, u32 event) } EXPORT_SYMBOL_GPL(__acpi_nfit_notify); -static void acpi_nfit_notify(struct acpi_device *adev, u32 event) -{ - device_lock(>dev); - __acpi_nfit_notify(>dev, adev->handle, event); - device_unlock(>dev); -} - static const struct acpi_device_id acpi_nfit_ids[] = { { "ACPI0012", 0 }, { "", 0 }, -- 2.41.0
[PATCH v5 06/10] acpi/hed: Move handler installing logic to driver
Currently logic for installing notifications from ACPI devices is implemented using notify callback in struct acpi_driver. Preparations are being made to replace acpi_driver with more generic struct platform_driver, which doesn't contain notify callback. Furthermore as of now handlers are being called indirectly through acpi_notify_device(), which decreases performance. Call acpi_dev_install_notify_handler() at the end of .add() callback. Call acpi_dev_remove_notify_handler() at the beginning of .remove() callback. Change arguments passed to the notify function to match with what's required by acpi_install_notify_handler(). Remove .notify callback initialization in acpi_driver. Suggested-by: Rafael J. Wysocki Signed-off-by: Michal Wilczynski --- drivers/acpi/hed.c | 17 ++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/drivers/acpi/hed.c b/drivers/acpi/hed.c index 78d44e3fe129..8f54560c6d1c 100644 --- a/drivers/acpi/hed.c +++ b/drivers/acpi/hed.c @@ -42,22 +42,34 @@ EXPORT_SYMBOL_GPL(unregister_acpi_hed_notifier); * it is used by HEST Generic Hardware Error Source with notify type * SCI. */ -static void acpi_hed_notify(struct acpi_device *device, u32 event) +static void acpi_hed_notify(acpi_handle handle, u32 event, void *data) { blocking_notifier_call_chain(_hed_notify_list, 0, NULL); } static int acpi_hed_add(struct acpi_device *device) { + int err; + /* Only one hardware error device */ if (hed_handle) return -EINVAL; hed_handle = device->handle; - return 0; + + err = acpi_dev_install_notify_handler(device, + ACPI_DEVICE_NOTIFY, + acpi_hed_notify); + if (err) + hed_handle = NULL; + + return err; } static void acpi_hed_remove(struct acpi_device *device) { + acpi_dev_remove_notify_handler(device, + ACPI_DEVICE_NOTIFY, + acpi_hed_notify); hed_handle = NULL; } @@ -68,7 +80,6 @@ static struct acpi_driver acpi_hed_driver = { .ops = { .add = acpi_hed_add, .remove = acpi_hed_remove, - .notify = acpi_hed_notify, }, }; module_acpi_driver(acpi_hed_driver); -- 2.41.0
[PATCH v5 05/10] acpi/battery: Move handler installing logic to driver
Currently logic for installing notifications from ACPI devices is implemented using notify callback in struct acpi_driver. Preparations are being made to replace acpi_driver with more generic struct platform_driver, which doesn't contain notify callback. Furthermore as of now handlers are being called indirectly through acpi_notify_device(), which decreases performance. Call acpi_dev_install_notify_handler() at the end of .add() callback. Call acpi_dev_remove_notify_handler() at the beginning of .remove() callback. Change arguments passed to the notify function to match with what's required by acpi_install_notify_handler(). Remove .notify callback initialization in acpi_driver. While at it, fix lack of whitespaces in .remove() callback. Suggested-by: Rafael J. Wysocki Signed-off-by: Michal Wilczynski --- drivers/acpi/battery.c | 30 -- 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c index 9c67ed02d797..6337e7b1f6e1 100644 --- a/drivers/acpi/battery.c +++ b/drivers/acpi/battery.c @@ -1034,11 +1034,14 @@ static void acpi_battery_refresh(struct acpi_battery *battery) } /* Driver Interface */ -static void acpi_battery_notify(struct acpi_device *device, u32 event) +static void acpi_battery_notify(acpi_handle handle, u32 event, void *data) { - struct acpi_battery *battery = acpi_driver_data(device); + struct acpi_device *device = data; + struct acpi_battery *battery; struct power_supply *old; + battery = acpi_driver_data(device); + if (!battery) return; old = battery->bat; @@ -1212,13 +1215,23 @@ static int acpi_battery_add(struct acpi_device *device) device_init_wakeup(>dev, 1); - return result; + result = acpi_dev_install_notify_handler(device, +ACPI_ALL_NOTIFY, +acpi_battery_notify); + if (result) + goto fail_deinit_wakup_and_unregister; + + return 0; +fail_deinit_wakup_and_unregister: + device_init_wakeup(>dev, 0); + unregister_pm_notifier(>pm_nb); fail: sysfs_remove_battery(battery); mutex_destroy(>lock); mutex_destroy(>sysfs_lock); kfree(battery); + return result; } @@ -1228,10 +1241,17 @@ static void acpi_battery_remove(struct acpi_device *device) if (!device || !acpi_driver_data(device)) return; - device_init_wakeup(>dev, 0); + battery = acpi_driver_data(device); + + acpi_dev_remove_notify_handler(device, + ACPI_ALL_NOTIFY, + acpi_battery_notify); + + device_init_wakeup(>dev, 0); unregister_pm_notifier(>pm_nb); sysfs_remove_battery(battery); + mutex_destroy(>lock); mutex_destroy(>sysfs_lock); kfree(battery); @@ -1264,11 +1284,9 @@ static struct acpi_driver acpi_battery_driver = { .name = "battery", .class = ACPI_BATTERY_CLASS, .ids = battery_device_ids, - .flags = ACPI_DRIVER_ALL_NOTIFY_EVENTS, .ops = { .add = acpi_battery_add, .remove = acpi_battery_remove, - .notify = acpi_battery_notify, }, .drv.pm = _battery_pm, }; -- 2.41.0
[PATCH v5 04/10] acpi/video: Move handler installing logic to driver
Currently logic for installing notifications from ACPI devices is implemented using notify callback in struct acpi_driver. Preparations are being made to replace acpi_driver with more generic struct platform_driver, which doesn't contain notify callback. Furthermore as of now handlers are being called indirectly through acpi_notify_device(), which decreases performance. Call acpi_dev_install_notify_handler() at the end of .add() callback. Call acpi_dev_remove_notify_handler() at the beginning of .remove() callback. Change arguments passed to the notify function to match with what's required by acpi_install_notify_handler(). Remove .notify callback initialization in acpi_driver. Suggested-by: Rafael J. Wysocki Signed-off-by: Michal Wilczynski --- drivers/acpi/acpi_video.c | 26 ++ 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c index 62f4364e4460..60b7013d0009 100644 --- a/drivers/acpi/acpi_video.c +++ b/drivers/acpi/acpi_video.c @@ -77,7 +77,7 @@ static DEFINE_MUTEX(video_list_lock); static LIST_HEAD(video_bus_head); static int acpi_video_bus_add(struct acpi_device *device); static void acpi_video_bus_remove(struct acpi_device *device); -static void acpi_video_bus_notify(struct acpi_device *device, u32 event); +static void acpi_video_bus_notify(acpi_handle handle, u32 event, void *data); /* * Indices in the _BCL method response: the first two items are special, @@ -104,7 +104,6 @@ static struct acpi_driver acpi_video_bus = { .ops = { .add = acpi_video_bus_add, .remove = acpi_video_bus_remove, - .notify = acpi_video_bus_notify, }, }; @@ -1527,12 +1526,15 @@ static int acpi_video_bus_stop_devices(struct acpi_video_bus *video) acpi_osi_is_win8() ? 0 : 1); } -static void acpi_video_bus_notify(struct acpi_device *device, u32 event) +static void acpi_video_bus_notify(acpi_handle handle, u32 event, void *data) { - struct acpi_video_bus *video = acpi_driver_data(device); + struct acpi_device *device = data; + struct acpi_video_bus *video; struct input_dev *input; int keycode = 0; + video = acpi_driver_data(device); + if (!video || !video->input) return; @@ -2053,8 +2055,20 @@ static int acpi_video_bus_add(struct acpi_device *device) acpi_video_bus_add_notify_handler(video); + error = acpi_dev_install_notify_handler(device, + ACPI_DEVICE_NOTIFY, + acpi_video_bus_notify); + if (error) + goto err_remove_and_unregister_video; + return 0; +err_remove_and_unregister_video: + mutex_lock(_list_lock); + list_del(>entry); + mutex_unlock(_list_lock); + acpi_video_bus_remove_notify_handler(video); + acpi_video_bus_unregister_backlight(video); err_put_video: acpi_video_bus_put_devices(video); kfree(video->attached_array); @@ -2075,6 +2089,10 @@ static void acpi_video_bus_remove(struct acpi_device *device) video = acpi_driver_data(device); + acpi_dev_remove_notify_handler(device, + ACPI_DEVICE_NOTIFY, + acpi_video_bus_notify); + mutex_lock(_list_lock); list_del(>entry); mutex_unlock(_list_lock); -- 2.41.0
[PATCH v5 03/10] acpi/ac: Move handler installing logic to driver
Currently logic for installing notifications from ACPI devices is implemented using notify callback in struct acpi_driver. Preparations are being made to replace acpi_driver with more generic struct platform_driver, which doesn't contain notify callback. Furthermore as of now handlers are being called indirectly through acpi_notify_device(), which decreases performance. Call acpi_dev_install_notify_handler() at the end of .add() callback. Call acpi_dev_remove_notify_handler() at the beginning of .remove() callback. Change arguments passed to the notify function to match with what's required by acpi_install_notify_handler(). Remove .notify callback initialization in acpi_driver. Suggested-by: Rafael J. Wysocki Signed-off-by: Michal Wilczynski --- drivers/acpi/ac.c | 33 - 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c index 1ace70b831cd..207ee3c85bad 100644 --- a/drivers/acpi/ac.c +++ b/drivers/acpi/ac.c @@ -34,7 +34,7 @@ MODULE_LICENSE("GPL"); static int acpi_ac_add(struct acpi_device *device); static void acpi_ac_remove(struct acpi_device *device); -static void acpi_ac_notify(struct acpi_device *device, u32 event); +static void acpi_ac_notify(acpi_handle handle, u32 event, void *data); static const struct acpi_device_id ac_device_ids[] = { {"ACPI0003", 0}, @@ -54,11 +54,9 @@ static struct acpi_driver acpi_ac_driver = { .name = "ac", .class = ACPI_AC_CLASS, .ids = ac_device_ids, - .flags = ACPI_DRIVER_ALL_NOTIFY_EVENTS, .ops = { .add = acpi_ac_add, .remove = acpi_ac_remove, - .notify = acpi_ac_notify, }, .drv.pm = _ac_pm, }; @@ -128,9 +126,12 @@ static enum power_supply_property ac_props[] = { }; /* Driver Model */ -static void acpi_ac_notify(struct acpi_device *device, u32 event) +static void acpi_ac_notify(acpi_handle handle, u32 event, void *data) { - struct acpi_ac *ac = acpi_driver_data(device); + struct acpi_device *device = data; + struct acpi_ac *ac; + + ac = acpi_driver_data(device); if (!ac) return; @@ -235,7 +236,7 @@ static int acpi_ac_add(struct acpi_device *device) result = acpi_ac_get_state(ac); if (result) - goto end; + goto err_release_ac; psy_cfg.drv_data = ac; @@ -248,7 +249,7 @@ static int acpi_ac_add(struct acpi_device *device) >charger_desc, _cfg); if (IS_ERR(ac->charger)) { result = PTR_ERR(ac->charger); - goto end; + goto err_release_ac; } pr_info("%s [%s] (%s)\n", acpi_device_name(device), @@ -256,9 +257,20 @@ static int acpi_ac_add(struct acpi_device *device) ac->battery_nb.notifier_call = acpi_ac_battery_notify; register_acpi_notifier(>battery_nb); -end: + + result = acpi_dev_install_notify_handler(device, +ACPI_ALL_NOTIFY, +acpi_ac_notify); if (result) - kfree(ac); + goto err_unregister; + + return 0; + +err_unregister: + power_supply_unregister(ac->charger); + unregister_acpi_notifier(>battery_nb); +err_release_ac: + kfree(ac); return result; } @@ -297,6 +309,9 @@ static void acpi_ac_remove(struct acpi_device *device) ac = acpi_driver_data(device); + acpi_dev_remove_notify_handler(device, + ACPI_ALL_NOTIFY, + acpi_ac_notify); power_supply_unregister(ac->charger); unregister_acpi_notifier(>battery_nb); -- 2.41.0
[PATCH v5 02/10] acpi/bus: Set driver_data to NULL every time .add() fails
Most drivers set driver_data during .add() callback, but usually they don't set it back to NULL in case of a failure. Set driver_data to NULL in acpi_device_probe() to avoid code duplication. Signed-off-by: Michal Wilczynski --- drivers/acpi/bus.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c index 22468589c551..c1cb570c8d8c 100644 --- a/drivers/acpi/bus.c +++ b/drivers/acpi/bus.c @@ -1017,8 +1017,10 @@ static int acpi_device_probe(struct device *dev) return -ENOSYS; ret = acpi_drv->ops.add(acpi_dev); - if (ret) + if (ret) { + acpi_dev->driver_data = NULL; return ret; + } pr_debug("Driver [%s] successfully bound to device [%s]\n", acpi_drv->name, acpi_dev->pnp.bus_id); -- 2.41.0
[PATCH v5 01/10] acpi/bus: Introduce wrappers for ACPICA event handler install/remove
Introduce new acpi_dev_install_notify_handler() and acpi_dev_remove_notify_handler(). Those functions are replacing old installers, and after all drivers switch to the new model, old installers will be removed. Make acpi_dev_install_notify_handler() and acpi_dev_remove_notify_handler() non-static, and export symbols. This will allow the drivers to call them directly, instead of relying on .notify callback. Suggested-by: Rafael J. Wysocki Signed-off-by: Michal Wilczynski --- drivers/acpi/bus.c | 26 ++ include/acpi/acpi_bus.h | 6 ++ 2 files changed, 32 insertions(+) diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c index b6ab3608d782..22468589c551 100644 --- a/drivers/acpi/bus.c +++ b/drivers/acpi/bus.c @@ -557,6 +557,32 @@ static void acpi_device_remove_notify_handler(struct acpi_device *device, acpi_os_wait_events_complete(); } +int acpi_dev_install_notify_handler(struct acpi_device *adev, + u32 handler_type, + acpi_notify_handler handler) +{ + acpi_status status; + + status = acpi_install_notify_handler(adev->handle, +handler_type, +handler, +adev); + if (ACPI_FAILURE(status)) + return -ENODEV; + + return 0; +} +EXPORT_SYMBOL(acpi_dev_install_notify_handler); + +void acpi_dev_remove_notify_handler(struct acpi_device *adev, + u32 handler_type, + acpi_notify_handler handler) +{ + acpi_remove_notify_handler(adev->handle, handler_type, handler); + acpi_os_wait_events_complete(); +} +EXPORT_SYMBOL(acpi_dev_remove_notify_handler); + /* Handle events targeting \_SB device (at present only graceful shutdown) */ #define ACPI_SB_NOTIFY_SHUTDOWN_REQUEST 0x81 diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h index c941d99162c0..23fbe4a16972 100644 --- a/include/acpi/acpi_bus.h +++ b/include/acpi/acpi_bus.h @@ -515,6 +515,12 @@ void acpi_bus_private_data_handler(acpi_handle, void *); int acpi_bus_get_private_data(acpi_handle, void **); int acpi_bus_attach_private_data(acpi_handle, void *); void acpi_bus_detach_private_data(acpi_handle); +int acpi_dev_install_notify_handler(struct acpi_device *adev, + u32 handler_type, + acpi_notify_handler handler); +void acpi_dev_remove_notify_handler(struct acpi_device *adev, + u32 handler_type, + acpi_notify_handler handler); extern int acpi_notifier_call_chain(struct acpi_device *, u32, u32); extern int register_acpi_notifier(struct notifier_block *); extern int unregister_acpi_notifier(struct notifier_block *); -- 2.41.0
[PATCH v5 00/10] Remove .notify callback in acpi_device_ops
*** IMPORTANT *** This is part 1 - only drivers in acpi directory to ease up review process. Rest of the drivers will be handled in separate patchsets. Currently drivers support ACPI event handlers by defining .notify callback in acpi_device_ops. This solution is suboptimal as event handler installer installs intermediary function acpi_notify_device as a handler in every driver. Also this approach requires extra variable 'flags' for specifying event types that the driver want to subscribe to. Additionally this is a pre-work required to align acpi_driver with platform_driver and eventually replace acpi_driver with platform_driver. Remove .notify callback from the acpi_device_ops. Replace it with each driver installing and removing it's event handlers. v5: - rebased on top of Rafael changes [1], they're not merged yet - fixed rollback in multiple drivers so they don't leak resources on failure - made this part 1, meaning only drivers in acpi directory, rest of the drivers will be handled in separate patchsets to ease up review v4: - added one commit for previously missed driver sony-laptop, refactored return statements, added NULL check for event installer v3: - lkp still reported some failures for eeepc, fujitsu and toshiba_bluetooth, fix those v2: - fix compilation errors for drivers [1]: https://lore.kernel.org/linux-acpi/1847933.atdPhlSkOF@kreacher/ Michal Wilczynski (10): acpi/bus: Introduce wrappers for ACPICA event handler install/remove acpi/bus: Set driver_data to NULL every time .add() fails acpi/ac: Move handler installing logic to driver acpi/video: Move handler installing logic to driver acpi/battery: Move handler installing logic to driver acpi/hed: Move handler installing logic to driver acpi/nfit: Move acpi_nfit_notify() before acpi_nfit_add() acpi/nfit: Improve terminator line in acpi_nfit_ids acpi/nfit: Move handler installing logic to driver acpi/thermal: Move handler installing logic to driver drivers/acpi/ac.c | 33 - drivers/acpi/acpi_video.c | 26 ++ drivers/acpi/battery.c| 30 -- drivers/acpi/bus.c| 30 +- drivers/acpi/hed.c| 17 ++--- drivers/acpi/nfit/core.c | 32 ++-- drivers/acpi/thermal.c| 28 ++-- include/acpi/acpi_bus.h | 6 ++ 8 files changed, 163 insertions(+), 39 deletions(-) -- 2.41.0
[PATCH] nvdimm: make nd_class variable static
The nd_class is not used outside of drivers/nvdimm/bus.c and thus sparse is generating the following warning. Remove this by making it static: drivers/nvdimm/bus.c:28:14: warning: symbol 'nd_class' was not declared. Should it be static? Signed-off-by: Ben Dooks --- drivers/nvdimm/bus.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/nvdimm/bus.c b/drivers/nvdimm/bus.c index 954dbc105fc8..5852fe290523 100644 --- a/drivers/nvdimm/bus.c +++ b/drivers/nvdimm/bus.c @@ -25,7 +25,7 @@ int nvdimm_major; static int nvdimm_bus_major; -struct class *nd_class; +static struct class *nd_class; static DEFINE_IDA(nd_ida); static int to_nd_device_type(const struct device *dev) -- 2.39.2
[PATCH] nvdimm: make security_show static
The security_show function is not used outsid of drivers/nvdimm/dimm_devs.c and the attribute it is for is also already static. Silence the sparse warning for this not being declared by making it static. Fixes: drivers/nvdimm/dimm_devs.c:352:9: warning: symbol 'security_show' was not declared. Should it be static? Signed-off-by: Ben Dooks --- drivers/nvdimm/dimm_devs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c index 957f7c3d17ba..1273873582be 100644 --- a/drivers/nvdimm/dimm_devs.c +++ b/drivers/nvdimm/dimm_devs.c @@ -349,8 +349,8 @@ static ssize_t available_slots_show(struct device *dev, } static DEVICE_ATTR_RO(available_slots); -ssize_t security_show(struct device *dev, - struct device_attribute *attr, char *buf) +static ssize_t security_show(struct device *dev, +struct device_attribute *attr, char *buf) { struct nvdimm *nvdimm = to_nvdimm(dev); -- 2.39.2
Re: [PATCH] nvdimm: make security_show static
On 6/16/23 09:09, Ben Dooks wrote: The security_show function is not used outsid of drivers/nvdimm/dimm_devs.c s/outsid/outside/ and the attribute it is for is also already static. Silence the sparse warning for this not being declared by making it static. Fixes: drivers/nvdimm/dimm_devs.c:352:9: warning: symbol 'security_show' was not declared. Should it be static? Signed-off-by: Ben Dooks Reviewed-by: Dave Jiang --- drivers/nvdimm/dimm_devs.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/nvdimm/dimm_devs.c b/drivers/nvdimm/dimm_devs.c index 957f7c3d17ba..1273873582be 100644 --- a/drivers/nvdimm/dimm_devs.c +++ b/drivers/nvdimm/dimm_devs.c @@ -349,8 +349,8 @@ static ssize_t available_slots_show(struct device *dev, } static DEVICE_ATTR_RO(available_slots); -ssize_t security_show(struct device *dev, - struct device_attribute *attr, char *buf) +static ssize_t security_show(struct device *dev, +struct device_attribute *attr, char *buf) { struct nvdimm *nvdimm = to_nvdimm(dev);
[PATCH] dax: include bus.h for definition of run_dax()
The run_dax() prototype is defined in "bus.h" but drivers/dax/super.c does not include this. Include bus.h to silece the following sparse warning: drivers/dax/super.c:337:6: warning: symbol 'run_dax' was not declared. Should it be static? Signed-off-by: Ben Dooks --- drivers/dax/super.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/dax/super.c b/drivers/dax/super.c index c4c4728a36e4..8c05dae19bfe 100644 --- a/drivers/dax/super.c +++ b/drivers/dax/super.c @@ -14,6 +14,7 @@ #include #include #include "dax-private.h" +#include "bus.h" /** * struct dax_device - anchor object for dax services -- 2.39.2
Re: [PATCH 3/3] dax/kmem: Always enroll hotplugged memory for memmap_on_memory
On 16.06.23 00:00, Vishal Verma wrote: With DAX memory regions originating from CXL memory expanders or NVDIMMs, the kmem driver may be hot-adding huge amounts of system memory on a system without enough 'regular' main memory to support the memmap for it. To avoid this, ensure that all kmem managed hotplugged memory is added with the MHP_MEMMAP_ON_MEMORY flag to place the memmap on the new memory region being hot added. To do this, call add_memory() in chunks of memory_block_size_bytes() as that is a requirement for memmap_on_memory. Additionally, Use the mhp_flag to force the memmap_on_memory checks regardless of the respective module parameter setting. Cc: "Rafael J. Wysocki" Cc: Len Brown Cc: Andrew Morton Cc: David Hildenbrand Cc: Oscar Salvador Cc: Dan Williams Cc: Dave Jiang Cc: Dave Hansen Cc: Huang Ying Signed-off-by: Vishal Verma --- drivers/dax/kmem.c | 49 - 1 file changed, 36 insertions(+), 13 deletions(-) diff --git a/drivers/dax/kmem.c b/drivers/dax/kmem.c index 7b36db6f1cbd..0751346193ef 100644 --- a/drivers/dax/kmem.c +++ b/drivers/dax/kmem.c @@ -12,6 +12,7 @@ #include #include #include +#include #include "dax-private.h" #include "bus.h" @@ -105,6 +106,7 @@ static int dev_dax_kmem_probe(struct dev_dax *dev_dax) data->mgid = rc; for (i = 0; i < dev_dax->nr_range; i++) { + u64 cur_start, cur_len, remaining; struct resource *res; struct range range; @@ -137,21 +139,42 @@ static int dev_dax_kmem_probe(struct dev_dax *dev_dax) res->flags = IORESOURCE_SYSTEM_RAM; /* -* Ensure that future kexec'd kernels will not treat -* this as RAM automatically. +* Add memory in chunks of memory_block_size_bytes() so that +* it is considered for MHP_MEMMAP_ON_MEMORY +* @range has already been aligned to memory_block_size_bytes(), +* so the following loop will always break it down cleanly. */ - rc = add_memory_driver_managed(data->mgid, range.start, - range_len(), kmem_name, MHP_NID_IS_MGID); + cur_start = range.start; + cur_len = memory_block_size_bytes(); + remaining = range_len(); + while (remaining) { + mhp_t mhp_flags = MHP_NID_IS_MGID; - if (rc) { - dev_warn(dev, "mapping%d: %#llx-%#llx memory add failed\n", - i, range.start, range.end); - remove_resource(res); - kfree(res); - data->res[i] = NULL; - if (mapped) - continue; - goto err_request_mem; + if (mhp_supports_memmap_on_memory(cur_len, + MHP_MEMMAP_ON_MEMORY)) + mhp_flags |= MHP_MEMMAP_ON_MEMORY; + /* +* Ensure that future kexec'd kernels will not treat +* this as RAM automatically. +*/ + rc = add_memory_driver_managed(data->mgid, cur_start, + cur_len, kmem_name, + mhp_flags); + + if (rc) { + dev_warn(dev, +"mapping%d: %#llx-%#llx memory add failed\n", +i, cur_start, cur_start + cur_len - 1); + remove_resource(res); + kfree(res); + data->res[i] = NULL; + if (mapped) + continue; + goto err_request_mem; + } + + cur_start += cur_len; + remaining -= cur_len; } mapped++; } Maybe the better alternative is teach add_memory_resource()/try_remove_memory() to do that internally. In the add_memory_resource() case, it might be a loop around that memmap_on_memory + arch_add_memory code path (well, and the error path also needs adjustment): /* * Self hosted memmap array */ if (mhp_flags & MHP_MEMMAP_ON_MEMORY) { if (!mhp_supports_memmap_on_memory(size)) { ret = -EINVAL; goto error; } mhp_altmap.free = PHYS_PFN(size); mhp_altmap.base_pfn = PHYS_PFN(start); params.altmap = _altmap; } /* call arch's memory hotadd */ ret =
Re: [PATCH 2/3] mm/memory_hotplug: Export symbol mhp_supports_memmap_on_memory()
On 16.06.23 00:00, Vishal Verma wrote: In preparation for the dax/kmem driver, which can be built as a module, to use this interface, export it with EXPORT_SYMBOL_GPL(). Cc: "Rafael J. Wysocki" Cc: Len Brown Cc: Andrew Morton Cc: David Hildenbrand Cc: Oscar Salvador Cc: Dan Williams Cc: Dave Jiang Cc: Dave Hansen Cc: Huang Ying Signed-off-by: Vishal Verma --- mm/memory_hotplug.c | 1 + 1 file changed, 1 insertion(+) diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c index bb3845830922..92922080d3fa 100644 --- a/mm/memory_hotplug.c +++ b/mm/memory_hotplug.c @@ -1328,6 +1328,7 @@ bool mhp_supports_memmap_on_memory(unsigned long size, mhp_t mhp_flags) IS_ALIGNED(remaining_size, (pageblock_nr_pages << PAGE_SHIFT)); return false; } +EXPORT_SYMBOL_GPL(mhp_supports_memmap_on_memory); /* * NOTE: The caller must call lock_device_hotplug() to serialize hotplug Reviewed-by: David Hildenbrand -- Cheers, David / dhildenb
Re: [PATCH 1/3] mm/memory_hotplug: Allow an override for the memmap_on_memory param
On 16.06.23 00:00, Vishal Verma wrote: For memory hotplug to consider MHP_MEMMAP_ON_MEMORY behavior, the 'memmap_on_memory' module parameter was a hard requirement. In preparation for the dax/kmem driver to use memmap_on_memory semantics, arrange for the module parameter check to be bypassed via the appropriate mhp_flag. Recall that the kmem driver could contribute huge amounts of hotplugged memory originating from special purposes devices such as CXL memory expanders. In some cases memmap_on_memory may be the /only/ way this new memory can be hotplugged. Hence it makes sense for kmem to have a way to force memmap_on_memory without depending on a module param, if all the other conditions for it are met. Just let the admin configure it. After all, an admin is involved in configuring the dax/kmem device to begin with. If add_memory() fails you could give a useful hint to the admin. -- Cheers, David / dhildenb
Re: [PATCH 0/3] mm: use memmap_on_memory semantics for dax/kmem
On 16.06.23 00:00, Vishal Verma wrote: The dax/kmem driver can potentially hot-add large amounts of memory originating from CXL memory expanders, or NVDIMMs, or other 'device memories'. There is a chance there isn't enough regular system memory available to fit ythe memmap for this new memory. It's therefore desirable, if all other conditions are met, for the kmem managed memory to place its memmap on the newly added memory itself. Arrange for this by first allowing for a module parameter override for the mhp_supports_memmap_on_memory() test using a flag, adjusting the only other caller of this interface in dirvers/acpi/acpi_memoryhotplug.c, exporting the symbol so it can be called by kmem.c, and finally changing the kmem driver to add_memory() in chunks of memory_block_size_bytes(). 1) Why is the override a requirement here? Just let the admin configure it then then add conditional support for kmem. 2) I recall that there are cases where we don't want the memmap to land on slow memory (which online_movable would achieve). Just imagine the slow PMEM case. So this might need another configuration knob on the kmem side. I recall some discussions on doing that chunk handling internally (so kmem can just perform one add_memory() and we'd split that up internally). -- Cheers, David / dhildenb
Re: [PATCH 3/3] dax/kmem: Always enroll hotplugged memory for memmap_on_memory
Vishal Verma writes: > With DAX memory regions originating from CXL memory expanders or > NVDIMMs, the kmem driver may be hot-adding huge amounts of system memory > on a system without enough 'regular' main memory to support the memmap > for it. To avoid this, ensure that all kmem managed hotplugged memory is > added with the MHP_MEMMAP_ON_MEMORY flag to place the memmap on the > new memory region being hot added. > > To do this, call add_memory() in chunks of memory_block_size_bytes() as > that is a requirement for memmap_on_memory. Additionally, Use the > mhp_flag to force the memmap_on_memory checks regardless of the > respective module parameter setting. > > Cc: "Rafael J. Wysocki" > Cc: Len Brown > Cc: Andrew Morton > Cc: David Hildenbrand > Cc: Oscar Salvador > Cc: Dan Williams > Cc: Dave Jiang > Cc: Dave Hansen > Cc: Huang Ying > Signed-off-by: Vishal Verma > --- > drivers/dax/kmem.c | 49 - > 1 file changed, 36 insertions(+), 13 deletions(-) > > diff --git a/drivers/dax/kmem.c b/drivers/dax/kmem.c > index 7b36db6f1cbd..0751346193ef 100644 > --- a/drivers/dax/kmem.c > +++ b/drivers/dax/kmem.c > @@ -12,6 +12,7 @@ > #include > #include > #include > +#include > #include "dax-private.h" > #include "bus.h" > > @@ -105,6 +106,7 @@ static int dev_dax_kmem_probe(struct dev_dax *dev_dax) > data->mgid = rc; > > for (i = 0; i < dev_dax->nr_range; i++) { > + u64 cur_start, cur_len, remaining; > struct resource *res; > struct range range; > > @@ -137,21 +139,42 @@ static int dev_dax_kmem_probe(struct dev_dax *dev_dax) > res->flags = IORESOURCE_SYSTEM_RAM; > > /* > - * Ensure that future kexec'd kernels will not treat > - * this as RAM automatically. > + * Add memory in chunks of memory_block_size_bytes() so that > + * it is considered for MHP_MEMMAP_ON_MEMORY > + * @range has already been aligned to memory_block_size_bytes(), > + * so the following loop will always break it down cleanly. >*/ > - rc = add_memory_driver_managed(data->mgid, range.start, > - range_len(), kmem_name, MHP_NID_IS_MGID); > + cur_start = range.start; > + cur_len = memory_block_size_bytes(); > + remaining = range_len(); > + while (remaining) { > + mhp_t mhp_flags = MHP_NID_IS_MGID; > > - if (rc) { > - dev_warn(dev, "mapping%d: %#llx-%#llx memory add > failed\n", > - i, range.start, range.end); > - remove_resource(res); > - kfree(res); > - data->res[i] = NULL; > - if (mapped) > - continue; > - goto err_request_mem; > + if (mhp_supports_memmap_on_memory(cur_len, > + MHP_MEMMAP_ON_MEMORY)) > + mhp_flags |= MHP_MEMMAP_ON_MEMORY; > + /* > + * Ensure that future kexec'd kernels will not treat > + * this as RAM automatically. > + */ > + rc = add_memory_driver_managed(data->mgid, cur_start, > +cur_len, kmem_name, > +mhp_flags); > + > + if (rc) { > + dev_warn(dev, > + "mapping%d: %#llx-%#llx memory add > failed\n", > + i, cur_start, cur_start + cur_len - 1); > + remove_resource(res); > + kfree(res); > + data->res[i] = NULL; > + if (mapped) > + continue; > + goto err_request_mem; > + } > + > + cur_start += cur_len; > + remaining -= cur_len; > } > mapped++; > } It appears that we need to hot-remove memory in the granularity of memory_block_size_bytes() too, according to try_remove_memory(). If so, it seems better to allocate one dax_kmem_data.res[] element for each memory block instead of dax region? Best Regards, Huang, Ying
Re: [PATCH 1/3] mm/memory_hotplug: Allow an override for the memmap_on_memory param
Hi, Vishal, Thanks for your patch! Vishal Verma writes: > For memory hotplug to consider MHP_MEMMAP_ON_MEMORY behavior, the > 'memmap_on_memory' module parameter was a hard requirement. > > In preparation for the dax/kmem driver to use memmap_on_memory > semantics, arrange for the module parameter check to be bypassed via the > appropriate mhp_flag. > > Recall that the kmem driver could contribute huge amounts of hotplugged > memory originating from special purposes devices such as CXL memory > expanders. In some cases memmap_on_memory may be the /only/ way this new > memory can be hotplugged. Hence it makes sense for kmem to have a way to > force memmap_on_memory without depending on a module param, if all the > other conditions for it are met. > > The only other user of this interface is acpi/acpi_memoryhotplug.c, > which only enables the mhp_flag if an initial > mhp_supports_memmap_on_memory() test passes. Maintain the existing > behavior and semantics for this by performing the initial check from > acpi without the MHP_MEMMAP_ON_MEMORY flag, so its decision falls back > to the module parameter. > > Cc: "Rafael J. Wysocki" > Cc: Len Brown > Cc: Andrew Morton > Cc: David Hildenbrand > Cc: Oscar Salvador > Cc: Dan Williams > Cc: Dave Jiang > Cc: Dave Hansen > Cc: Huang Ying > Signed-off-by: Vishal Verma > --- > include/linux/memory_hotplug.h | 2 +- > drivers/acpi/acpi_memhotplug.c | 2 +- > mm/memory_hotplug.c| 24 > 3 files changed, 18 insertions(+), 10 deletions(-) > > diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h > index 9fcbf5706595..c9ddcd3cad70 100644 > --- a/include/linux/memory_hotplug.h > +++ b/include/linux/memory_hotplug.h > @@ -358,7 +358,7 @@ extern struct zone *zone_for_pfn_range(int online_type, > int nid, > extern int arch_create_linear_mapping(int nid, u64 start, u64 size, > struct mhp_params *params); > void arch_remove_linear_mapping(u64 start, u64 size); > -extern bool mhp_supports_memmap_on_memory(unsigned long size); > +extern bool mhp_supports_memmap_on_memory(unsigned long size, mhp_t > mhp_flags); > #endif /* CONFIG_MEMORY_HOTPLUG */ > > #endif /* __LINUX_MEMORY_HOTPLUG_H */ > diff --git a/drivers/acpi/acpi_memhotplug.c b/drivers/acpi/acpi_memhotplug.c > index 24f662d8bd39..119d3bb49753 100644 > --- a/drivers/acpi/acpi_memhotplug.c > +++ b/drivers/acpi/acpi_memhotplug.c > @@ -211,7 +211,7 @@ static int acpi_memory_enable_device(struct > acpi_memory_device *mem_device) > if (!info->length) > continue; > > - if (mhp_supports_memmap_on_memory(info->length)) > + if (mhp_supports_memmap_on_memory(info->length, 0)) > mhp_flags |= MHP_MEMMAP_ON_MEMORY; > result = __add_memory(mgid, info->start_addr, info->length, > mhp_flags); > diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c > index 8e0fa209d533..bb3845830922 100644 > --- a/mm/memory_hotplug.c > +++ b/mm/memory_hotplug.c > @@ -1283,15 +1283,21 @@ static int online_memory_block(struct memory_block > *mem, void *arg) > return device_online(>dev); > } > > -bool mhp_supports_memmap_on_memory(unsigned long size) > +bool mhp_supports_memmap_on_memory(unsigned long size, mhp_t mhp_flags) > { > unsigned long nr_vmemmap_pages = size / PAGE_SIZE; > unsigned long vmemmap_size = nr_vmemmap_pages * sizeof(struct page); > unsigned long remaining_size = size - vmemmap_size; > > /* > - * Besides having arch support and the feature enabled at runtime, we > - * need a few more assumptions to hold true: > + * The MHP_MEMMAP_ON_MEMORY flag indicates a caller that wants to force > + * memmap_on_memory (if other conditions are met), regardless of the > + * module parameter. drivers/dax/kmem.c is an example, where large > + * amounts of hotplug memory may come from, and the only option to > + * successfully online all of it is to place the memmap on this memory. > + * > + * Besides having arch support and the feature enabled at runtime or > + * via the mhp_flag, we need a few more assumptions to hold true: >* >* a) We span a single memory block: memory onlining/offlinin;g happens >*in memory block granularity. We don't want the vmemmap of online > @@ -1315,10 +1321,12 @@ bool mhp_supports_memmap_on_memory(unsigned long size) >* altmap as an alternative source of memory, and we do not > exactly >* populate a single PMD. >*/ > - return mhp_memmap_on_memory() && > -size == memory_block_size_bytes() && > -IS_ALIGNED(vmemmap_size, PMD_SIZE) && > -IS_ALIGNED(remaining_size, (pageblock_nr_pages << PAGE_SHIFT)); > + > + if ((mhp_flags & MHP_MEMMAP_ON_MEMORY) || mhp_memmap_on_memory()) > + return