lkml.org 
[lkml]   [2013]   [Sep]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: Avoiding the dentry d_lock on final dput(), part deux: transactional memory
    On Tue, Oct 01, 2013 at 12:05:03PM +1000, Benjamin Herrenschmidt wrote:
    > On Mon, 2013-09-30 at 17:56 -0700, Linus Torvalds wrote:
    > > On Mon, Sep 30, 2013 at 5:36 PM, Michael Neuling <mikey@neuling.org> wrote:
    > > >
    > > > The scary part is that we to make all register volatile. You were not
    > > > that keen on doing this as there are a lot of places in exception
    > > > entry/exit where we only save/restore a subset of the registers. We'd
    > > > need to catch all these.
    > >
    > > Ugh. It's very possible it's not worth using for the kernel then. The
    > > example I posted is normally fine *without* any transactional support,
    > > since it's a very local per-dentry lock, and since we only take that
    > > lock when the last reference drops (so it's not some common directory
    > > dentry, it's a end-point file dentry). In fact, on ARM they just made
    > > the cmpxchg much faster by making it entirely non-serializing (since
    > > it only updates a reference count, there is no locking involved apart
    > > from checking that the lock state is unlocked)

    A memory-barrier-free cmpxchg() would be easy on Power as well.

    > > So there is basically never any contention, and the transaction needs
    > > to basically be pretty much the same cost as a "cmpxchg". It's not
    > > clear if the intel TSX is good enough for that, and if you have to
    > > save a lot of registers in order to use transactions on POWER8, I
    > > doubt it's worthwhile.
    >
    > Well we don't have to, I think Mikey wasn't totally clear about that
    > "making all registers volatile" business :-) This is just something we
    > need to handle in assembly if we are going to reclaim the suspended
    > transaction.
    >
    > So basically, what we need is something along the lines of
    > enable_kernel_tm() which checks if there's a suspended user transaction
    > and if yes, kills/reclaims it.
    >
    > Then we also need to handle in our interrupt handlers that we have an
    > active/suspended transaction from a kernel state, which we don't deal
    > with at this point, and do whatever has to be done to kill it... we
    > might get away with something simple if we can state that we only allow
    > kernel transactions at task level and not from interrupt/softirq
    > contexts, at least initially.

    Call me a coward, but this is starting to sound a bit scary. ;-)

    > > We have very few - if any - locks where contention or even cache
    > > bouncing is common or normal. Sure, we have a few particular loads
    > > that can trigger it, but even that is becoming rare. So from a
    > > performance standpoint, the target always needs to be "comparable to
    > > hot spinlock in local cache".
    >
    > I am not quite familiar with the performance profile of our
    > transactional hardware. I think we should definitely try to hack
    > something together for that dput() case and measure it.
    >
    > > >> They also have interesting ordering semantics vs. locks, we need to be
    > > >> a tad careful (as long as we don't access a lock variable
    > > >> transactionally we should be ok. If we do, then spin_unlock needs a
    > > >> stronger barrier).
    > > >
    > > > Yep.
    > >
    > > Well, just about any kernel transaction will at least read the state
    > > of a lock. Without that, it's generally totally useless. My dput()
    > > example sequence very much verified that the lock was not held, for
    > > example.
    > >
    > > I'm not sure how that affects anything. The actual transaction had
    > > better not be visible inside the locked region (ie as far as any lock
    > > users go, transactions better all happen fully before or after the
    > > lock, if they read the lock and see it being unlocked).
    > >
    > > That said, I cannot see how POWER8 could possibly violate that rule.
    > > The whole "transactions are atomic" is kind of the whole and only
    > > point of a transaction. So I'm not sure what odd lock restrictions
    > > POWER8 could have.
    >
    > Has to do with the memory model :-(
    >
    > I dug the whole story from my mbox and the situation is indeed as dire
    > as feared. If the transaction reads the lock, then the corresponding
    > spin_lock must have a full sync barrier in it instead of the current
    > lighter one.
    >
    > Now I believe we are already "on the fence" with our locks today since
    > technically speaking, our unlock + lock sequence is *not* exactly a full
    > barrier (it is only if it's the same lock I think)
    >
    > CC'ing Paul McKenney here who's been chasing that issue. In the end, we
    > might end up having to turn our locks into sync anyway

    Well, there have been a lot of fixed software bugs since the last
    suspicious sighting, but on the other hand, I am just now getting my
    RCU testing going again on Power. I would certainly feel better
    about things if unlock-lock was really truly a full barrier, but
    this clearly needs a clean sighting.

    > Yay ! The isanity^Wjoy of an OO memory model !

    ;-) ;-) ;-)

    Thanx, Paul

    > > > FWIW eg.
    > > >
    > > > tbegin
    > > > beq abort /* passes first time through */
    > > > ....
    > > > transactional stuff
    > > > ....
    > > > tend
    > > > b pass
    > > >
    > > > abort:
    > > >
    > > > pass:
    > >
    > > That's fine, and matches the x86 semantics fairly closely, except
    > > "xbegin" kind of "contains" that "jump to abort address". But we could
    > > definitely use the same models. Call it
    > > "transaction_begin/abort/end()", and it should be architecture-neutral
    > > naming-wise.
    >
    > Right.
    >
    > > Of course, if tbegin then acts basically like some crazy
    > > assembly-level setjmp (I'm guessing it does exactly, and presumably
    > > precisely that kind of compiler support - ie a function with
    > > "__attribute((returns_twice))" in gcc-speak), the overhead of doing it
    > > may kill it.
    >
    > Well, all the registers are checkpointed so we *should* be ok but gcc
    > always makes me nervous in those circumstances ...
    >
    > Ben.
    >
    >
    > > Linus
    > > --
    > > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    > > the body of a message to majordomo@vger.kernel.org
    > > More majordomo info at http://vger.kernel.org/majordomo-info.html
    > > Please read the FAQ at http://www.tux.org/lkml/
    >
    >



    \
     
     \ /
      Last update: 2013-10-01 05:21    [W:6.584 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site