2021-10-05 07:55:32

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH RESEND 0/3] Represent cluster topology and enable load balance between clusters

On Fri, Oct 01, 2021 at 04:22:46PM -0700, Tim Chen wrote:
> On Fri, 2021-10-01 at 16:57 +0200, Peter Zijlstra wrote:

> > The one questino I have is, do we want default y?
>
> I also agree that default y is preferable.

I'll change at least the x86 one to:

default y
depends on SMP

> > The one nit I have is the Kconfig text, I'm not really sure that's
> > clarifying what a cluster is.
>
> Do you have a preference of a different name other than cluster?
> Or simply better documentation on what a cluster is for ARM64
> and x86 in Kconfig?

Yes, better wording as to what a cluster is. Currently the x86 and arm64
ones actually differ:

x86:
help
Cluster scheduler support improves the CPU scheduler's decision
making when dealing with machines that have clusters of CPUs
sharing L2 cache. If unsure say N here.

arm64:
help
Cluster scheduler support improves the CPU scheduler's decision
making when dealing with machines that have clusters(sharing internal
bus or sharing LLC cache tag). If unsure say N here.


(also, all this stuff being replicated across arch/*/Kconfig seems
unfortunate)


2021-10-05 13:44:28

by Valentin Schneider

[permalink] [raw]
Subject: Re: [PATCH RESEND 0/3] Represent cluster topology and enable load balance between clusters

On 05/10/21 09:50, Peter Zijlstra wrote:
> On Fri, Oct 01, 2021 at 04:22:46PM -0700, Tim Chen wrote:
>> On Fri, 2021-10-01 at 16:57 +0200, Peter Zijlstra wrote:
>
>> > The one questino I have is, do we want default y?
>>
>> I also agree that default y is preferable.
>
> I'll change at least the x86 one to:
>
> default y
> depends on SMP
>

Huh, so the arm64 SCHED_{SMT,MC} configs are defaultless (I added SCHED_SMT
to arm64's defconfig not so long ago), but x86 has them default y, which
I'm thinking is a tad better, and would be nice to harmonize. Unfortunately
different architectures have their own dependency requirements - arm has
ARM_CPU_TOPOLOGY, parisc has PARISC_CPU_TOPOLOGY...

Would you hate making SCHED_* a "generic" config, with a common default and
help text, and punt the arch specific stuff to an ARCH_SUPPORTS_* knob?

Something like:

arch/arm/Kconfig:
select ARCH_SUPPORTS_SCHED_MC if ARM_CPU_TOPOLOGY

init/Kconfig:
config SCHED_MC
def_bool y
depends on ARCH_SUPPORTS_SCHED_MC && SMP

2021-10-05 13:52:52

by Valentin Schneider

[permalink] [raw]
Subject: Re: [PATCH RESEND 0/3] Represent cluster topology and enable load balance between clusters

On 05/10/21 14:42, Valentin Schneider wrote:
> On 05/10/21 09:50, Peter Zijlstra wrote:
>> On Fri, Oct 01, 2021 at 04:22:46PM -0700, Tim Chen wrote:
>>> On Fri, 2021-10-01 at 16:57 +0200, Peter Zijlstra wrote:
>>
>>> > The one questino I have is, do we want default y?
>>>
>>> I also agree that default y is preferable.
>>
>> I'll change at least the x86 one to:
>>
>> default y
>> depends on SMP
>>
>
> Huh, so the arm64 SCHED_{SMT,MC} configs are defaultless (I added SCHED_SMT
> to arm64's defconfig not so long ago), but x86 has them default y, which
> I'm thinking is a tad better, and would be nice to harmonize. Unfortunately
> different architectures have their own dependency requirements - arm has
> ARM_CPU_TOPOLOGY, parisc has PARISC_CPU_TOPOLOGY...
>
> Would you hate making SCHED_* a "generic" config, with a common default and
> help text, and punt the arch specific stuff to an ARCH_SUPPORTS_* knob?
>
> Something like:
>
> arch/arm/Kconfig:
> select ARCH_SUPPORTS_SCHED_MC if ARM_CPU_TOPOLOGY
>
> init/Kconfig:
> config SCHED_MC
> def_bool y
> depends on ARCH_SUPPORTS_SCHED_MC && SMP

... Which is essentially what Barry suggested somewhere else in the thread
(I TL; DR'd).