On Tue, Sep 3, 2013 at 12:19 PM, Ingo Molnar <mingo@xxxxxxxxxx> wrote:
>
> * Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>
>>
>> - the lglock data structure isn't a percpu data structure, it's this
>> stupid global data structure that has a percpu pointer in it. So that
>> first "mov (%rdi),%rdx" is purely to load what is effectively a constant
>> address (per lglock).
>>
>> And that's not because it wants to be, but because we associate
>> global lockdep data with it. Ugh. If it wasn't for that, we could just
>> make them percpu.
>
> I don't think that's fundamental - the per CPU lock was percpu before:
[...]
> but AFAICS got converted to a pointer via this commit:
>
> commit eea62f831b8030b0eeea8314eed73b6132d1de26
> Author: Andi Kleen <ak@xxxxxxxxxxxxxxx>
> Date: Tue May 8 13:32:24 2012 +0930
>
> brlocks/lglocks: turn into functions
So instead of reverting that entirely, how about making "struct
lglock" always per entirely per-cpu, and replacing the percpu pointer
with the lock itself.
Then, we say "the lockdep map is always on CPU#0".
TOTALLY UNTESTED PATCH ATTACHED. It compiles in at least a couple of
configurations, and I checked that this removes _one_ of the
indirections (the other one is because we don't have a native per-cpu
spinlock helper function, so we need to do that percpu base addition),
but I haven't dared try to actually try to boot it.
Comments?
I'll try booting it and seeing if it actually works (and if it makes
any difference), but it seems to be a reasonable approach. I think it
actually cleans things up a bit, but maybe that's just because I
touched the code now.
Linus
Attachment:
patch.diff
Description: Binary data
