Re: [PATCH] opp: Invalidate current opp when draining the opp list

From: Viresh Kumar <viresh.kumar@linaro.org>
To: Beata Michalska <beata.michalska@arm.com>
Cc: linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org,
	vireshk@kernel.org, nm@ti.com, sboyd@kernel.org
Subject: Re: [PATCH] opp: Invalidate current opp when draining the opp list
Date: Fri, 12 Mar 2021 09:19:42 +0530	[thread overview]
Message-ID: <20210312034942.jh3txxoowbo2ekyn@vireshk-i7> (raw)
In-Reply-To: <20210310230300.GA26976@e120325.cambridge.arm.com>

On 10-03-21, 23:03, Beata Michalska wrote:
> > diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> > index c2689386a906..150be4c28c99 100644
> > --- a/drivers/opp/core.c
> > +++ b/drivers/opp/core.c
> > @@ -1492,7 +1492,11 @@ static struct dev_pm_opp *_opp_get_next(struct opp_table *opp_table,
> >  
> >  	mutex_lock(&opp_table->lock);
> >  	list_for_each_entry(temp, &opp_table->opp_list, node) {
> > -		if (dynamic == temp->dynamic) {
> > +		/*
> > +		 * Refcount must be dropped only once for each OPP by OPP core,
> > +		 * do that with help of "removed" flag.
> > +		 */
> > +		if (!temp->removed && dynamic == temp->dynamic) {
> >  			opp = temp;
> >  			break;
> >  		}
> How about tweaking the _opp_get_next to use list_for_each_entry_continue
> instead ? It would eliminate the need of tracking the 'removed' status
> and could save few cycles as it wouldn't have to go through the list
> starting from it's beginning each time it is being called.
> Happy to draft another version.

I tried that as well, but the problem is that we need to drop locks in
between and if the next OPP somehow gets freed or another one gets
added there, we can be in trouble. To make this work without any side
effects, the new field was kind of required.

-- 
viresh