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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Sep 3, 2013 at 8:41 AM, Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> I've done that, and it matches the PEBS runs, except obviously with
> the instruction skew (so then depending on run it's 95% the
> instruction after the xadd). So the PEBS profiles are entirely
> consistent with other data.

So one thing that strikes me about our lg-locks is that they are
designed to be cheap, but they force this insane 3-deep memory access
chain to lock them.

That may be a large part of why lg_local_lock shows up so clearly on
my profiles: the single "lock xadd" instruction ends up not just being
serializing, but it is what actually consumes the previous memory
reads.

The core of the lg_local_lock sequence ends up being this
four-instruction sequence:

    mov    (%rdi),%rdx
    add    %gs:0xcd48,%rdx
    mov    $0x100,%eax
    lock   xadd   %ax,(%rdx)

and that's a nasty chain of dependent memory loads. First we load the
percpu address, then we add the percpu offset to that, and then we do
the xadd on the result.

It's kind of sad, because in *theory* we could get rid of that whole
thing entirely, and just do it as one single

    mov    $0x100,%eax
    lock xadd %ax,%gs:vfsmount_lock

that only has one single memory access, not three dependent ones.

But the two extra memory accesses come from:

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

 - we don't have a percpu spinlock accessor, so we always need to turn
the percpu address into a global address by adding the percpu base
(and that's the "add %gsL...,%rdx" part).

Oh well. This whole "lg_local_lock" is really noticeable on my
test-case mainly because my test-case only stat's a pathname with a
single path component, so the whole lookup really is dominated by all
the "setup/teardown" code. Real loads tend to look up much longer
pathnames, so the setup/teardown isn't so dominant, and actually
looking up the dentries from the hash chain is where most of the time
goes. But it's annoying to have that one big spike in the profile and
not being able to do anything about it.

           Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]
  Powered by Linux