Re: [PATCH v4 04/17] x86/split_lock: Align x86_capability to unsigned long to avoid split locked access

From: Dave Hansen
Date: Mon Mar 04 2019 - 14:29:44 EST


On 3/4/19 11:15 AM, Fenghua Yu wrote:
> On Mon, Mar 04, 2019 at 10:52:19AM -0800, Dave Hansen wrote:
>> On 3/1/19 6:44 PM, Fenghua Yu wrote:
>>> diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
>>> index 33051436c864..eb8ae701ef65 100644
>>> --- a/arch/x86/include/asm/processor.h
>>> +++ b/arch/x86/include/asm/processor.h
>>> @@ -93,7 +93,9 @@ struct cpuinfo_x86 {
>>> __u32 extended_cpuid_level;
>>> /* Maximum supported CPUID level, -1=no CPUID: */
>>> int cpuid_level;
>>> - __u32 x86_capability[NCAPINTS + NBUGINTS];
>>> + /* Unsigned long alignment to avoid split lock in atomic bitmap ops */
>>> + __u32 x86_capability[NCAPINTS + NBUGINTS]
>>> + __aligned(sizeof(unsigned long));
>> I think this also warrants a comment in the changelog about the
>> alignment of 'struct cpuinfo_x86'.
> How about add "Depending on the starting address where GCC generates
> for data of struct cpuinfo_x86, x86_capability[] may or may not align to
> unsigned long...."?

If that's the case, then what good is this patch? Sounds like some of
the story is missing.

You might want to dig through some of the past discussions about this.
I know this exact topic has been broached with Thomas.