* [PATCH 1/2] PM / Qos: Fix a free error in the dev_pm_qos_constraints_destroy() @ 2012-11-01 10:47 Lan Tianyu 2012-11-01 10:47 ` [PATCH 2/2] PM / Qos: Add notifier for pm qos flags Lan Tianyu 2012-11-01 14:27 ` [PATCH 1/2] PM / Qos: Fix a free error in the dev_pm_qos_constraints_destroy() Rafael J. Wysocki 0 siblings, 2 replies; 5+ messages in thread From: Lan Tianyu @ 2012-11-01 10:47 UTC (permalink / raw) To: rjw, stern; +Cc: Lan Tianyu, linux-pm, linux-acpi Free a wrong point to struct dev_pm_qos->latency which suppose to be the point to struct dev_pm_qos. The patch is to fix the issue. Signed-off-by: Lan Tianyu <tianyu.lan@intel.com> --- drivers/base/power/qos.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/base/power/qos.c b/drivers/base/power/qos.c index 3a7687a..31d3f48 100644 --- a/drivers/base/power/qos.c +++ b/drivers/base/power/qos.c @@ -253,7 +253,7 @@ void dev_pm_qos_constraints_destroy(struct device *dev) spin_unlock_irq(&dev->power.lock); kfree(c->notifiers); - kfree(c); + kfree(qos); out: mutex_unlock(&dev_pm_qos_mtx); -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] PM / Qos: Add notifier for pm qos flags 2012-11-01 10:47 [PATCH 1/2] PM / Qos: Fix a free error in the dev_pm_qos_constraints_destroy() Lan Tianyu @ 2012-11-01 10:47 ` Lan Tianyu 2012-11-01 14:26 ` Rafael J. Wysocki 2012-11-01 14:27 ` [PATCH 1/2] PM / Qos: Fix a free error in the dev_pm_qos_constraints_destroy() Rafael J. Wysocki 1 sibling, 1 reply; 5+ messages in thread From: Lan Tianyu @ 2012-11-01 10:47 UTC (permalink / raw) To: rjw, stern; +Cc: Lan Tianyu, linux-pm, linux-acpi If pm qos flags was changed, device driver would take different action according flags. E.g NO_POWER_OFF flag, if it was set when a device had been power off, device driver should power on the device, resume it and suspend it again without not power off. So device driver should be notified. This patch is to add notifier for pm qos flags and notify device when flags is changed. Signed-off-by: Lan Tianyu <tianyu.lan@intel.com> --- drivers/base/power/qos.c | 86 +++++++++++++++++++++++++++++++++++++--------- include/linux/pm_qos.h | 7 ++-- 2 files changed, 75 insertions(+), 18 deletions(-) diff --git a/drivers/base/power/qos.c b/drivers/base/power/qos.c index 31d3f48..c37f52f 100644 --- a/drivers/base/power/qos.c +++ b/drivers/base/power/qos.c @@ -149,6 +149,10 @@ static int apply_constraint(struct dev_pm_qos_request *req, case DEV_PM_QOS_FLAGS: ret = pm_qos_update_flags(&qos->flags, &req->data.flr, action, value); + if (ret) + blocking_notifier_call_chain(qos->flags.notifiers, + (unsigned long)qos->flags.effective_flags, + req); break; default: ret = -EINVAL; @@ -168,26 +172,38 @@ static int dev_pm_qos_constraints_allocate(struct device *dev) { struct dev_pm_qos *qos; struct pm_qos_constraints *c; - struct blocking_notifier_head *n; + struct pm_qos_flags *flags; + struct blocking_notifier_head *constraints_notify; + struct blocking_notifier_head *flags_notify; + int ret = 0; qos = kzalloc(sizeof(*qos), GFP_KERNEL); if (!qos) return -ENOMEM; - n = kzalloc(sizeof(*n), GFP_KERNEL); - if (!n) { - kfree(qos); - return -ENOMEM; + constraints_notify = kzalloc(sizeof(*constraints_notify), GFP_KERNEL); + if (!constraints_notify) { + ret = -ENOMEM; + goto alloc_constraints_fail; + } + BLOCKING_INIT_NOTIFIER_HEAD(constraints_notify); + + flags_notify = kzalloc(sizeof(*flags_notify), GFP_KERNEL); + if (!flags_notify) { + ret = -ENOMEM; + goto alloc_flags_fail; } - BLOCKING_INIT_NOTIFIER_HEAD(n); + BLOCKING_INIT_NOTIFIER_HEAD(flags_notify); c = &qos->latency; plist_head_init(&c->list); c->target_value = PM_QOS_DEV_LAT_DEFAULT_VALUE; c->default_value = PM_QOS_DEV_LAT_DEFAULT_VALUE; c->type = PM_QOS_MIN; - c->notifiers = n; + c->notifiers = constraints_notify; + flags = &qos->flags; + flags->notifiers = flags_notify; INIT_LIST_HEAD(&qos->flags.list); spin_lock_irq(&dev->power.lock); @@ -195,6 +211,12 @@ static int dev_pm_qos_constraints_allocate(struct device *dev) spin_unlock_irq(&dev->power.lock); return 0; + +alloc_flags_fail: + kfree(constraints_notify); +alloc_constraints_fail: + kfree(qos); + return ret; } /** @@ -432,6 +454,7 @@ EXPORT_SYMBOL_GPL(dev_pm_qos_remove_request); * * @dev: target device for the constraint * @notifier: notifier block managed by caller. + * @type: type of notifier * * Will register the notifier into a notification chain that gets called * upon changes to the target value for the device. @@ -439,9 +462,11 @@ EXPORT_SYMBOL_GPL(dev_pm_qos_remove_request); * If the device's constraints object doesn't exist when this routine is called, * it will be created (or error code will be returned if that fails). */ -int dev_pm_qos_add_notifier(struct device *dev, struct notifier_block *notifier) +int dev_pm_qos_add_notifier(struct device *dev, struct notifier_block *notifier, + enum dev_pm_qos_req_type type) { int ret = 0; + struct blocking_notifier_head *notifiers_head = NULL; mutex_lock(&dev_pm_qos_mtx); @@ -449,9 +474,22 @@ int dev_pm_qos_add_notifier(struct device *dev, struct notifier_block *notifier) ret = dev->power.power_state.event != PM_EVENT_INVALID ? dev_pm_qos_constraints_allocate(dev) : -ENODEV; - if (!ret) - ret = blocking_notifier_chain_register( - dev->power.qos->latency.notifiers, notifier); + if (!ret) { + switch (type) { + case DEV_PM_QOS_LATENCY: + notifiers_head = dev->power.qos->latency.notifiers; + break; + case DEV_PM_QOS_FLAGS: + notifiers_head = dev->power.qos->flags.notifiers; + break; + default: + break; + } + + if (notifiers_head) + ret = blocking_notifier_chain_register( + notifiers_head, notifier); + } mutex_unlock(&dev_pm_qos_mtx); return ret; @@ -464,22 +502,38 @@ EXPORT_SYMBOL_GPL(dev_pm_qos_add_notifier); * * @dev: target device for the constraint * @notifier: notifier block to be removed. + * @type: type of notifier * * Will remove the notifier from the notification chain that gets called * upon changes to the target value. */ int dev_pm_qos_remove_notifier(struct device *dev, - struct notifier_block *notifier) + struct notifier_block *notifier, enum dev_pm_qos_req_type type) { int retval = 0; + struct blocking_notifier_head *notifiers_head = NULL; + mutex_lock(&dev_pm_qos_mtx); /* Silently return if the constraints object is not present. */ - if (dev->power.qos) - retval = blocking_notifier_chain_unregister( - dev->power.qos->latency.notifiers, - notifier); + if (dev->power.qos) { + switch (type) { + case DEV_PM_QOS_LATENCY: + notifiers_head = dev->power.qos->latency.notifiers; + break; + case DEV_PM_QOS_FLAGS: + notifiers_head = dev->power.qos->flags.notifiers; + break; + default: + break; + } + + if (notifiers_head) + retval = blocking_notifier_chain_unregister( + notifiers_head, + notifier); + } mutex_unlock(&dev_pm_qos_mtx); return retval; diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h index 5a95013..479da6c 100644 --- a/include/linux/pm_qos.h +++ b/include/linux/pm_qos.h @@ -84,6 +84,7 @@ struct pm_qos_constraints { struct pm_qos_flags { struct list_head list; s32 effective_flags; /* Do not change to 64 bit */ + struct blocking_notifier_head *notifiers; }; struct dev_pm_qos { @@ -134,9 +135,11 @@ int dev_pm_qos_add_request(struct device *dev, struct dev_pm_qos_request *req, int dev_pm_qos_update_request(struct dev_pm_qos_request *req, s32 new_value); int dev_pm_qos_remove_request(struct dev_pm_qos_request *req); int dev_pm_qos_add_notifier(struct device *dev, - struct notifier_block *notifier); + struct notifier_block *notifier, + enum dev_pm_qos_req_type type); int dev_pm_qos_remove_notifier(struct device *dev, - struct notifier_block *notifier); + struct notifier_block *notifier, + enum dev_pm_qos_req_type type); int dev_pm_qos_add_global_notifier(struct notifier_block *notifier); int dev_pm_qos_remove_global_notifier(struct notifier_block *notifier); void dev_pm_qos_constraints_init(struct device *dev); -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] PM / Qos: Add notifier for pm qos flags 2012-11-01 10:47 ` [PATCH 2/2] PM / Qos: Add notifier for pm qos flags Lan Tianyu @ 2012-11-01 14:26 ` Rafael J. Wysocki 2012-11-01 23:21 ` [PATCH] PM / QoS: Document request manipulation requirement for flags Rafael J. Wysocki 0 siblings, 1 reply; 5+ messages in thread From: Rafael J. Wysocki @ 2012-11-01 14:26 UTC (permalink / raw) To: Lan Tianyu; +Cc: stern, linux-pm, linux-acpi On Thursday, November 01, 2012 06:47:26 PM Lan Tianyu wrote: > If pm qos flags was changed, device driver would take different > action according flags. E.g NO_POWER_OFF flag, if it was set when > a device had been power off, device driver should power on the device, > resume it and suspend it again without not power off. So device driver > should be notified. This patch is to add notifier for pm qos flags and > notify device when flags is changed. > > Signed-off-by: Lan Tianyu <tianyu.lan@intel.com> I haven't added notifiers for PM QoS flags on purpose, so I'm not going to apply this patch. Instead, it is required that callers of dev_pm_qos_update_request() for DEV_PM_QOS_FLAGS type of requests ensure that the device is not RPM_SUSPENDED when calling that routine (please note that the user interface calls pm_runtime_get_sync() on the device before changing its flags). Well, I think I should add that to the kerneldoc comment of dev_pm_qos_update_request(). Moreover, I'm planning to remove the notifiers from the device PM QoS code entirely and change the locking so that dev_pm_qos_update_request() and friends may be called from interrupt context (which is necessary for some use cases). Thanks, Rafael > --- > drivers/base/power/qos.c | 86 +++++++++++++++++++++++++++++++++++++--------- > include/linux/pm_qos.h | 7 ++-- > 2 files changed, 75 insertions(+), 18 deletions(-) > > diff --git a/drivers/base/power/qos.c b/drivers/base/power/qos.c > index 31d3f48..c37f52f 100644 > --- a/drivers/base/power/qos.c > +++ b/drivers/base/power/qos.c > @@ -149,6 +149,10 @@ static int apply_constraint(struct dev_pm_qos_request *req, > case DEV_PM_QOS_FLAGS: > ret = pm_qos_update_flags(&qos->flags, &req->data.flr, > action, value); > + if (ret) > + blocking_notifier_call_chain(qos->flags.notifiers, > + (unsigned long)qos->flags.effective_flags, > + req); > break; > default: > ret = -EINVAL; > @@ -168,26 +172,38 @@ static int dev_pm_qos_constraints_allocate(struct device *dev) > { > struct dev_pm_qos *qos; > struct pm_qos_constraints *c; > - struct blocking_notifier_head *n; > + struct pm_qos_flags *flags; > + struct blocking_notifier_head *constraints_notify; > + struct blocking_notifier_head *flags_notify; > + int ret = 0; > > qos = kzalloc(sizeof(*qos), GFP_KERNEL); > if (!qos) > return -ENOMEM; > > - n = kzalloc(sizeof(*n), GFP_KERNEL); > - if (!n) { > - kfree(qos); > - return -ENOMEM; > + constraints_notify = kzalloc(sizeof(*constraints_notify), GFP_KERNEL); > + if (!constraints_notify) { > + ret = -ENOMEM; > + goto alloc_constraints_fail; > + } > + BLOCKING_INIT_NOTIFIER_HEAD(constraints_notify); > + > + flags_notify = kzalloc(sizeof(*flags_notify), GFP_KERNEL); > + if (!flags_notify) { > + ret = -ENOMEM; > + goto alloc_flags_fail; > } > - BLOCKING_INIT_NOTIFIER_HEAD(n); > + BLOCKING_INIT_NOTIFIER_HEAD(flags_notify); > > c = &qos->latency; > plist_head_init(&c->list); > c->target_value = PM_QOS_DEV_LAT_DEFAULT_VALUE; > c->default_value = PM_QOS_DEV_LAT_DEFAULT_VALUE; > c->type = PM_QOS_MIN; > - c->notifiers = n; > + c->notifiers = constraints_notify; > > + flags = &qos->flags; > + flags->notifiers = flags_notify; > INIT_LIST_HEAD(&qos->flags.list); > > spin_lock_irq(&dev->power.lock); > @@ -195,6 +211,12 @@ static int dev_pm_qos_constraints_allocate(struct device *dev) > spin_unlock_irq(&dev->power.lock); > > return 0; > + > +alloc_flags_fail: > + kfree(constraints_notify); > +alloc_constraints_fail: > + kfree(qos); > + return ret; > } > > /** > @@ -432,6 +454,7 @@ EXPORT_SYMBOL_GPL(dev_pm_qos_remove_request); > * > * @dev: target device for the constraint > * @notifier: notifier block managed by caller. > + * @type: type of notifier > * > * Will register the notifier into a notification chain that gets called > * upon changes to the target value for the device. > @@ -439,9 +462,11 @@ EXPORT_SYMBOL_GPL(dev_pm_qos_remove_request); > * If the device's constraints object doesn't exist when this routine is called, > * it will be created (or error code will be returned if that fails). > */ > -int dev_pm_qos_add_notifier(struct device *dev, struct notifier_block *notifier) > +int dev_pm_qos_add_notifier(struct device *dev, struct notifier_block *notifier, > + enum dev_pm_qos_req_type type) > { > int ret = 0; > + struct blocking_notifier_head *notifiers_head = NULL; > > mutex_lock(&dev_pm_qos_mtx); > > @@ -449,9 +474,22 @@ int dev_pm_qos_add_notifier(struct device *dev, struct notifier_block *notifier) > ret = dev->power.power_state.event != PM_EVENT_INVALID ? > dev_pm_qos_constraints_allocate(dev) : -ENODEV; > > - if (!ret) > - ret = blocking_notifier_chain_register( > - dev->power.qos->latency.notifiers, notifier); > + if (!ret) { > + switch (type) { > + case DEV_PM_QOS_LATENCY: > + notifiers_head = dev->power.qos->latency.notifiers; > + break; > + case DEV_PM_QOS_FLAGS: > + notifiers_head = dev->power.qos->flags.notifiers; > + break; > + default: > + break; > + } > + > + if (notifiers_head) > + ret = blocking_notifier_chain_register( > + notifiers_head, notifier); > + } > > mutex_unlock(&dev_pm_qos_mtx); > return ret; > @@ -464,22 +502,38 @@ EXPORT_SYMBOL_GPL(dev_pm_qos_add_notifier); > * > * @dev: target device for the constraint > * @notifier: notifier block to be removed. > + * @type: type of notifier > * > * Will remove the notifier from the notification chain that gets called > * upon changes to the target value. > */ > int dev_pm_qos_remove_notifier(struct device *dev, > - struct notifier_block *notifier) > + struct notifier_block *notifier, enum dev_pm_qos_req_type type) > { > int retval = 0; > + struct blocking_notifier_head *notifiers_head = NULL; > + > > mutex_lock(&dev_pm_qos_mtx); > > /* Silently return if the constraints object is not present. */ > - if (dev->power.qos) > - retval = blocking_notifier_chain_unregister( > - dev->power.qos->latency.notifiers, > - notifier); > + if (dev->power.qos) { > + switch (type) { > + case DEV_PM_QOS_LATENCY: > + notifiers_head = dev->power.qos->latency.notifiers; > + break; > + case DEV_PM_QOS_FLAGS: > + notifiers_head = dev->power.qos->flags.notifiers; > + break; > + default: > + break; > + } > + > + if (notifiers_head) > + retval = blocking_notifier_chain_unregister( > + notifiers_head, > + notifier); > + } > > mutex_unlock(&dev_pm_qos_mtx); > return retval; > diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h > index 5a95013..479da6c 100644 > --- a/include/linux/pm_qos.h > +++ b/include/linux/pm_qos.h > @@ -84,6 +84,7 @@ struct pm_qos_constraints { > struct pm_qos_flags { > struct list_head list; > s32 effective_flags; /* Do not change to 64 bit */ > + struct blocking_notifier_head *notifiers; > }; > > struct dev_pm_qos { > @@ -134,9 +135,11 @@ int dev_pm_qos_add_request(struct device *dev, struct dev_pm_qos_request *req, > int dev_pm_qos_update_request(struct dev_pm_qos_request *req, s32 new_value); > int dev_pm_qos_remove_request(struct dev_pm_qos_request *req); > int dev_pm_qos_add_notifier(struct device *dev, > - struct notifier_block *notifier); > + struct notifier_block *notifier, > + enum dev_pm_qos_req_type type); > int dev_pm_qos_remove_notifier(struct device *dev, > - struct notifier_block *notifier); > + struct notifier_block *notifier, > + enum dev_pm_qos_req_type type); > int dev_pm_qos_add_global_notifier(struct notifier_block *notifier); > int dev_pm_qos_remove_global_notifier(struct notifier_block *notifier); > void dev_pm_qos_constraints_init(struct device *dev); > -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] PM / QoS: Document request manipulation requirement for flags 2012-11-01 14:26 ` Rafael J. Wysocki @ 2012-11-01 23:21 ` Rafael J. Wysocki 0 siblings, 0 replies; 5+ messages in thread From: Rafael J. Wysocki @ 2012-11-01 23:21 UTC (permalink / raw) To: Lan Tianyu, linux-pm; +Cc: stern, linux-acpi From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> In fact, the callers of dev_pm_qos_add_request(), dev_pm_qos_update_request() and dev_pm_qos_remove_request() for requests of type DEV_PM_QOS_FLAGS need to ensure that the target device is not RPM_SUSPENDED before using any of these functions (or be prepared for the new PM QoS flags to take effect after the device has been resumed). Document this in their kerneldoc comments. Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> --- drivers/base/power/qos.c | 9 +++++++++ 1 file changed, 9 insertions(+) Index: linux/drivers/base/power/qos.c =================================================================== --- linux.orig/drivers/base/power/qos.c +++ linux/drivers/base/power/qos.c @@ -277,6 +277,9 @@ void dev_pm_qos_constraints_destroy(stru * -EINVAL in case of wrong parameters, -ENOMEM if there's not enough memory * to allocate for data structures, -ENODEV if the device has just been removed * from the system. + * + * Callers should ensure that the target device is not RPM_SUSPENDED before + * using this function for requests of type DEV_PM_QOS_FLAGS. */ int dev_pm_qos_add_request(struct device *dev, struct dev_pm_qos_request *req, enum dev_pm_qos_req_type type, s32 value) @@ -367,6 +370,9 @@ static int __dev_pm_qos_update_request(s * 0 if the aggregated constraint value has not changed, * -EINVAL in case of wrong parameters, -ENODEV if the device has been * removed from the system + * + * Callers should ensure that the target device is not RPM_SUSPENDED before + * using this function for requests of type DEV_PM_QOS_FLAGS. */ int dev_pm_qos_update_request(struct dev_pm_qos_request *req, s32 new_value) { @@ -398,6 +404,9 @@ EXPORT_SYMBOL_GPL(dev_pm_qos_update_requ * 0 if the aggregated constraint value has not changed, * -EINVAL in case of wrong parameters, -ENODEV if the device has been * removed from the system + * + * Callers should ensure that the target device is not RPM_SUSPENDED before + * using this function for requests of type DEV_PM_QOS_FLAGS. */ int dev_pm_qos_remove_request(struct dev_pm_qos_request *req) { -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] PM / Qos: Fix a free error in the dev_pm_qos_constraints_destroy() 2012-11-01 10:47 [PATCH 1/2] PM / Qos: Fix a free error in the dev_pm_qos_constraints_destroy() Lan Tianyu 2012-11-01 10:47 ` [PATCH 2/2] PM / Qos: Add notifier for pm qos flags Lan Tianyu @ 2012-11-01 14:27 ` Rafael J. Wysocki 1 sibling, 0 replies; 5+ messages in thread From: Rafael J. Wysocki @ 2012-11-01 14:27 UTC (permalink / raw) To: Lan Tianyu; +Cc: stern, linux-pm, linux-acpi On Thursday, November 01, 2012 06:47:25 PM Lan Tianyu wrote: > Free a wrong point to struct dev_pm_qos->latency which suppose to > be the point to struct dev_pm_qos. The patch is to fix the issue. > > Signed-off-by: Lan Tianyu <tianyu.lan@intel.com> Thanks, will apply. Rafael > --- > drivers/base/power/qos.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/base/power/qos.c b/drivers/base/power/qos.c > index 3a7687a..31d3f48 100644 > --- a/drivers/base/power/qos.c > +++ b/drivers/base/power/qos.c > @@ -253,7 +253,7 @@ void dev_pm_qos_constraints_destroy(struct device *dev) > spin_unlock_irq(&dev->power.lock); > > kfree(c->notifiers); > - kfree(c); > + kfree(qos); > > out: > mutex_unlock(&dev_pm_qos_mtx); > -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-11-01 23:17 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-11-01 10:47 [PATCH 1/2] PM / Qos: Fix a free error in the dev_pm_qos_constraints_destroy() Lan Tianyu 2012-11-01 10:47 ` [PATCH 2/2] PM / Qos: Add notifier for pm qos flags Lan Tianyu 2012-11-01 14:26 ` Rafael J. Wysocki 2012-11-01 23:21 ` [PATCH] PM / QoS: Document request manipulation requirement for flags Rafael J. Wysocki 2012-11-01 14:27 ` [PATCH 1/2] PM / Qos: Fix a free error in the dev_pm_qos_constraints_destroy() Rafael J. Wysocki
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).