Re: [PATCH v7 1/4] spinlock: A new lockref structure for locklessupdate of refcount

[Linus Torvalds]

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