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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20210312034942.jh3txxoowbo2ekyn@vireshk-i7>
Date:   Fri, 12 Mar 2021 09:19:42 +0530
From:   Viresh Kumar <viresh.kumar@...aro.org>
To:     Beata Michalska <beata.michalska@....com>
Cc:     linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org,
        vireshk@...nel.org, nm@...com, sboyd@...nel.org
Subject: Re: [PATCH] opp: Invalidate current opp when draining the opp list

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