On Wed, Jul 24, 2019 at 10:17 PM Viresh Kumar <viresh.ku...@linaro.org> wrote: > > On 24-07-19, 21:09, Saravana Kannan wrote: > > On Wed, Jul 24, 2019 at 8:07 PM Viresh Kumar <viresh.ku...@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.
Doing operations lazily right before they are needed isn't something new in the kernel. It's done all over the place (VFP save/restore?). It's not worth arguing though -- so I'll agree to disagree but follow the Maintainer's preference. > > 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 ? My point is that both of us are hypothesizing and for some micro-optimization like this, data is needed. -Saravana