lkml.org 
[lkml]   [2006]   [Jul]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] Convert idr's internal locking to _irqsave variant
    On Thu, 13 Jul 2006 08:42:47 -0700
    Roland Dreier <rdreier@cisco.com> wrote:

    > > Sigh. It was always a mistake (of the kernel programming 101 type) to put
    > > any locking at all in the idr code. At some stage we need to weed it all
    > > out and move it to callers.
    > >
    > > Your fix is yet more fallout from that mistake.
    >
    > Agreed. Consider me on the hook to fix this up in a better way once
    > my life is a little saner. Maybe I'll try to cook something up on the
    > plane ride to Ottawa.
    >

    I suspect it'll get really ugly. It's a container library which needs to
    allocate memory when items are added, like the radix-tree. Either it needs
    to assume GFP_ATOMIC, which is bad and can easily fail or it does weird
    things like radix_tree_preload().

    The basic problem is:

    idr_pre_get(GFP_KERNEL);
    spin_lock(my_lock);
    idr_get_new(..);

    which is racy, because some other CPU could have got in there and consumed
    some of the pool which was precharged by idr_pre_get().

    It's wildly improbable that it'll actually fail. It requires all of:

    a) that the race occur

    b) that the racing thread consume an appreciable amount of the pool

    c) that this thread also consume an appreciable amount (such that the
    total of both exceeds the pool size).

    d) that a (needs to be added) GFP_ATOMIC attempt to replenish the pool
    inside idr_get_new() fails.


    -
    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: 2006-07-13 23:01    [W:0.025 / U:0.016 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site