public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/2] Move x86_cpu_to_apicid to the __read_mostly section
@ 2012-05-21 15:02 Vlad Zolotarov
  2012-05-21 15:23 ` Ingo Molnar
  0 siblings, 1 reply; 10+ messages in thread
From: Vlad Zolotarov @ 2012-05-21 15:02 UTC (permalink / raw)
  To: Ingo Molnar, H. Peter Anvin, Thomas Gleixner, linux-kernel, Shai,
	ido

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.

v4 changes:
- Fixed the authors signatures in the patches.

v3 changes:
- Added the missing definitions of DEFINE_EARLY_PER_CPU_READ_MOSTLY()
  and DECLARE_EARLY_PER_CPU_READ_MOSTLY() macros in the !CONFIG_SMP code
  path in arch/x86/include/asm/percpu.h.

thanks,
vlad

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 0/2] Move x86_cpu_to_apicid to the __read_mostly section
  2012-05-21 15:02 [PATCH v4 0/2] Move x86_cpu_to_apicid to the __read_mostly section Vlad Zolotarov
@ 2012-05-21 15:23 ` Ingo Molnar
  2012-05-21 15:42   ` Vlad Zolotarov
  0 siblings, 1 reply; 10+ messages in thread
From: Ingo Molnar @ 2012-05-21 15:23 UTC (permalink / raw)
  To: Vlad Zolotarov; +Cc: H. Peter Anvin, Thomas Gleixner, linux-kernel, Shai, ido


* Vlad Zolotarov <vlad@scalemp.com> wrote:

> 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.

Why have you resent this? The feedback I gave has not been 
addressed:

> 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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 0/2] Move x86_cpu_to_apicid to the __read_mostly section
  2012-05-21 15:23 ` Ingo Molnar
@ 2012-05-21 15:42   ` Vlad Zolotarov
  2012-05-21 20:19     ` Ingo Molnar
  0 siblings, 1 reply; 10+ messages in thread
From: Vlad Zolotarov @ 2012-05-21 15:42 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: H. Peter Anvin, Thomas Gleixner, linux-kernel, Shai, ido

On Monday, May 21, 2012 17:23:48 Ingo Molnar wrote:
> * Vlad Zolotarov <vlad@scalemp.com> wrote:
> > 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.
> 
> Why have you resent this? The feedback I gave has not been
> 
> addressed:

Hmmm... I'm a bit confused. There were two feedbacks/threads: one on "Signed-
off-by" format and the other where u asked for a justification on a vSMP side. 

The signed-off format sounded to me as a clear blocker for a series so I fixed 
it and respined. I also mentioned it in patch0.

The second thread seams like getting to submitting a separate patch with a doc 
under Documents and vSMP testing results explaining and justifying when and 
were per-CPU and/or __read_mostly variables should be used. 

Pls., correct me if I misunderstood u and let me know what should I do next in 
order to make this patch series accepted.

thanks,
vlad


> > 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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 0/2] Move x86_cpu_to_apicid to the __read_mostly section
  2012-05-21 15:42   ` Vlad Zolotarov
@ 2012-05-21 20:19     ` Ingo Molnar
  2012-05-22 15:55       ` Vlad Zolotarov
  0 siblings, 1 reply; 10+ messages in thread
From: Ingo Molnar @ 2012-05-21 20:19 UTC (permalink / raw)
  To: Vlad Zolotarov; +Cc: H. Peter Anvin, Thomas Gleixner, linux-kernel, Shai, ido


* Vlad Zolotarov <vlad@scalemp.com> wrote:

> On Monday, May 21, 2012 17:23:48 Ingo Molnar wrote:
> > * Vlad Zolotarov <vlad@scalemp.com> wrote:
> > > 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.
> > 
> > Why have you resent this? The feedback I gave has not been
> > 
> > addressed:
> 
> Hmmm... I'm a bit confused. There were two feedbacks/threads: 
> one on "Signed- off-by" format and the other where u asked for 
> a justification on a vSMP side.
> 
> The signed-off format sounded to me as a clear blocker for a 
> series so I fixed it and respined. I also mentioned it in 
> patch0.

Well, you need to address all blockers before we can proceed.

> The second thread seams like getting to submitting a separate 
> patch with a doc under Documents and vSMP testing results 
> explaining and justifying when and were per-CPU and/or 
> __read_mostly variables should be used.

No. As I said I'm not convinced that there are fewer read-mostly 
than read-write percpu variables. Please:

> 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.

I.e. *numbers* are needed: roughly how many percpu variables in 
a defconfig of one type versus the other type. This settles the 
question whether we want to identify read-mostly or 
write-frequently variables, to address this particular problem 
...

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 0/2] Move x86_cpu_to_apicid to the __read_mostly section
  2012-05-21 20:19     ` Ingo Molnar
@ 2012-05-22 15:55       ` Vlad Zolotarov
  2012-05-22 15:59         ` Vlad Zolotarov
  2012-05-23  9:16         ` Vlad Zolotarov
  0 siblings, 2 replies; 10+ messages in thread
From: Vlad Zolotarov @ 2012-05-22 15:55 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: H. Peter Anvin, Thomas Gleixner, linux-kernel, Shai, ido

> > 
> > 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.
> 
> I.e. *numbers* are needed: roughly how many percpu variables in
> a defconfig of one type versus the other type. This settles the
> question whether we want to identify read-mostly or
> write-frequently variables, to address this particular problem
> ...

Ok, let's start with *numbers*:
- In the defconfig compilation i've got  219 per_cpu variables: 218 declared 
as NOT read_mostly and only one (tlb_vector_offset) - as read_mostly.
- From those that are not declared as read_mostly the grep on
"time|lock|state|stats|last|left|work|owned|reason|list|idle|count|warn|head|
rcu|nr_|pvecs|irq", which are a likely candidates to be NOT read_mostly (I 
verified a few from each pattern) returned 88. I created the above patterns 
list after walking through about the 1/3 of the per_cpu variables list.

Unfortunately I have no time to analyze all 219 variables in depth - I leave 
it to the maintainers of the code containing them but it's obvious that there 
are quite a lot read-write per_cpu variables in the kernel code and u know, 
I'm not surprised - if u consider the reasoning to declare a per-cpu variable 
u might notice that in most cases u need it when u actually  mean to actively 
write to it. This is because a main motivation for using per_cpu variables is 
to hide/prevent from other CPUs seeing the *changes* of the local per_cpu 
variable: e.g. lists used for lockless stuff, local (per-CPU) locks, counters, 
etc.

On the other hand declaration of a read_mostly per-cpu variable performance-
wise is similar to using a regular read_mostly (*not* per_cpu) array and the 
main difference is a semantics which I feel like a weaker motivation. 

> 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 guess the numbers above tell us the opposite. So, I think we'd better stick 
with the read_mostly semantics.  ;)

I also would like to draw your attention to the fact that this patch series 
doesn't introduce the read_mostly semantics either in a per-cpu context or in 
a non-per-cpu context: 
Originally we have discovered that x86_cpu_to_apicid variable has a 
read_mostly nature and is quite contented and wanted to define it as 
__read_mostly as it should be in order to prevent false sharing. 

More specifically, lapic_events and x86_cpu_to_apicid shared the same cache 
line and the first one was frequently written.

Since it's defined as an EARLY_PER_CPU variable we had to define the missing 
XXX_EARLY_PER_CPU_READ_MOSTLY() macros. 

Then u asked me to see if other variables from smp.h are also read_mostly, 
which I did however it wasn't our intention to change any infrastructure, on 
the contrary we used the existing one. I have a feeling that thought the 
opposite... ;)

So, pls., tell me what's next? Frankly, I don't think the *numbers* part above 
is of any interest to the wide public except for u and me... ;) 
However the "lapic_event" fact above might clarify the motivation a bit more.

Pls., comment.

thanks,
vlad

> 
> Thanks,
> 
> 	Ingo

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 0/2] Move x86_cpu_to_apicid to the __read_mostly section
  2012-05-22 15:55       ` Vlad Zolotarov
@ 2012-05-22 15:59         ` Vlad Zolotarov
  2012-05-23  9:16         ` Vlad Zolotarov
  1 sibling, 0 replies; 10+ messages in thread
From: Vlad Zolotarov @ 2012-05-22 15:59 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: H. Peter Anvin, Thomas Gleixner, linux-kernel, Shai, ido

On Tuesday, May 22, 2012 18:55:41 Vlad Zolotarov wrote:
> > > 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.
> > 
> > I.e. *numbers* are needed: roughly how many percpu variables in
> > a defconfig of one type versus the other type. This settles the
> > question whether we want to identify read-mostly or
> > write-frequently variables, to address this particular problem
> > ...
> 
> Ok, let's start with *numbers*:
> - In the defconfig compilation i've got  219 per_cpu variables: 218 declared
> as NOT read_mostly and only one (tlb_vector_offset) - as read_mostly. -
> From those that are not declared as read_mostly the grep on
> "time|lock|state|stats|last|left|work|owned|reason|list|idle|count|warn|head
> | rcu|nr_|pvecs|irq", which are a likely candidates to be NOT read_mostly (I
> verified a few from each pattern) returned 88. I created the above patterns
> list after walking through about the 1/3 of the per_cpu variables list.
> 
> Unfortunately I have no time to analyze all 219 variables in depth - I leave
> it to the maintainers of the code containing them but it's obvious that
> there are quite a lot read-write per_cpu variables in the kernel code and u
> know, I'm not surprised - if u consider the reasoning to declare a per-cpu
> variable u might notice that in most cases u need it when u actually  mean
> to actively write to it. This is because a main motivation for using
> per_cpu variables is to hide/prevent from other CPUs seeing the *changes*
> of the local per_cpu variable: e.g. lists used for lockless stuff, local
> (per-CPU) locks, counters, etc.
> 
> On the other hand declaration of a read_mostly per-cpu variable performance-
> wise is similar to using a regular read_mostly (*not* per_cpu) array and
> the main difference is a semantics which I feel like a weaker motivation.
> > 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 guess the numbers above tell us the opposite. So, I think we'd better
> stick with the read_mostly semantics.  ;)
> 
> I also would like to draw your attention to the fact that this patch series
> doesn't introduce the read_mostly semantics either in a per-cpu context or
> in a non-per-cpu context:
> Originally we have discovered that x86_cpu_to_apicid variable has a
> read_mostly nature and is quite contented and wanted to define it as
> __read_mostly as it should be in order to prevent false sharing.
> 
> More specifically, lapic_events and x86_cpu_to_apicid shared the same cache
> line and the first one was frequently written.
> 
> Since it's defined as an EARLY_PER_CPU variable we had to define the missing
> XXX_EARLY_PER_CPU_READ_MOSTLY() macros.
> 
> Then u asked me to see if other variables from smp.h are also read_mostly,
> which I did however it wasn't our intention to change any infrastructure, on
> the contrary we used the existing one. I have a feeling that thought the
> opposite... ;)

I have a feeling that U thought the opposite... ;)

> 
> So, pls., tell me what's next? Frankly, I don't think the *numbers* part
> above is of any interest to the wide public except for u and me... ;)
> However the "lapic_event" fact above might clarify the motivation a bit
> more.
> 
> Pls., comment.
> 
> thanks,
> vlad
> 
> > Thanks,
> > 
> > 	Ingo
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 0/2] Move x86_cpu_to_apicid to the __read_mostly section
  2012-05-22 15:55       ` Vlad Zolotarov
  2012-05-22 15:59         ` Vlad Zolotarov
@ 2012-05-23  9:16         ` Vlad Zolotarov
  2012-06-07  8:18           ` Vlad Zolotarov
  1 sibling, 1 reply; 10+ messages in thread
From: Vlad Zolotarov @ 2012-05-23  9:16 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: H. Peter Anvin, Thomas Gleixner, linux-kernel, Shai, ido

On Tuesday, May 22, 2012 18:55:41 Vlad Zolotarov wrote:
> > > 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.
> > 
> > I.e. *numbers* are needed: roughly how many percpu variables in
> > a defconfig of one type versus the other type. This settles the
> > question whether we want to identify read-mostly or
> > write-frequently variables, to address this particular problem
> > ...
> 

Ingo, here is the proposal to the patch (series) description:

------------------------------------------------------------------------------------------------
Added "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. More specifically, on vSMP 
Foundation platform x86_cpu_to_apicid shared the same internode_cache_line 
with frequently written lapic_events.

>From the analysis of the first 33 per_cpu variables  out of 219 (memories they 
describe, to be more specific) the 8 have read_mostly nature 
(tlb_vector_offset, cpu_loops_per_jiffy, xen_debug_irq, etc.) and 25 are 
frequently written (irq_stack_union, gdt_page, exception_stacks, idt_desc, 
etc.). Assuming that the spread of the rest of the per_cpu variables is 
similar, identifying the read mostly memories will make more sense in terms of 
long-term code maintenance comparing to identifying frequently written 
memories.
---------------------------------------------------------------------------------------------------

Pls., tell me if the above looks satisfactory to u in light of all your 
previous remarks.

If yes - I'll respin the series with the description above.

thanks,
vlad

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 0/2] Move x86_cpu_to_apicid to the __read_mostly section
  2012-05-23  9:16         ` Vlad Zolotarov
@ 2012-06-07  8:18           ` Vlad Zolotarov
  2012-06-11  9:00             ` Ingo Molnar
  0 siblings, 1 reply; 10+ messages in thread
From: Vlad Zolotarov @ 2012-06-07  8:18 UTC (permalink / raw)
  To: Ingo Molnar, linux-kernel; +Cc: Shai

On Wednesday 23 May 2012 12:16:29 Vlad Zolotarov wrote:
> On Tuesday, May 22, 2012 18:55:41 Vlad Zolotarov wrote:
> > > > 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.
> > > 
> > > I.e. *numbers* are needed: roughly how many percpu variables in
> > > a defconfig of one type versus the other type. This settles the
> > > question whether we want to identify read-mostly or
> > > write-frequently variables, to address this particular problem
> > > ...
> 
> Ingo, here is the proposal to the patch (series) description:
> 
> ----------------------------------------------------------------------------
> -------------------- Added "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. More
> specifically, on vSMP Foundation platform x86_cpu_to_apicid shared the same
> internode_cache_line with frequently written lapic_events.
> 
> From the analysis of the first 33 per_cpu variables  out of 219 (memories
> they describe, to be more specific) the 8 have read_mostly nature
> (tlb_vector_offset, cpu_loops_per_jiffy, xen_debug_irq, etc.) and 25 are
> frequently written (irq_stack_union, gdt_page, exception_stacks, idt_desc,
> etc.). Assuming that the spread of the rest of the per_cpu variables is
> similar, identifying the read mostly memories will make more sense in terms
> of long-term code maintenance comparing to identifying frequently written
> memories.
> ----------------------------------------------------------------------------
> -----------------------
> 
> Pls., tell me if the above looks satisfactory to u in light of all your
> previous remarks.
> 
> If yes - I'll respin the series with the description above.

Ingo, sorry for bothering. Could u, pls., tell if the above description is ok? We'd
like to move on with this patch series.

thanks in advance,
vlad

> 
> thanks,
> vlad
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 0/2] Move x86_cpu_to_apicid to the __read_mostly section
  2012-06-07  8:18           ` Vlad Zolotarov
@ 2012-06-11  9:00             ` Ingo Molnar
  2012-06-11  9:08               ` Vlad Zolotarov
  0 siblings, 1 reply; 10+ messages in thread
From: Ingo Molnar @ 2012-06-11  9:00 UTC (permalink / raw)
  To: Vlad Zolotarov; +Cc: linux-kernel, Shai, Thomas Gleixner, H. Peter Anvin


* Vlad Zolotarov <vlad@scalemp.com> wrote:

> On Wednesday 23 May 2012 12:16:29 Vlad Zolotarov wrote:
> > On Tuesday, May 22, 2012 18:55:41 Vlad Zolotarov wrote:
> > > > > 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.
> > > > 
> > > > I.e. *numbers* are needed: roughly how many percpu variables in
> > > > a defconfig of one type versus the other type. This settles the
> > > > question whether we want to identify read-mostly or
> > > > write-frequently variables, to address this particular problem
> > > > ...
> > 
> > Ingo, here is the proposal to the patch (series) description:
> > 
> > ----------------------------------------------------------------------------
> > -------------------- Added "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. More
> > specifically, on vSMP Foundation platform x86_cpu_to_apicid shared the same
> > internode_cache_line with frequently written lapic_events.
> > 
> > From the analysis of the first 33 per_cpu variables  out of 219 (memories
> > they describe, to be more specific) the 8 have read_mostly nature
> > (tlb_vector_offset, cpu_loops_per_jiffy, xen_debug_irq, etc.) and 25 are
> > frequently written (irq_stack_union, gdt_page, exception_stacks, idt_desc,
> > etc.). Assuming that the spread of the rest of the per_cpu variables is
> > similar, identifying the read mostly memories will make more sense in terms
> > of long-term code maintenance comparing to identifying frequently written
> > memories.
> > ----------------------------------------------------------------------------
> > -----------------------
> > 
> > Pls., tell me if the above looks satisfactory to u in light of all your
> > previous remarks.
> > 
> > If yes - I'll respin the series with the description above.
> 
> Ingo, sorry for bothering. Could u, pls., tell if the above 
> description is ok? We'd like to move on with this patch 
> series.

Yeah, that description and analysis looks good and sensible.

Mind resending the updated patches in a new thread?

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v4 0/2] Move x86_cpu_to_apicid to the __read_mostly section
  2012-06-11  9:00             ` Ingo Molnar
@ 2012-06-11  9:08               ` Vlad Zolotarov
  0 siblings, 0 replies; 10+ messages in thread
From: Vlad Zolotarov @ 2012-06-11  9:08 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: linux-kernel, Shai, Thomas Gleixner, H. Peter Anvin

On Monday 11 June 2012 11:00:31 Ingo Molnar wrote:
> * Vlad Zolotarov <vlad@scalemp.com> wrote:
> > On Wednesday 23 May 2012 12:16:29 Vlad Zolotarov wrote:
> > > On Tuesday, May 22, 2012 18:55:41 Vlad Zolotarov wrote:
> > > > > > 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.
> > > > > 
> > > > > I.e. *numbers* are needed: roughly how many percpu variables in
> > > > > a defconfig of one type versus the other type. This settles the
> > > > > question whether we want to identify read-mostly or
> > > > > write-frequently variables, to address this particular problem
> > > > > ...
> > > 
> > > Ingo, here is the proposal to the patch (series) description:
> > > 
> > > ------------------------------------------------------------------------
> > > ---- -------------------- Added "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. More
> > > specifically, on vSMP Foundation platform x86_cpu_to_apicid shared the
> > > same
> > > internode_cache_line with frequently written lapic_events.
> > > 
> > > From the analysis of the first 33 per_cpu variables  out of 219
> > > (memories
> > > they describe, to be more specific) the 8 have read_mostly nature
> > > (tlb_vector_offset, cpu_loops_per_jiffy, xen_debug_irq, etc.) and 25 are
> > > frequently written (irq_stack_union, gdt_page, exception_stacks,
> > > idt_desc,
> > > etc.). Assuming that the spread of the rest of the per_cpu variables is
> > > similar, identifying the read mostly memories will make more sense in
> > > terms
> > > of long-term code maintenance comparing to identifying frequently
> > > written
> > > memories.
> > > ------------------------------------------------------------------------
> > > ---- -----------------------
> > > 
> > > Pls., tell me if the above looks satisfactory to u in light of all your
> > > previous remarks.
> > > 
> > > If yes - I'll respin the series with the description above.
> > 
> > Ingo, sorry for bothering. Could u, pls., tell if the above
> > description is ok? We'd like to move on with this patch
> > series.
> 
> Yeah, that description and analysis looks good and sensible.
> 
> Mind resending the updated patches in a new thread?

Sure. I'm on it right now, sir. :)

thanks,
vlad

> 
> Thanks,
> 
> 	Ingo

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2012-06-11  9:08 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-05-21 15:02 [PATCH v4 0/2] Move x86_cpu_to_apicid to the __read_mostly section Vlad Zolotarov
2012-05-21 15:23 ` Ingo Molnar
2012-05-21 15:42   ` Vlad Zolotarov
2012-05-21 20:19     ` Ingo Molnar
2012-05-22 15:55       ` Vlad Zolotarov
2012-05-22 15:59         ` Vlad Zolotarov
2012-05-23  9:16         ` Vlad Zolotarov
2012-06-07  8:18           ` Vlad Zolotarov
2012-06-11  9:00             ` Ingo Molnar
2012-06-11  9:08               ` Vlad Zolotarov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox