drivers/opp/core.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
From: Di Shen <di.shen@unisoc.com>
There is a race between opp_table_find_key and dev_pm_opp_add_dynamic:
dev_pm_opp_add_dynamic dev_pm_opp_find_freq_exact
_opp_add_v1 _find_key
_opp_add _opp_table_find_key
mutex_lock(opp_table->lock)
list_add(&new_opp->node, head)
mutex_unlock(opp_table->lock)
mutex_lock(opp_table->lock)
dev_pm_opp_get(opp)
kref_get(&opp->kref)
refcount_inc(&kref->refcount);
mutex_unlock(opp_table->lock)
kref_init(&new_opp->kref)
refcount_set(&kref->refcount, 1);
dev_pm_opp_put()
kref_put_mutex()
This would cause the opp be freed.
So use the mutex to protect opp's ref count.
Co-developed-by: Ling Xu <ling_ling.xu@unisoc.com>
Signed-off-by: Ling Xu <ling_ling.xu@unisoc.com>
Signed-off-by: Di Shen <di.shen@unisoc.com>
---
drivers/opp/core.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 866641666e41..34cdcf8f598d 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -2088,11 +2088,10 @@ int _opp_add(struct device *dev, struct dev_pm_opp *new_opp,
return ret;
list_add(&new_opp->node, head);
+ new_opp->opp_table = opp_table;
+ kref_init(&new_opp->kref);
}
- new_opp->opp_table = opp_table;
- kref_init(&new_opp->kref);
-
opp_debug_create_one(new_opp, opp_table);
if (!_opp_supported_by_regulators(new_opp, opp_table)) {
--
2.25.1
On 27-04-26, 20:00, Xuewen Yan wrote: > From: Di Shen <di.shen@unisoc.com> > > There is a race between opp_table_find_key and dev_pm_opp_add_dynamic: > > dev_pm_opp_add_dynamic dev_pm_opp_find_freq_exact > _opp_add_v1 _find_key > _opp_add _opp_table_find_key > mutex_lock(opp_table->lock) > list_add(&new_opp->node, head) > mutex_unlock(opp_table->lock) > mutex_lock(opp_table->lock) > dev_pm_opp_get(opp) > kref_get(&opp->kref) > refcount_inc(&kref->refcount); > mutex_unlock(opp_table->lock) > kref_init(&new_opp->kref) > refcount_set(&kref->refcount, 1); > dev_pm_opp_put() > kref_put_mutex() > > This would cause the opp be freed. > > So use the mutex to protect opp's ref count. > > Co-developed-by: Ling Xu <ling_ling.xu@unisoc.com> > Signed-off-by: Ling Xu <ling_ling.xu@unisoc.com> > Signed-off-by: Di Shen <di.shen@unisoc.com> > --- > drivers/opp/core.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) Good catch. Applied. Thanks. -- viresh
Hi Viresh, On Tue, May 5, 2026 at 11:36 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > On 27-04-26, 20:00, Xuewen Yan wrote: > > From: Di Shen <di.shen@unisoc.com> > > > > There is a race between opp_table_find_key and dev_pm_opp_add_dynamic: > > > > dev_pm_opp_add_dynamic dev_pm_opp_find_freq_exact > > _opp_add_v1 _find_key > > _opp_add _opp_table_find_key > > mutex_lock(opp_table->lock) > > list_add(&new_opp->node, head) > > mutex_unlock(opp_table->lock) > > mutex_lock(opp_table->lock) > > dev_pm_opp_get(opp) > > kref_get(&opp->kref) > > refcount_inc(&kref->refcount); > > mutex_unlock(opp_table->lock) > > kref_init(&new_opp->kref) > > refcount_set(&kref->refcount, 1); > > dev_pm_opp_put() > > kref_put_mutex() > > > > This would cause the opp be freed. > > > > So use the mutex to protect opp's ref count. > > > > Co-developed-by: Ling Xu <ling_ling.xu@unisoc.com> > > Signed-off-by: Ling Xu <ling_ling.xu@unisoc.com> > > Signed-off-by: Di Shen <di.shen@unisoc.com> > > --- > > drivers/opp/core.c | 5 ++--- > > 1 file changed, 2 insertions(+), 3 deletions(-) > > Good catch. > > Applied. Thanks. Need we add fix tag? Fixes: 7034764a1e4a (PM / OPP: Add 'struct kref' to struct dev_pm_opp) > > -- > viresh
On 07-05-26, 10:09, Xuewen Yan wrote: > Need we add fix tag? > Fixes: 7034764a1e4a (PM / OPP: Add 'struct kref' to struct dev_pm_opp) Done -- viresh
Hi Viresh,
Do you have any comments on this patch?
Thanks!
On Mon, Apr 27, 2026 at 8:01 PM Xuewen Yan <xuewen.yan@unisoc.com> wrote:
>
> From: Di Shen <di.shen@unisoc.com>
>
> There is a race between opp_table_find_key and dev_pm_opp_add_dynamic:
>
> dev_pm_opp_add_dynamic dev_pm_opp_find_freq_exact
> _opp_add_v1 _find_key
> _opp_add _opp_table_find_key
> mutex_lock(opp_table->lock)
> list_add(&new_opp->node, head)
> mutex_unlock(opp_table->lock)
> mutex_lock(opp_table->lock)
> dev_pm_opp_get(opp)
> kref_get(&opp->kref)
> refcount_inc(&kref->refcount);
> mutex_unlock(opp_table->lock)
> kref_init(&new_opp->kref)
> refcount_set(&kref->refcount, 1);
> dev_pm_opp_put()
> kref_put_mutex()
>
> This would cause the opp be freed.
>
> So use the mutex to protect opp's ref count.
>
> Co-developed-by: Ling Xu <ling_ling.xu@unisoc.com>
> Signed-off-by: Ling Xu <ling_ling.xu@unisoc.com>
> Signed-off-by: Di Shen <di.shen@unisoc.com>
> ---
> drivers/opp/core.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> index 866641666e41..34cdcf8f598d 100644
> --- a/drivers/opp/core.c
> +++ b/drivers/opp/core.c
> @@ -2088,11 +2088,10 @@ int _opp_add(struct device *dev, struct dev_pm_opp *new_opp,
> return ret;
>
> list_add(&new_opp->node, head);
> + new_opp->opp_table = opp_table;
> + kref_init(&new_opp->kref);
> }
>
> - new_opp->opp_table = opp_table;
> - kref_init(&new_opp->kref);
> -
> opp_debug_create_one(new_opp, opp_table);
>
> if (!_opp_supported_by_regulators(new_opp, opp_table)) {
> --
> 2.25.1
>
© 2016 - 2026 Red Hat, Inc.