Messages in this thread | | | Date | Thu, 25 Jul 2019 10:47:42 +0530 | From | Viresh Kumar <> | Subject | Re: [PATCH v3 3/5] OPP: Improve require-opps linking |
| |
On 24-07-19, 21:09, Saravana Kannan wrote: > On Wed, Jul 24, 2019 at 8:07 PM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > We should be doing this whenever a new OPP table is created, and see > > if someone else was waiting for this OPP table to come alive. > > Searching the global OPP table list seems a ton more wasteful than > doing the lazy linking. I'd rather not do this.
We can see how best to optimize that, but it will be done only once while a new OPP table is created and putting stress there is the right thing to do IMO. And doing anything like that in a place like opp-set-rate is the worst one. It will be a bad choice by design if you ask me and so I am very much against that.
> > Also we > > must make sure that we do this linking only if the new OPP table has > > its own required-opps links fixed, otherwise delay further. > > This can be done. Although even without doing that, this patch is > making things better by not failing silently like it does today? Can I > do this later as a separate patch set series?
I would like this to get fixed now in a proper way, there is no hurry for a quick fix currently. No band-aids please.
> > Even then I don't want to add these checks to those places. For the > > opp-set-rate routine, add another flag to the OPP table which > > indicates if we are ready to do dvfs or not and mark it true only > > after the required-opps are all set. > > Honestly, this seems like extra memory and micro optimization without > any data to back it.
Again, opp-set-rate isn't supposed to do something like this. It shouldn't handle initializations of things, that is broken design.
> Show me data that checking all these table > pointers is noticeably slower than what I'm doing. What's the max > "required tables count" you've seen in upstream so far?
Running anything extra (specially some initialization stuff) in opp-set-rate is wrong as per me and as a Maintainer of the OPP core it is my responsibility to not allow such things to happen.
> I'd even argue that doing it the way I do might actually reduce the > cache misses/warm the cache because those pointers are going to be > searched/used right after anyway.
So you want to make the cache hot with data, by running some code at a place where it is not required to be run really, and the fact that most of the data cached may not get used anyway ? And that is an improvement somehow ?
-- viresh
| |