Messages in this thread | | | Date | Tue, 1 Dec 2020 15:22:24 +0000 | From | Mel Gorman <> | Subject | Re: [PATCH] cpuidle: Select polling interval based on a c-state with a longer target residency |
| |
On Tue, Dec 01, 2020 at 04:08:02PM +0100, Rafael J. Wysocki wrote: > > > Also this is about certain drivers only which support the "polling > > > idle state" (the ACPI one and intel_idle only AFAICS). So I'm not > > > sure about the framework-level tunable here. > > > > > > Moreover, to be precise, that value is the maximum time to do the > > > polling (in one go) in the case when requesting any "physical" idle > > > states is likely to hurt energy-efficiency or latency. In particular, > > > it doesn't mean that idle CPUs will do the idle polling every time. > > > > > > > At first I was nodding along and thinking "sure". Then I started > > thinking about what the configuration space then looks like and how a > > user might reasonably interpret it. You were right during the review of > > the first version, it's a mess because it's driver specific and difficult > > to interpret even on a per-driver basis because there is no control of > > when a rescheduling event may occur. > > Indeed. > > > You suggest making poll=0 would be valid but that might be interpreted > > as being equivalent to idle=poll on x86 which is not the same thing. > > processor_idle and intel_idle would have understandable semantics if the > > parameter was maxpoll but it's not as understandable for haltpoll. > > Well, my point was basically that if the plan was to add a boot > parameter to control the polling behavior, it would be prudent to also > allow the admin to specify that they didn't want any polling at all. > > But frankly I was hoping to drive you away from that idea which seems > to have worked. :-) >
Yes, it most certainly worked. Thanks for repeating yourself in a different way so that your concern could penetrate my thick skull :D
> > Finally, the parameter partially ties us into the current > > implementation. For example, the polling loop is based on clock time but > > we know looking up the clock is costly in itself so it's very granular > > based on the magic "check every 200 loops" logic meaning we can go over > > the expected maxiumum polling inverval. If we ever changed that into a > > calibration loop to estimate the number of loops then the polling interval > > changes slightly even for the same parameter as we no longer depend on the > > granularity of calling local_clock. If we ever decided to use adaptive > > polling similar to haltpoll then the behaviour changes again resulting > > in bugs because the driver.poll parameter means something new. > > Right. > > > Using min_cstate was definitely a hazard because it showed up in both > > microbenchmarks and real workloads but you were right, lets only > > introduce a tunable when and if there is no other choice in the matter. > > > > So, informally the following patch is the next candidate. I'm happy to > > resend it as a separate mail if you prefer and think the patch is ok. > > I actually can apply it right away, so no need to resend. >
Thanks very much.
-- Mel Gorman SUSE Labs
| |