Messages in this thread | | | Date | Thu, 13 Jul 2006 13:54:46 -0700 | From | Andrew Morton <> | Subject | Re: [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/
| |