Re: [PATCH] Convert idr's internal locking to _irqsave variant

[Andrew Morton]

On Thu, 13 Jul 2006 14:03:21 -0700
Roland Dreier <rdreier@xxxxxxxxx> wrote:

>  > 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().
> 
> Actually I don't think it has to be too bad.  We could tweak the
> interface a little bit so that consumers do something like:
> 
> 	struct idr_layer *layer = NULL;	/* opaque */
> 
> retry:
>         spin_lock(&my_idr_lock);
> 	ret = idr_get_new(&my_idr, ptr, &id, layer);
>         spin_unlock(&my_idr_lock);
> 
>         if (ret == -EAGAIN) {
> 		layer = idr_alloc_layer(&my_idr, GFP_KERNEL);
> 		if (!IS_ERR(layer))
> 			goto retry;
> 	}
> 
> in other words make the consumer responsible for passing in new memory
> that can be used for a new entry (or freed if other entries have
> become free in the meantime).
> 

Good point, a try-again loop would work.  Do we really need the caller to
maintain a cache?  I suspect something like

drat:
	if (idr_pre_get(GFP_KERNEL) == ENOMEM)
		give_up();
	spin_lock();
	ret = idr_get_new();
	spin_unlock();
	if (ret == ENOMEM)
		goto drat;

would do it.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/