| Message ID | 20230804214051.136268-1-robdclark@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [RFC] PM / QoS: Decouple request alloc from dev_pm_qos_mtx (alternative solution) | expand |
On Fri, Aug 4, 2023 at 11:41 PM Rob Clark <robdclark@gmail.com> wrote: > > From: Rob Clark <robdclark@chromium.org> > > Similar to the previous patch, move the allocation out from under > dev_pm_qos_mtx, by speculatively doing the allocation and handle > any race after acquiring dev_pm_qos_mtx by freeing the redundant > allocation. > > Suggested-by: Rafael J. Wysocki <rafael@kernel.org> > Signed-off-by: Rob Clark <robdclark@chromium.org> > --- > This is an alternative to https://patchwork.freedesktop.org/patch/551417/?series=115028&rev=4 > > So, this does _slightly_ change error paths, for ex > dev_pm_qos_update_user_latency_tolerance() will now allocate > dev->power.qos in some error cases. But this seems harmless? It is harmless AFAICS. > A slightly more complicated version of this could conserve the > previous error path behavior, but I figured I'd try the simpler > thing first. Good choice! > drivers/base/power/qos.c | 13 +++++++------ > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/drivers/base/power/qos.c b/drivers/base/power/qos.c > index 1b73a704aac1..c7ba85e89c42 100644 > --- a/drivers/base/power/qos.c > +++ b/drivers/base/power/qos.c > @@ -920,8 +920,12 @@ s32 dev_pm_qos_get_user_latency_tolerance(struct device *dev) > int dev_pm_qos_update_user_latency_tolerance(struct device *dev, s32 val) > { > struct dev_pm_qos *qos = dev_pm_qos_constraints_allocate(); > + struct dev_pm_qos_request *req = NULL; > int ret = 0; > > + if (!dev->power.qos->latency_tolerance_req) > + req = kzalloc(sizeof(*req), GFP_KERNEL); > + > mutex_lock(&dev_pm_qos_mtx); > > dev_pm_qos_constraints_set(dev, qos); > @@ -935,8 +939,6 @@ int dev_pm_qos_update_user_latency_tolerance(struct device *dev, s32 val) > goto out; > > if (!dev->power.qos->latency_tolerance_req) { > - struct dev_pm_qos_request *req; > - > if (val < 0) { > if (val == PM_QOS_LATENCY_TOLERANCE_NO_CONSTRAINT) > ret = 0; > @@ -944,17 +946,15 @@ int dev_pm_qos_update_user_latency_tolerance(struct device *dev, s32 val) > ret = -EINVAL; > goto out; > } > - req = kzalloc(sizeof(*req), GFP_KERNEL); > if (!req) { > ret = -ENOMEM; > goto out; > } > ret = __dev_pm_qos_add_request(dev, req, DEV_PM_QOS_LATENCY_TOLERANCE, val); > - if (ret < 0) { > - kfree(req); > + if (ret < 0) > goto out; > - } > dev->power.qos->latency_tolerance_req = req; > + req = NULL; > } else { > if (val < 0) { > __dev_pm_qos_drop_user_request(dev, DEV_PM_QOS_LATENCY_TOLERANCE); > @@ -966,6 +966,7 @@ int dev_pm_qos_update_user_latency_tolerance(struct device *dev, s32 val) > > out: > mutex_unlock(&dev_pm_qos_mtx); > + kfree(req); > return ret; > } > EXPORT_SYMBOL_GPL(dev_pm_qos_update_user_latency_tolerance); > -- Yes, something like this, please!
diff --git a/drivers/base/power/qos.c b/drivers/base/power/qos.c index 1b73a704aac1..c7ba85e89c42 100644 --- a/drivers/base/power/qos.c +++ b/drivers/base/power/qos.c @@ -920,8 +920,12 @@ s32 dev_pm_qos_get_user_latency_tolerance(struct device *dev) int dev_pm_qos_update_user_latency_tolerance(struct device *dev, s32 val) { struct dev_pm_qos *qos = dev_pm_qos_constraints_allocate(); + struct dev_pm_qos_request *req = NULL; int ret = 0; + if (!dev->power.qos->latency_tolerance_req) + req = kzalloc(sizeof(*req), GFP_KERNEL); + mutex_lock(&dev_pm_qos_mtx); dev_pm_qos_constraints_set(dev, qos); @@ -935,8 +939,6 @@ int dev_pm_qos_update_user_latency_tolerance(struct device *dev, s32 val) goto out; if (!dev->power.qos->latency_tolerance_req) { - struct dev_pm_qos_request *req; - if (val < 0) { if (val == PM_QOS_LATENCY_TOLERANCE_NO_CONSTRAINT) ret = 0; @@ -944,17 +946,15 @@ int dev_pm_qos_update_user_latency_tolerance(struct device *dev, s32 val) ret = -EINVAL; goto out; } - req = kzalloc(sizeof(*req), GFP_KERNEL); if (!req) { ret = -ENOMEM; goto out; } ret = __dev_pm_qos_add_request(dev, req, DEV_PM_QOS_LATENCY_TOLERANCE, val); - if (ret < 0) { - kfree(req); + if (ret < 0) goto out; - } dev->power.qos->latency_tolerance_req = req; + req = NULL; } else { if (val < 0) { __dev_pm_qos_drop_user_request(dev, DEV_PM_QOS_LATENCY_TOLERANCE); @@ -966,6 +966,7 @@ int dev_pm_qos_update_user_latency_tolerance(struct device *dev, s32 val) out: mutex_unlock(&dev_pm_qos_mtx); + kfree(req); return ret; } EXPORT_SYMBOL_GPL(dev_pm_qos_update_user_latency_tolerance);