On 24-07-19, 21:09, Saravana Kannan wrote: > On Wed, Jul 24, 2019 at 8:07 PM Viresh Kumar <[email protected]> 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
