Re: [Xen-devel] [PATCH v2 1/2] xen/link: Introduce .bss.percpu.page_aligned

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 29/07/2019 09:52, Roger Pau Monné wrote:
> On Fri, Jul 26, 2019 at 09:32:21PM +0100, Andrew Cooper wrote:
>> Future changes are going to need to page align some percpu data.
>>
>> This means that the percpu area needs suitably aligning in the BSS so CPU0 has
>> correctly aligned data.  Shuffle the exact link order of items within the BSS
>> to give .bss.percpu.page_aligned appropriate alignment.
>>
>> In addition, we need to be able to specify an alignment attribute to
>> __DEFINE_PER_CPU().  Rework it so the caller passes in all attributes, and
>> adjust DEFINE_PER_CPU{,_READ_MOSTLY}() to match.  This has the added bonus
>> that it is now possible to grep for .bss.percpu and find all the users.
>>
>> Finally, introduce DEFINE_PER_CPU_PAGE_ALIGNED() which uses both section and
>> alignment attributes.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
>
>> diff --git a/xen/include/asm-x86/percpu.h b/xen/include/asm-x86/percpu.h
>> index ff34dc7897..5b6cef04c4 100644
>> --- a/xen/include/asm-x86/percpu.h
>> +++ b/xen/include/asm-x86/percpu.h
>> @@ -7,10 +7,8 @@ extern unsigned long __per_cpu_offset[NR_CPUS];
>>  void percpu_init_areas(void);
>>  #endif
>>  
>> -/* Separate out the type, so (int[3], foo) works. */
>> -#define __DEFINE_PER_CPU(type, name, suffix)                    \
>> -    __section(".bss.percpu" #suffix)                            \
>> -    __typeof__(type) per_cpu_##name
>> +#define __DEFINE_PER_CPU(attr, type, name) \
>> +    attr __typeof__(type) per_cpu_ ## name
>>  
>>  /* var is in discarded region: offset to particular copy we want */
>>  #define per_cpu(var, cpu)  \
>> diff --git a/xen/include/xen/percpu.h b/xen/include/xen/percpu.h
>> index aeec5c19d6..71a31cc361 100644
>> --- a/xen/include/xen/percpu.h
>> +++ b/xen/include/xen/percpu.h
>> @@ -9,9 +9,15 @@
>>   * The _##name concatenation is being used here to prevent 'name' from getting
>>   * macro expanded, while still allowing a per-architecture symbol name prefix.
>>   */
>> -#define DEFINE_PER_CPU(type, name) __DEFINE_PER_CPU(type, _##name, )
>> +#define DEFINE_PER_CPU(type, name) \
>> +    __DEFINE_PER_CPU(__section(".bss.percpu"), type, _ ## name)
>> +
>> +#define DEFINE_PER_CPU_PAGE_ALIGNED(type, name) \
>> +    __DEFINE_PER_CPU(__section(".bss.percpu.page_aligned") \
>> +                     __aligned(PAGE_SIZE), type, _ ## name)
>> +
>>  #define DEFINE_PER_CPU_READ_MOSTLY(type, name) \
>> -	__DEFINE_PER_CPU(type, _##name, .read_mostly)
>> +    __DEFINE_PER_CPU(__section(".bss.percpu.read_mostly"), type, _ ## name)
> AFAICT also adding a '_' here will result in variable names with
> per_cpu__foo, which is inline with the previous behaviour, but I'm not
> sure of the point of the double underscore.

Its double, to avoid token expansion.  See the comment in context at the
top of this hunk.

Without it, the schedulers don't compile because cpumask_scratch is both
the name of a percpu variable, and an alias to
&this_cpu(cpumask_scratch).  Omitting the token concatenation here
causes &this_cpu() to be expanded into __DEFINE_PER_CPU().

~Andrew

P.S. Guess who tried to remove this piece of "fun" to begin with...

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel