| Message ID | 20200610143943.12548-1-gustav.wiklander@axis.com |
|---|---|
| State | New |
| Headers | show |
| Series | PM / Domains: Add module ref count for each consumer | expand |
On Wed, Jun 10, 2020 at 07:24:02PM +0200, Gustav Wiklander wrote: > On 6/10/20 4:52 PM, Greg KH wrote: > > On Wed, Jun 10, 2020 at 04:39:43PM +0200, Gustav Wiklander wrote: > > > From: Gustav Wiklander <gustavwi@axis.com> > > > > > > Currently a pm_domain can be unloaded without regard for consumers. > > > This patch adds a module dependecy for every registered consumer. > > > Now a power domain driver can only be unloaded if no consumers are > > > registered. > > > > What is the problem with doing this? Shouldn't when a power domain is > > unregistered, the consumers are properly torn down? Some subsystems > > allow you to unload a module at any point in time, and properly clean > > things up. What is the problem today that you are trying to solve with > > this (remember, removing modules only happens by developers, no > > real-world system ever automatically onloads a module.) > > > > > Hi Greg and Rafael, > > Thanks for the quick reply. > > PM domains shall call pm_genpd_remove at removal. As described in the > definition pm_genpd_remove will fail if any device is still associated to > it. *However*, at module removal the kernel ignores device removal failure > and still unloads the powerdomain module. So shouldn't the driver that controls that device be fixed up to properly remove the association first? Is there any in-kernel code that has this problem today? Adding module reference logic for an operation that has to be initiated by a developer only, seems like it's not really needed. thanks, greg k-h
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 0a01df608849..80723f6d5e6b 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -1499,11 +1499,18 @@ static int genpd_add_device(struct generic_pm_domain *genpd, struct device *dev, if (IS_ERR(gpd_data)) return PTR_ERR(gpd_data); + if (!try_module_get(genpd->owner)) { + ret = -ENODEV; + goto out; + } + gpd_data->cpu = genpd_get_cpu(genpd, base_dev); ret = genpd->attach_dev ? genpd->attach_dev(genpd, dev) : 0; - if (ret) + if (ret) { + module_put(genpd->owner); goto out; + } genpd_lock(genpd); @@ -1579,6 +1586,8 @@ static int genpd_remove_device(struct generic_pm_domain *genpd, genpd_free_dev_data(dev, gpd_data); + module_put(genpd->owner); + return 0; out: diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h index 9ec78ee53652..777c1b30e5af 100644 --- a/include/linux/pm_domain.h +++ b/include/linux/pm_domain.h @@ -9,6 +9,7 @@ #define _LINUX_PM_DOMAIN_H #include <linux/device.h> +#include <linux/module.h> #include <linux/mutex.h> #include <linux/pm.h> #include <linux/err.h> @@ -93,6 +94,7 @@ struct opp_table; struct generic_pm_domain { struct device dev; + struct module *owner; /* Module owner of the PM domain */ struct dev_pm_domain domain; /* PM domain operations */ struct list_head gpd_list_node; /* Node in the global PM domains list */ struct list_head master_links; /* Links with PM domain as a master */