Reduce migrations and unnecessary spreading of load to multiple CPUs

The select_idle_sibling (SIS) rewrite in commit 10e2f1acd010 ("sched/core:
Rewrite and improve select_idle_siblings()") replaced a domain iteration with a search that broadly speaking does a wrapped walk of the scheduler domain sharing a last-level-cache. While this had a number of improvements, one consequence is that two tasks that share a waker/wakee relationship push each other around a socket. Even though two tasks may be active, all cores are evenly used. This is great from a search perspective and spreads a load across individual cores but it has adverse consequences for cpufreq. As each CPU has relatively low utilisation, cpufreq may decide the utilisation is too low to used a higher P-state and overall computation throughput suffers.

While individual cpufreq and cpuidle drivers may compensate by artifically boosting P-state (at c0) or avoiding lower C-states (during idle), it does not help if hardware-based cpufreq (e.g. HWP) is used.

Quoted 10 lines · Mel Gorman

The select_idle_sibling (SIS) rewrite in commit 10e2f1acd010 ("sched/core:
Rewrite and improve select_idle_siblings()") replaced a domain iteration with a search that broadly speaking does a wrapped walk of the scheduler domain sharing a last-level-cache. While this had a number of improvements, one consequence is that two tasks that share a waker/wakee relationship push each other around a socket. Even though two tasks may be active, all cores are evenly used. This is great from a search perspective and spreads a load across individual cores but it has adverse consequences for cpufreq. As each CPU has relatively low utilisation, cpufreq may decide the utilisation is too low to used a higher P-state and overall computation throughput suffers.

Quoted 3 lines

While individual cpufreq and cpuidle drivers may compensate by artifically boosting P-state (at c0) or avoiding lower C-states (during idle), it does not help if hardware-based cpufreq (e.g. HWP) is used.

Not saying this patch is bad; but Rafael / Srinivas we really should do better. Why isn't cpufreq (esp. sugov) fixing this? HWP or not, we can still give it hints, and it looks like we're not doing that.

Mel, what hardware are you testing this on?

Quoted 4 lines · Peter Zijlstra

Not saying this patch is bad; but Rafael / Srinivas we really should do better. Why isn't cpufreq (esp. sugov) fixing this? HWP or not, we can still give it hints, and it looks like we're not doing that.

I'm not sure if HWP can fix it because of the per-cpu nature of its decisions. I believe it can only give the most basic of hints to hardware like an energy performance profile or bias (EPP and EPB respectively). Of course HWP can be turned off but not many people can detect that it's an appropriate decision, or even desirable, and there is always the caveat that disabling it increases the system CPU footprint.

Quoted 7 lines · Mel Gorman

Quoted 4 lines · Peter Zijlstra

Not saying this patch is bad; but Rafael / Srinivas we really should do better. Why isn't cpufreq (esp. sugov) fixing this? HWP or not, we can still give it hints, and it looks like we're not doing that.

Quoted 6 lines

I'm not sure if HWP can fix it because of the per-cpu nature of its decisions. I believe it can only give the most basic of hints to hardware like an energy performance profile or bias (EPP and EPB respectively). Of course HWP can be turned off but not many people can detect that it's an appropriate decision, or even desirable, and there is always the caveat that disabling it increases the system CPU footprint.

IA32_HWP_REQUEST has "Minimum_Performance", "Maximum_Performance" and "Desired_Performance" fields which can be used to give explicit frequency hints. And we really _should_ be doing that.

Because, esp. in this scenario; a task migrating; the hardware really can't do anything sensible, whereas the OS _knows_.

Quoted 2 lines

Because, esp. in this scenario; a task migrating; the hardware really can't do anything sensible, whereas the OS _knows_.

Potentially yes. One option without HWP would be to track utilisation for a task or artifically boost it for a short period after migration so a higher p-state is potentially selected. With HWP, a hint would have to be given to the hardware to try select a higher frequency but I've no idea how expensive that is or how it would behave on different implementations of HWP. It may also be a game of whack-a-mole trying to get every cpufreq configuration correct.

One advantage of using fewer cores is that it should work regardless of cpufreq driver.

Quoted 10 lines · Mel Gorman

Quoted 2 lines

Because, esp. in this scenario; a task migrating; the hardware really can't do anything sensible, whereas the OS _knows_.

Potentially yes. One option without HWP would be to track utilisation for a task or artifically boost it for a short period after migration so a higher p-state is potentially selected. With HWP, a hint would have to be given to the hardware to try select a higher frequency but I've no idea how expensive that is or how it would behave on different implementations of HWP. It may also be a game of whack-a-mole trying to get every cpufreq configuration correct.

We have much of this infrastructure and have been working on improving it [1]. We already track per task utilization and feed it into a cpufreq governor (schedutil).

Quoted 2 lines

One advantage of using fewer cores is that it should work regardless of cpufreq driver.

Sure, and I started out by saying this patch isn't necessarily bad; but I think our 'use' [2] of HWP _is_.

Quoted 7 lines · Mel Gorman

Quoted 4 lines · Peter Zijlstra

Not saying this patch is bad; but Rafael / Srinivas we really should do better. Why isn't cpufreq (esp. sugov) fixing this? HWP or not, we can still give it hints, and it looks like we're not doing that.

Quoted 6 lines

I'm not sure if HWP can fix it because of the per-cpu nature of its decisions. I believe it can only give the most basic of hints to hardware like an energy performance profile or bias (EPP and EPB respectively). Of course HWP can be turned off but not many people can detect that it's an appropriate decision, or even desirable, and there is always the caveat that disabling it increases the system CPU footprint.

IA32_HWP_REQUEST has "Minimum_Performance", "Maximum_Performance" and "Desired_Performance" fields which can be used to give explicit frequency hints. And we really _should_ be doing that.

Because, esp. in this scenario; a task migrating; the hardware really can't do anything sensible, whereas the OS _knows_.

Quoted 6 lines · CET Peter Zijlstra

IA32_HWP_REQUEST has "Minimum_Performance", "Maximum_Performance" and "Desired_Performance" fields which can be used to give explicit frequency hints. And we really _should_ be doing that.

Because, esp. in this scenario; a task migrating; the hardware really can't do anything sensible, whereas the OS _knows_.

But IA32_HWP_REQUEST is not a cheap MSR to write to.

Quoted 9 lines · Rafael J. Wysocki

Quoted 6 lines · CET Peter Zijlstra

IA32_HWP_REQUEST has "Minimum_Performance", "Maximum_Performance" and "Desired_Performance" fields which can be used to give explicit frequency hints. And we really _should_ be doing that.

Because, esp. in this scenario; a task migrating; the hardware really can't do anything sensible, whereas the OS _knows_.

Quoted 1 line

But IA32_HWP_REQUEST is not a cheap MSR to write to.

That just means we might need to throttle writing to it, like it already does for the regular pstate (PERF_CTRL) msr in any case (also, is that a cheap msr?)

Not touching it at all seems silly.

Quoted 10 lines · Rafael J. Wysocki

Quoted 8 lines · CET Peter Zijlstra

IA32_HWP_REQUEST has "Minimum_Performance", "Maximum_Performance" and
"Desired_Performance" fields which can be used to give explicit frequency hints. And we really _should_ be doing that.

Because, esp. in this scenario; a task migrating; the hardware really
can't do anything sensible, whereas the OS _knows_.

Quoted 1 line

But IA32_HWP_REQUEST is not a cheap MSR to write to.

That just means we might need to throttle writing to it, like it already
does for the regular pstate (PERF_CTRL) msr in any case (also, is that a
cheap msr?)

Not touching it at all seems silly.

But now that you made me look, intel_pstate_hwp_set() is horrible crap.
You should _never_ do things like:

rdmsr_on_cpu()
/* frob value */
wrmsr_on_cpu()

That's insane.

Much more throttling required compared to PERF_CTL. MSR_HWP_REQUEST is much slower compared to PERF_CTL (as high as 10:1).

Quoted 9 lines · Rafael J. Wysocki

Quoted 6 lines · CET Peter Zijlstra

IA32_HWP_REQUEST has "Minimum_Performance", "Maximum_Performance" and "Desired_Performance" fields which can be used to give explicit frequency hints. And we really _should_ be doing that.

Because, esp. in this scenario; a task migrating; the hardware really can't do anything sensible, whereas the OS _knows_.

Quoted 1 line

But IA32_HWP_REQUEST is not a cheap MSR to write to.

That just means we might need to throttle writing to it, like it already does for the regular pstate (PERF_CTRL) msr in any case (also, is that a cheap msr?)

Not touching it at all seems silly.

But now that you made me look, intel_pstate_hwp_set() is horrible crap. You should _never_ do things like:

rdmsr_on_cpu()
/* frob value */
wrmsr_on_cpu()

That's insane.

Quoted 16 lines · CET Peter Zijlstra

Quoted 10 lines · Rafael J. Wysocki

Quoted 6 lines · CET Peter Zijlstra

IA32_HWP_REQUEST has "Minimum_Performance", "Maximum_Performance" and "Desired_Performance" fields which can be used to give explicit frequency hints. And we really _should_ be doing that.

Because, esp. in this scenario; a task migrating; the hardware really can't do anything sensible, whereas the OS _knows_.

But IA32_HWP_REQUEST is not a cheap MSR to write to.

That just means we might need to throttle writing to it, like it already does for the regular pstate (PERF_CTRL) msr in any case (also, is that a cheap msr?)

Quoted 1 line

Not touching it at all seems silly.

OK

So what field precisely would you touch? "desired"? If so, does that actually guarantee anything to happen?

Quoted 8 lines

But now that you made me look, intel_pstate_hwp_set() is horrible crap. You should _never_ do things like:

rdmsr_on_cpu()
/* frob value */
wrmsr_on_cpu()

That's insane.

I guess you mean it does too many IPIs? Or that it shouldn't do any IPIs at all?

Quoted 28 lines · Rafael J. Wysocki

Quoted 25 lines · CET Peter Zijlstra

Quoted 11 lines · Rafael J. Wysocki

Quoted 7 lines · CET Peter Zijlstra

IA32_HWP_REQUEST has "Minimum_Performance", "Maximum_Performance" and "Desired_Performance" fields which can be used to give explicit
frequency hints. And we really _should_ be doing that.

Quoted 4 lines

Quoted 3 lines

Because, esp. in this scenario; a task migrating; the hardware really
can't do anything sensible, whereas the OS _knows_.

But IA32_HWP_REQUEST is not a cheap MSR to write to.

That just means we might need to throttle writing to it, like it already
does for the regular pstate (PERF_CTRL) msr in any case (also, is that a
cheap msr?)

Not touching it at all seems silly.

Quoted 5 lines

OK

So what field precisely would you touch?  "desired"?  If so, does that actually
guarantee anything to happen?

No idea, desired would be the one I would start with, it matches with the intent here. But I've no idea what our current HWP implementation actually does with it.

Quoted 31 lines · Peter Zijlstra

Quoted 30 lines · Rafael J. Wysocki

Quoted 17 lines · CET Peter Zijlstra

Quoted 14 lines · Rafael J. Wysocki

Quoted 10 lines · CET Peter Zijlstra

IA32_HWP_REQUEST has "Minimum_Performance", "Maximum_Performance" and "Desired_Performance" fields which can be used to give explicit
frequency hints. And we really _should_ be doing that.

Because, esp. in this scenario; a task migrating; the hardware really
can't do anything sensible, whereas the OS _knows_.

Quoted 8 lines

Quoted 1 line

But IA32_HWP_REQUEST is not a cheap MSR to write to.

That just means we might need to throttle writing to it, like it already
does for the regular pstate (PERF_CTRL) msr in any case (also, is that a
cheap msr?)

Not touching it at all seems silly.

OK

Quoted 6 lines

Quoted 3 lines

So what field precisely would you touch? "desired"? If so, does that actually
guarantee anything to happen?

No idea, desired would be the one I would start with, it matches with the intent here. But I've no idea what our current HWP implementation actually does with it.

Desired !=0 will disable HWP autonomous mode of frequency selection.

Quoted 45 lines · CET Srinivas Pandruvada

Quoted 43 lines · Peter Zijlstra

Quoted 40 lines · Rafael J. Wysocki

Quoted 29 lines · CET Peter Zijlstra

Quoted 13 lines · Rafael J. Wysocki

Quoted 8 lines · CET Peter Zijlstra

IA32_HWP_REQUEST has "Minimum_Performance", "Maximum_Performance" and "Desired_Performance" fields which can be used to give explicit
frequency hints. And we really _should_ be doing that.

Quoted 4 lines

Quoted 3 lines

Because, esp. in this scenario; a task migrating; the hardware really
can't do anything sensible, whereas the OS _knows_.

But IA32_HWP_REQUEST is not a cheap MSR to write to.

That just means we might need to throttle writing to it, like it
already
does for the regular pstate (PERF_CTRL) msr in any case (also, is
that a
cheap msr?)

Quoted 1 line

Not touching it at all seems silly.

OK

So what field precisely would you touch?  "desired"?  If so, does
that actually
guarantee anything to happen?

Quoted 7 lines

Quoted 5 lines

No idea, desired would be the one I would start with, it matches with
the intent here. But I've no idea what our current HWP implementation
actually does with it.

Desired !=0 will disable HWP autonomous mode of frequency selection.

But I don't think it will just run at "desired" then, will it?

Quoted 8 lines

Quoted 7 lines

Quoted 5 lines

No idea, desired would be the one I would start with, it matches with
the intent here. But I've no idea what our current HWP implementation
actually does with it.

Desired !=0 will disable HWP autonomous mode of frequency selection.

But I don't think it will just run at "desired" then, will it?

HWP all are these hints only not a guarantee.

There are totally different way HWP is handled in client an servers. If you set desired all heuristics they collected will be dumped, so they suggest don't set desired when you are in autonomous mode. If we really want a boost set the EPP. We know that EPP makes lots of measurable difference.

Quoted 12 lines · Srinivas Pandruvada

Quoted 11 lines

Quoted 9 lines

Quoted 6 lines

No idea, desired would be the one I would start with, it matches
with
the intent here. But I've no idea what our current HWP implementation
actually does with it.

Desired !=0 will disable HWP autonomous mode of frequency selection.

But I don't think it will just run at "desired" then, will it?

Quoted 1 line

HWP all are these hints only not a guarantee.

Sure, but the lack on detection when tasks are low utilisation but still
latency/throughput sensitive is problematic. Users shouldn't have to know they need to disable HWP or set performance goernor out of the box.
It's only going to get worse as sockets get larger.

Quoted 9 lines

There are totally different way HWP is handled in client an servers.
If you set desired all heuristics they collected will be dumped, so they suggest don't set desired when you are in autonomous mode. If we
really want a boost set the EPP. We know that EPP makes lots of measurable difference.

Sure boosting EPP makes a difference -- it's essentially what the performance
goveror does and I know that can be done by a user but it's still basically a
cop-out. Default performance for low utilisation or lightly loaded machines
is poor. Maybe it should be set based on the ACPI preferred profile but
that information is not always available. It would be nice if *some* sort of hint about new migrations or tasks waking from IO would be desirable.

Quoted 6 lines

Sure, but the lack on detection when tasks are low utilisation but still
latency/throughput sensitive is problematic. Users shouldn't have to know they need to disable HWP or set performance goernor out of the box.
It's only going to get worse as sockets get larger.

I am not saying that we shouldn't do anything. Can you give me some workloads which you care the most?

Quoted 10 lines

Quoted 9 lines

There are totally different way HWP is handled in client an servers.
If you set desired all heuristics they collected will be dumped, so they suggest don't set desired when you are in autonomous mode. If we
really want a boost set the EPP. We know that EPP makes lots of measurable difference.

Quoted 11 lines

Sure boosting EPP makes a difference -- it's essentially what the performance
goveror does and I know that can be done by a user but it's still basically a
cop-out. Default performance for low utilisation or lightly loaded machines
is poor. Maybe it should be set based on the ACPI preferred profile but
that information is not always available. It would be nice if *some* sort of hint about new migrations or tasks waking from IO would be desirable.

EPP is a range not a single value. So you don't need to make EPP=0 as a performance governor. PeterZ gave me some scheduler change to experiment, which can be used as hint to play with EPP.

Quoted 10 lines · Srinivas Pandruvada

Quoted 9 lines

Sure, but the lack on detection when tasks are low utilisation but
still
latency/throughput sensitive is problematic. Users shouldn't have to
know they need to disable HWP or set performance goernor out of the
box.
It's only going to get worse as sockets get larger.

Quoted 3 lines

I am not saying that we shouldn't do anything. Can you give me some workloads which you care the most?

The proprietary workloads I'm aware of are useless to the discussion as they cannot be trivially reproduced and are typically only available
under NDA. However, hints can be gotten by looking at the number of cases
where recommended tunings limits C-states, set the performance governor,
alter intel_pstate setpoint (if not HWP) etc.

For the purposes of illustration, dbench at low thread counts does a reasonable job even though it's not that interesting a workload in general. With ext4 in particular, the journalling thread interactions bounce tasks around the machine and the short sleeps for IO both combine
to have relatively low utilisation on individual CPUs. It's less pronounced
on xfs as it bounces less due to using kworkers instead of kthreads.

Quoted 28 lines

Quoted 12 lines

Quoted 11 lines

There are totally different way HWP is handled in client an servers.
If you set desired all heuristics they collected will be dumped, so
they suggest don't set desired when you are in autonomous mode. If
we
really want a boost set the EPP. We know that EPP makes lots of measurable difference.

Quoted 15 lines

Sure boosting EPP makes a difference -- it's essentially what the performance
goveror does and I know that can be done by a user but it's still basically a
cop-out. Default performance for low utilisation or lightly loaded
machines
is poor. Maybe it should be set based on the ACPI preferred profile
but
that information is not always available. It would be nice if *some*
sort of hint about new migrations or tasks waking from IO would be
desirable.

Quoted 5 lines

EPP is a range not a single value. So you don't need to make EPP=0 as a
performance governor. PeterZ gave me some scheduler change to experiment, which can be used as hint to play with EPP.

I know EPP is a range, default from bios usually appear to be 6 or 7 but
I didn't do much experiementation to see if there is another value that
works better. Even if there is, the default may need to change as not many
people even know what EPP is or how it should be tuned.

Quoted 7 lines

I know EPP is a range, default from bios usually appear to be 6 or 7 but
I didn't do much experiementation to see if there is another value that
works better. Even if there is, the default may need to change as not many
people even know what EPP is or how it should be tuned.

I think you are talking about EPB not EPP because of ranges you mentioned here. EPP is a value from 0 to 255. EPP is part of HWP_REQUEST MSR.
EPB with HWP is used only in Broadwell server. I think you are using Skylake here.

Quoted 7 lines · Srinivas Pandruvada

Quoted 7 lines

Quoted 7 lines

Quoted 5 lines

No idea, desired would be the one I would start with, it matches with
the intent here. But I've no idea what our current HWP implementation
actually does with it.

Desired !=0 will disable HWP autonomous mode of frequency selection.

Quoted 2 lines

Quoted 1 line

But I don't think it will just run at "desired" then, will it?

HWP all are these hints only not a guarantee.

Sure, but the lack on detection when tasks are low utilisation but still latency/throughput sensitive is problematic. Users shouldn't have to know they need to disable HWP or set performance goernor out of the box. It's only going to get worse as sockets get larger.

Quoted 6 lines

There are totally different way HWP is handled in client an servers. If you set desired all heuristics they collected will be dumped, so they suggest don't set desired when you are in autonomous mode. If we really want a boost set the EPP. We know that EPP makes lots of measurable difference.

Sure boosting EPP makes a difference -- it's essentially what the performance goveror does and I know that can be done by a user but it's still basically a cop-out. Default performance for low utilisation or lightly loaded machines is poor. Maybe it should be set based on the ACPI preferred profile but that information is not always available. It would be nice if *some* sort of hint about new migrations or tasks waking from IO would be desirable.

Quoted 54 lines · Rafael J. Wysocki

Quoted 52 lines · CET Srinivas Pandruvada

Quoted 43 lines · Peter Zijlstra

Quoted 33 lines · Rafael J. Wysocki

Quoted 29 lines · CET Peter Zijlstra

Quoted 13 lines · Rafael J. Wysocki

Quoted 8 lines · CET Peter Zijlstra

IA32_HWP_REQUEST has "Minimum_Performance", "Maximum_Performance" and "Desired_Performance" fields which can be used to give explicit
frequency hints. And we really _should_ be doing that.

Quoted 4 lines

Quoted 3 lines

Because, esp. in this scenario; a task migrating; the hardware really
can't do anything sensible, whereas the OS _knows_.

But IA32_HWP_REQUEST is not a cheap MSR to write to.

That just means we might need to throttle writing to it, like it
already
does for the regular pstate (PERF_CTRL) msr in any case (also, is
that a
cheap msr?)

Quoted 7 lines

Quoted 1 line

Not touching it at all seems silly.

OK

So what field precisely would you touch? "desired"? If so, does
that actually
guarantee anything to happen?

Quoted 5 lines

No idea, desired would be the one I would start with, it matches with
the intent here. But I've no idea what our current HWP implementation
actually does with it.

Desired !=0 will disable HWP autonomous mode of frequency selection.

Quoted 1 line

But I don't think it will just run at "desired" then, will it?

HWP all are these hints only not a guarantee.
There are totally different way HWP is handled in client an servers. If you set desired all heuristics they collected will be dumped, so they suggest don't set desired when you are in autonomous mode. If we really want a boost set the EPP. We know that EPP makes lots of measurable difference.

Quoted 17 lines · Rafael J. Wysocki

Quoted 16 lines · CET Peter Zijlstra

Quoted 10 lines · Rafael J. Wysocki

Quoted 6 lines · CET Peter Zijlstra

IA32_HWP_REQUEST has "Minimum_Performance", "Maximum_Performance" and "Desired_Performance" fields which can be used to give explicit frequency hints. And we really _should_ be doing that.

Because, esp. in this scenario; a task migrating; the hardware really can't do anything sensible, whereas the OS _knows_.

But IA32_HWP_REQUEST is not a cheap MSR to write to.

That just means we might need to throttle writing to it, like it already does for the regular pstate (PERF_CTRL) msr in any case (also, is that a cheap msr?)

Quoted 6 lines

Quoted 1 line

Not touching it at all seems silly.

OK

So what field precisely would you touch? "desired"? If so, does that actually guarantee anything to happen?

No idea, desired would be the one I would start with, it matches with the intent here. But I've no idea what our current HWP implementation actually does with it.

Quoted 7 lines

Quoted 7 lines

But now that you made me look, intel_pstate_hwp_set() is horrible crap. You should _never_ do things like:

rdmsr_on_cpu()
/* frob value */
wrmsr_on_cpu()

Quoted 4 lines

Quoted 1 line

That's insane.

I guess you mean it does too many IPIs? Or that it shouldn't do any IPIs at all?

Yes, too many synchronous IPIs, which themselves are typically already more expensive than the MSR access.

At one point I looked to getting rid of the *msr_on_cpu() crud entirely, but there's just too much users out there I didn't feel like touching.

If you really care you can do async IPIs and do a custom serialization that only waits when you do back-to-back things, which should be fairly uncommon I'd think.

Quoted 3 lines

If you really care you can do async IPIs and do a custom serialization that only waits when you do back-to-back things, which should be fairly uncommon I'd think.

In this particular case we don't want to return to user space before the MSR is actually written with the new value.

+}

drivers/cpufreq/intel_pstate.c static void intel_pstate_hwp_set(unsigned int cpu) · line 779

Quoted 9 lines · Peter Zijlstra

drivers/cpufreq/intel_pstate.c +9 -0

+static void __intel_pstate_hwp_set_desired(int val)
+{
+	u64 value;
+
+	value = rdmsrl(MSR_HWP_REQUEST);
+	value &= ~GENMASK_ULL(23, 16);
+	value |= (val & 0xff) << 16;
+	wrmsrl(MSR_HWP_REQUEST, val);
+}

Also, if we keep a software shadow of that MSR, we can avoid the rdmsr, which might also help.

Quoted 14 lines · Peter Zijlstra

drivers/cpufreq/intel_pstate.c static void intel_pstate_hwp_set(unsigned int cpu) · line 779

Quoted 9 lines · Peter Zijlstra

drivers/cpufreq/intel_pstate.c +9 -0

+static void __intel_pstate_hwp_set_desired(int val)
+{
+	u64 value;
+
+	value = rdmsrl(MSR_HWP_REQUEST);
+	value &= ~GENMASK_ULL(23, 16);
+	value |= (val & 0xff) << 16;
+	wrmsrl(MSR_HWP_REQUEST, val);
+}

Also, if we keep a software shadow of that MSR, we can avoid the rdmsr, which might also help.

The reason we don't keep a software shadow as users can use x86-energy- perf utility or via BMC to adjust. CCed to Len.

Quoted 15 lines · Srinivas Pandruvada

Quoted 14 lines · Peter Zijlstra

drivers/cpufreq/intel_pstate.c static void intel_pstate_hwp_set(unsigned int cpu) · line 779

Quoted 9 lines · Peter Zijlstra

drivers/cpufreq/intel_pstate.c +9 -0

+static void __intel_pstate_hwp_set_desired(int val)
+{
+	u64 value;
+
+	value = rdmsrl(MSR_HWP_REQUEST);
+	value &= ~GENMASK_ULL(23, 16);
+	value |= (val & 0xff) << 16;
+	wrmsrl(MSR_HWP_REQUEST, val);
+}

Also, if we keep a software shadow of that MSR, we can avoid the rdmsr, which might also help.

Quoted 2 lines

The reason we don't keep a software shadow as users can use x86-energy- perf utility or via BMC to adjust. CCed to Len.

Total NAK on that. People using /dev/msr to prod at state get to keep whatever pieces they end up with.

We _really_ should make /dev/msr taint the kernel on write. Ingo, Thomas, can we pretty please just do that?

Quoted 2 lines

Yes, too many synchronous IPIs, which themselves are typically already more expensive than the MSR access.

We could do all of the updates in one IPI (as Srinivas said), but it would be more code, and custom code for that matter.

Is this really worth it for a slow path like this one?

Quoted 12 lines

Quoted 11 lines

Quoted 8 lines

But now that you made me look, intel_pstate_hwp_set() is horrible crap. You should _never_ do things like:

rdmsr_on_cpu()
/* frob value */
wrmsr_on_cpu()

That's insane.

I guess you mean it does too many IPIs? Or that it shouldn't do any IPIs at all?

Quoted 2 lines

Yes, too many synchronous IPIs, which themselves are typically already more expensive than the MSR access.

We could do all of the updates in one IPI (as Srinivas said), but it would be more code, and custom code for that matter.

Is this really worth it for a slow path like this one?

Quoted 1 line · Peter Zijlstra

Mel, what hardware are you testing this on?

The primary one was a single socket skylake machine with 8 threads (HT enabled).

Quoted 6 lines · Mel Gorman

Quoted 1 line · Peter Zijlstra

Mel, what hardware are you testing this on?

The primary one was a single socket skylake machine with 8 threads (HT enabled).

I took it for a spin in a 2 x Gold 6152 box.

Quoted 5 lines · Mike Galbraith

Quoted 4 lines · Mel Gorman

Quoted 1 line · Peter Zijlstra

Mel, what hardware are you testing this on?

Quoted 4 lines

Quoted 2 lines

The primary one was a single socket skylake machine with 8 threads (HT enabled).

I took it for a spin in a 2 x Gold 6152 box.

Can you translate that marketing speak for a simpleton like me? That's what we should've called SKL-EP, right?

The results can be less dramatic on NUMA where automatic balancing interferes with the test. It's also known that network benchmarks running on localhost also benefit quite a bit from this patch (roughly 10% on netperf RR for UDP and TCP depending on the machine). Hackbench also seens small improvements (6-11% depending on machine and thread count). The facebook schbench was also tested but in most cases showed little or no different to wakeup latencies.

Quoted 6 lines · Mel Gorman

The results can be less dramatic on NUMA where automatic balancing interferes with the test. It's also known that network benchmarks running on localhost also benefit quite a bit from this patch (roughly 10% on netperf RR for UDP and TCP depending on the machine). Hackbench also seens small improvements (6-11% depending on machine and thread count). The facebook schbench was also tested but in most cases showed little or no different to wakeup latencies.

What cpufreq setting were you using for these tests?

Quoted 6 lines · Mel Gorman

The results can be less dramatic on NUMA where automatic balancing interferes with the test. It's also known that network benchmarks running on localhost also benefit quite a bit from this patch (roughly 10% on netperf RR for UDP and TCP depending on the machine). Hackbench also seens small improvements (6-11% depending on machine and thread count). The facebook schbench was also tested but in most cases showed little or no different to wakeup latencies.

What cpufreq setting were you using for these tests?

Quoted 8 lines · Peter Zijlstra

Quoted 6 lines · Mel Gorman

The results can be less dramatic on NUMA where automatic balancing interferes with the test. It's also known that network benchmarks running on localhost also benefit quite a bit from this patch (roughly 10% on netperf RR for UDP and TCP depending on the machine). Hackbench also seens small improvements (6-11% depending on machine and thread count). The facebook schbench was also tested but in most cases showed little or no different to wakeup latencies.

Quoted 1 line

What cpufreq setting were you using for these tests?

I cannot measure any hackbench variation one way or the other with these patches using 'performance' mode. So I'll assume you've been running things with HWP or something.