Move x86_cpu_to_apicid to the __read_mostly section

Pls., consider applying this patch series.
It contains the following changes:
- Adds two new macros DEFINE_EARLY_PER_CPU_READ_MOSTLY() and DECLARE_EARLY_PER_CPU_READ_MOSTLY().
- Adds "read-mostly" qualifier to the following variables in smp.h:
- cpu_sibling_map
- cpu_core_map
- cpu_llc_shared_map
- cpu_llc_id
- cpu_number
- x86_cpu_to_apicid
- x86_bios_cpu_apicid
- x86_cpu_to_logical_apicid

As long as all the variables above are only written during the initialization, this change is meant to prevent the false sharing and improve the performance on large multiprocessor systems.

-----Original Message-----
From: Ingo Molnar [mailto:[email protected]] On Behalf Of Ingo Molnar
Sent: Monday, May 21, 2012 12:07
To: Vlad Zolotarov
Cc: Thomas Gleixner; linux-kernel; Ingo Molnar; H. Peter Anvin; Shai Fultheim ([email protected]); Ido Yariv
Subject: Re: [PATCH v3 0/2] Move x86_cpu_to_apicid to the __read_mostly section

Quoted 14 lines · Vlad Zolotarov

Pls., consider applying this patch series.
It contains the following changes:
- Adds two new macros DEFINE_EARLY_PER_CPU_READ_MOSTLY() and DECLARE_EARLY_PER_CPU_READ_MOSTLY().
- Adds "read-mostly" qualifier to the following variables in smp.h:
- cpu_sibling_map
- cpu_core_map
- cpu_llc_shared_map
- cpu_llc_id
- cpu_number
- x86_cpu_to_apicid
- x86_bios_cpu_apicid
- x86_cpu_to_logical_apicid

Quoted 4 lines

As long as all the variables above are only written during the initialization, this change is meant to prevent the false sharing and improve the performance on large multiprocessor systems.

Mind explaining why it helps performance? Are these percpu variables also accessed from other CPUs, increasing the cost of false sharing of cache-lines?

Or is the problem that the 'cache line' size is 4096 bytes on vSMP, making even singular cache misses expensive and you'd like to move these next to each other?

So it would be nice to see some before/after PMU stats demonstrating the improvement. (perf works on vSMP, right?)

Thanks,

Ingo

Ingo,

The reason for this, as you pointed out, is the 'cache line' size (4096 bytes). We see significant false sharing is we do not move this next to each other.

Quoted 5 lines · * Shai Fultheim ([email protected]) <[email protected]> wrote:

Ingo,

The reason for this, as you pointed out, is the 'cache line' size (4096 bytes). We see significant false sharing is we do not move this next to each other.

Which write-often variable caused the many cache flushes/fills? cpu_to_apicid is read mostly.

I.e. it might make more sense to identify the frequenty *modified* percpu variables, and move them to a separate section. I *think* most percpu variables are read mostly, so it would be more maintainable in the long run to figure out the frequently modified ones, not the frequently not modified ones.

Thanks,

Ingo

Quoted 10 lines · Ingo Molnar

Quoted 5 lines · * Shai Fultheim ([email protected]) <[email protected]> wrote:

Ingo,

The reason for this, as you pointed out, is the 'cache line' size (4096 bytes). We see significant false sharing is we do not move this next to each other.

Which write-often variable caused the many cache flushes/fills? cpu_to_apicid is read mostly.

Quoted 6 lines

I.e. it might make more sense to identify the frequenty *modified* percpu variables, and move them to a separate section. I *think* most percpu variables are read mostly, so it would be more maintainable in the long run to figure out the frequently modified ones, not the frequently not modified ones.

I tend to disagree about the general claim that most per-CPU variables are read-mostly: consider the per-CPU data structures used in lock-less algorithms like softnet_data used in a NAPI. I'm not sure what is a more common - read- only or not-read-only per-cpu data, but surely there are both...

Quoted 18 lines · Vlad Zolotarov

Quoted 10 lines · Ingo Molnar

Quoted 5 lines · * Shai Fultheim ([email protected]) <[email protected]> wrote:

Ingo,

The reason for this, as you pointed out, is the 'cache line' size (4096 bytes). We see significant false sharing is we do not move this next to each other.

Which write-often variable caused the many cache flushes/fills? cpu_to_apicid is read mostly.

Quoted 6 lines

I.e. it might make more sense to identify the frequenty *modified* percpu variables, and move them to a separate section. I *think* most percpu variables are read mostly, so it would be more maintainable in the long run to figure out the frequently modified ones, not the frequently not modified ones.

Quoted 5 lines

I tend to disagree about the general claim that most per-CPU variables are read-mostly: consider the per-CPU data structures used in lock-less algorithms like softnet_data used in a NAPI. I'm not sure what is a more common - read- only or not-read-only per-cpu data, but surely there are both...

Well, a quick tally of percpu variables on a 'make defconfig' kernel would tell us one way or another?

Here there's almost 200 percpu variables active in the 64-bit x86 defconfig, and a quick random sample suggests that most are read-mostly.

I have no fundamental prefer to either approach, but the direction taken should be justified explicitly, with numbers, arguments, etc. - also a short blurb somewhere in the headers that explains when they should be used, so that others can be aware of vSMP's special needs here.

Thanks,

Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Quoted 32 lines · On Monday, May 21, 2012 16:08:22 Ingo Molnar wrote:

Quoted 18 lines · Vlad Zolotarov

Quoted 10 lines · Ingo Molnar

Quoted 5 lines · * Shai Fultheim ([email protected]) <[email protected]> wrote:

Ingo,

The reason for this, as you pointed out, is the 'cache line' size (4096 bytes). We see significant false sharing is we do not move this next to each other.

Which write-often variable caused the many cache flushes/fills? cpu_to_apicid is read mostly.

Quoted 6 lines

I.e. it might make more sense to identify the frequenty *modified* percpu variables, and move them to a separate section. I *think* most percpu variables are read mostly, so it would be more maintainable in the long run to figure out the frequently modified ones, not the frequently not modified ones.

Quoted 5 lines

I tend to disagree about the general claim that most per-CPU variables are read-mostly: consider the per-CPU data structures used in lock-less algorithms like softnet_data used in a NAPI. I'm not sure what is a more common - read- only or not-read-only per-cpu data, but surely there are both...

Well, a quick tally of percpu variables on a 'make defconfig' kernel would tell us one way or another?

Here there's almost 200 percpu variables active in the 64-bit x86 defconfig, and a quick random sample suggests that most are read-mostly.

Quoted 5 lines

I have no fundamental prefer to either approach, but the direction taken should be justified explicitly, with numbers, arguments, etc. - also a short blurb somewhere in the headers that explains when they should be used, so that others can be aware of vSMP's special needs here.

There must be some misunderstanding - this patch is not a vSMP Foundation specific as it defines read-mostly variables as __read_mostly. The motivation for it is just the same as in a non-vSMP Foundation case. It's true that the performance gain this patch introduces in the vSMP Foundation is likely to be more significant than in a native Linux, however even for a native Linux it would still be a better code as __read_mostly is not a vSMP Foundation specific paradigm and, again, the variables modified are a clear read-mostly case.

So, the explanation u request above would be just the same as if I would explain when in general __read_mostly should be used.

I grep'ed the Documentation and haven't found any readme file with the explicit instructions when __read_mostly qualifier should be used and u r right we'd better write one.

I can create an initial version of such a doc but I think it would better come as a separate patch.