Re: [PATCH 1/2] powerpc/pseries: remove cede offline state for CPUs

Hi Gautham,

Gautham R Shenoy [off-list ref] writes:

On Mon, Jun 01, 2020 at 11:31:39PM -0500, Nathan Lynch wrote:

quoted

This effectively reverts commit 3aa565f53c39 ("powerpc/pseries: Add
hooks to put the CPU into an appropriate offline state"), which added
an offline mode for CPUs which uses the H_CEDE hcall instead of the
architected stop-self RTAS function in order to facilitate "folding"
of dedicated mode processors on PowerVM platforms to achieve energy
savings. This has been the default offline mode since its
introduction.

There's nothing about stop-self that would prevent the hypervisor from
achieving the energy savings available via H_CEDE, so the original
premise of this change appears to be flawed.

IIRC, back in 2009, when the Extended-CEDE was introduced, it couldn't
be exposed via the cpuidle subsystem since this state needs an
explicit H_PROD as opposed to the H_IPI which wakes up the regular
CEDE call. So, the alterative was to use the CPU-Hotplug way by having
a userspace daemon fold the cores which weren't needed currently and
bring them back online when they were needed. Back then, Long Term
CEDE was definitely faster compared to stop-self call (It is a pity
that I didn't post the numbers when I wrote the patch) and the
time-taken to unfold a core was definitely one of the concerns.
(https://lkml.org/lkml/2009/9/23/522).

Thanks for the background. Ideally, we would understand what has changed
since then... certainly the offline path would have been slower when
pseries_cpu_die() slept for 200ms between issuing
query-cpu-stopped-state calls, but that was changed in 2008 in
b906cfa397fd ("powerpc/pseries: Fix cpu hotplug") so perhaps it wasn't a
factor in your measurements. Even less sure about what could have
made the online path more expensive.

(For the benefit of anybody else referring to the 2009 discussion that
you linked: that exchange is a bit strange to me because there seems to
be a conception that stop-self releases the processor to the platform in
a way that could prevent the OS from restarting it on demand. I don't
believe this has ever been the case. Processor deallocation is a related
but separate workflow involving different RTAS functions.)

The patch looks good to me.
Reviewed-by: Gautham R. Shenoy <redacted>

I appreciate the review, thanks!