lkml.org 
[lkml]   [2014]   [Jun]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: safety of *mutex_unlock() (Was: [BUG] signal: sighand unprotected when accessed by /proc)
    From
    On Tue, Jun 10, 2014 at 5:56 AM, Paul E. McKenney
    <paulmck@linux.vnet.ibm.com> wrote:
    >
    > So to safely free a structure containing a mutex, is there some better
    > approach than the following?

    Yes.

    A data structure *containing* a mutex is fine. The problem only occurs
    when that mutex also acts the lock for the data structure. As a
    result, there are two fixes for the "locks are not one single atomic
    field":

    (a) don't do self-locking objects
    (b) use spinlocks for these "self-locking" objects

    And quite frankly, (a) is the normal "solution" to the problem.

    The fact is, having the data structure contain its own lifetime lock
    is unusual, and generally broken. The *normal* sequence for freeing
    something should be that the last access to it is the atomic referenc
    count access:

    .. do whatever, including "unlock(&mem->lock)" ..
    if (atomic_dec_and_test(&mem->refcount))
    .. we can now free it ..

    and that's safe. It doesn't matter if "mem" had a mutex in it, it
    doesn't matter if you walked around three times widdershins with a
    dead chicken around your neck. You can do whatever you want, the above
    is fine (and you shouldn't even need to worry about CPU memory
    ordering, because the alloc/free had better have the barriers to make
    sure) that nothing can leak from a free to the next allocation,
    although that is another discussion perhaps worth having).

    The whole notion of having the lock that protects the lifetime of the
    data structure inside the structure itself is pretty crazy. Because
    while it is true that we sometimes have the refcount be non-atomic,
    and instead protected with a lock, that lock is generally always
    *outside* the object itself, because you want the lock for lookup etc.
    So having the lock _and_ the refcount be inside the object really is
    crazy.

    That said, "crazy" has happened. We do occasionally do it. It's
    generally a mistake (the last example of this was the pipe thing), but
    sometimes we do it on purpose (the dcache, for example). You can do
    lookups without holding a lock (generally using RCU), and taking the
    lock and incrementing a refcount, but then the lock has to be a
    spinlock *anyway*, so that's ok.

    The last case where we actually had this bug (the afore-mentioned pipe
    thing), the mutex lock wasn't even needed - we had a real spinlock
    protecting the reference count. The bug was that we did the mutex
    unlock *after* the spinlock, for no good reason.

    So it really isn't normally a problem. The RT spinlock conversion
    people need to be aware of it, but normally you can't even screw this
    up.

    Linus


    \
     
     \ /
      Last update: 2014-06-10 18:41    [W:2.938 / U:0.152 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site