All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nathan Lynch <nathanl@linux.ibm.com>
To: "Michal Suchánek" <msuchanek@suse.de>
Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>,
	linux-kernel@vger.kernel.org,
	Srikar Dronamraju <srikar@linux.ibm.com>,
	npiggin@gmail.com, Laurent Dufour <ldufour@linux.ibm.com>,
	linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH] powerpc/pseries/cpuhp: respect current SMT when adding new CPU
Date: Mon, 13 Feb 2023 09:40:50 -0600	[thread overview]
Message-ID: <87fsb9a7zx.fsf@linux.ibm.com> (raw)
In-Reply-To: <20230213150429.GZ19419@kitsune.suse.cz>

Michal Suchánek <msuchanek@suse.de> writes:
> On Mon, Feb 13, 2023 at 08:46:50AM -0600, Nathan Lynch wrote:
>> Laurent Dufour <ldufour@linux.ibm.com> writes:
>> > When a new CPU is added, the kernel is activating all its threads. This
>> > leads to weird, but functional, result when adding CPU on a SMT 4 system
>> > for instance.
>> >
>> > Here the newly added CPU 1 has 8 threads while the other one has 4 threads
>> > active (system has been booted with the 'smt-enabled=4' kernel option):
>> >
>> > ltcden3-lp12:~ # ppc64_cpu --info
>> > Core   0:    0*    1*    2*    3*    4     5     6     7
>> > Core   1:    8*    9*   10*   11*   12*   13*   14*   15*
>> >
>> > There is no SMT value in the kernel. It is possible to run unbalanced LPAR
>> > with 2 threads for a CPU, 4 for another one, and 5 on the latest.
>> >
>> > To work around this possibility, and assuming that the LPAR run with the
>> > same number of threads for each CPU, which is the common case,
>> 
>> I am skeptical at best of baking that assumption into this code. Mixed
>> SMT modes within a partition doesn't strike me as an unreasonable
>> possibility for some use cases. And if that's wrong, then we should just
>> add a global smt value instead of using heuristics.
>> 
>> > the number
>> > of active threads of the CPU doing the hot-plug operation is computed. Only
>> > that number of threads will be activated for the newly added CPU.
>> >
>> > This way on a LPAR running in SMT=4, newly added CPU will be running 4
>> > threads, which is what a end user would expect.
>> 
>> I could see why most users would prefer this new behavior. But surely
>> some users have come to expect the existing behavior, which has been in
>> place for years, and developed workarounds that might be broken by this
>> change?
>> 
>> I would suggest that to handle this well, we need to give user space
>> more ability to tell the kernel what actions to take on added cores, on
>> an opt-in basis.
>> 
>> This could take the form of extending the DLPAR sysfs command set:
>> 
>> Option 1 - Add a flag that tells the kernel not to online any threads at
>> all; user space will online the desired threads later.
>> 
>> Option 2 - Add an option that tells the kernel which SMT mode to apply.
>
> powerpc-utils grew some drmgr hooks recently so maybe the policy can be
> moved to userspace?

I'm not sure whether the hook mechanism would come into play, but yes, I
am suggesting that user space be given the option of overriding the
kernel's current behavior.

WARNING: multiple messages have this Message-ID (diff)
From: Nathan Lynch <nathanl@linux.ibm.com>
To: "Michal Suchánek" <msuchanek@suse.de>
Cc: Laurent Dufour <ldufour@linux.ibm.com>,
	mpe@ellerman.id.au, npiggin@gmail.com,
	christophe.leroy@csgroup.eu,
	Srikar Dronamraju <srikar@linux.ibm.com>,
	linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
	Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Subject: Re: [PATCH] powerpc/pseries/cpuhp: respect current SMT when adding new CPU
Date: Mon, 13 Feb 2023 09:40:50 -0600	[thread overview]
Message-ID: <87fsb9a7zx.fsf@linux.ibm.com> (raw)
In-Reply-To: <20230213150429.GZ19419@kitsune.suse.cz>

Michal Suchánek <msuchanek@suse.de> writes:
> On Mon, Feb 13, 2023 at 08:46:50AM -0600, Nathan Lynch wrote:
>> Laurent Dufour <ldufour@linux.ibm.com> writes:
>> > When a new CPU is added, the kernel is activating all its threads. This
>> > leads to weird, but functional, result when adding CPU on a SMT 4 system
>> > for instance.
>> >
>> > Here the newly added CPU 1 has 8 threads while the other one has 4 threads
>> > active (system has been booted with the 'smt-enabled=4' kernel option):
>> >
>> > ltcden3-lp12:~ # ppc64_cpu --info
>> > Core   0:    0*    1*    2*    3*    4     5     6     7
>> > Core   1:    8*    9*   10*   11*   12*   13*   14*   15*
>> >
>> > There is no SMT value in the kernel. It is possible to run unbalanced LPAR
>> > with 2 threads for a CPU, 4 for another one, and 5 on the latest.
>> >
>> > To work around this possibility, and assuming that the LPAR run with the
>> > same number of threads for each CPU, which is the common case,
>> 
>> I am skeptical at best of baking that assumption into this code. Mixed
>> SMT modes within a partition doesn't strike me as an unreasonable
>> possibility for some use cases. And if that's wrong, then we should just
>> add a global smt value instead of using heuristics.
>> 
>> > the number
>> > of active threads of the CPU doing the hot-plug operation is computed. Only
>> > that number of threads will be activated for the newly added CPU.
>> >
>> > This way on a LPAR running in SMT=4, newly added CPU will be running 4
>> > threads, which is what a end user would expect.
>> 
>> I could see why most users would prefer this new behavior. But surely
>> some users have come to expect the existing behavior, which has been in
>> place for years, and developed workarounds that might be broken by this
>> change?
>> 
>> I would suggest that to handle this well, we need to give user space
>> more ability to tell the kernel what actions to take on added cores, on
>> an opt-in basis.
>> 
>> This could take the form of extending the DLPAR sysfs command set:
>> 
>> Option 1 - Add a flag that tells the kernel not to online any threads at
>> all; user space will online the desired threads later.
>> 
>> Option 2 - Add an option that tells the kernel which SMT mode to apply.
>
> powerpc-utils grew some drmgr hooks recently so maybe the policy can be
> moved to userspace?

I'm not sure whether the hook mechanism would come into play, but yes, I
am suggesting that user space be given the option of overriding the
kernel's current behavior.

  reply	other threads:[~2023-02-13 15:42 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-13 12:45 [PATCH] powerpc/pseries/cpuhp: respect current SMT when adding new CPU Laurent Dufour
2023-02-13 14:46 ` Nathan Lynch
2023-02-13 15:04   ` Michal Suchánek
2023-02-13 15:04     ` Michal Suchánek
2023-02-13 15:40     ` Nathan Lynch [this message]
2023-02-13 15:40       ` Nathan Lynch
2023-02-14 15:32       ` Laurent Dufour
2023-02-14 15:32         ` Laurent Dufour
2023-03-30 15:51       ` Laurent Dufour
2023-03-30 15:51         ` Laurent Dufour
2023-03-30 16:19         ` Michal Suchánek
2023-03-30 16:19           ` Michal Suchánek
2023-03-31 15:11           ` Laurent Dufour
2023-03-31 15:11             ` Laurent Dufour

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87fsb9a7zx.fsf@linux.ibm.com \
    --to=nathanl@linux.ibm.com \
    --cc=ldufour@linux.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=msuchanek@suse.de \
    --cc=npiggin@gmail.com \
    --cc=srikar@linux.ibm.com \
    --cc=srikar@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.