[RFC 03/10] lib/idr.c: only fill ida->idr when needed

From: Rasmus Villemoes
Date: Wed Dec 07 2016 - 20:29:35 EST


If I'm reading the code correctly, every call of ida_simple_get (and
almost all users of ida use that) starts by calling

ida_pre_get -> __idr_pre_get

which fills up the free list of &ida->idr with MAX_IDR_FREE (aka 8)
struct idr_layers. After a successful id allocation, ida_get_new_above
gets rid of up to one of these (maybe some were used during the
allocation, but in the common case all 8 will still be there). On the
next call of ida_simple_get, we again fill up &ida->idr, and
deallocate at most one, which at least contradicts the comment at the
bottom of ida_get_new_above.

So rather than unconditionally calling ida_pre_get in ida_simple_get,
only do that if we get -EAGAIN.

But I'm not really convinced that this is a good way of doing
things. Many users of ida probably only ever allocates one or two ids,
so those would not hit the "free the excess idr_layers" often enough
to actually get rid of the extra memory. In practice, those two bits
could cost around 16KB of memory. But maybe the users shouldn't be
using ida in the first place, then. (For example, the watchdog code
always passes max=MAX_DOGS==32, so they'd be much better served with a
small fixed bitmap).

Signed-off-by: Rasmus Villemoes <linux@xxxxxxxxxxxxxxxxxx>
---
lib/idr.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/idr.c b/lib/idr.c
index 1e786f817e66..6393feb4b087 100644
--- a/lib/idr.c
+++ b/lib/idr.c
@@ -1092,9 +1092,6 @@ int ida_simple_get(struct ida *ida, unsigned int start, unsigned int end,
}

again:
- if (!ida_pre_get(ida, gfp_mask))
- return -ENOMEM;
-
spin_lock_irqsave(&simple_ida_lock, flags);
ret = ida_get_new_above(ida, start, &id);
if (!ret) {
@@ -1107,8 +1104,11 @@ int ida_simple_get(struct ida *ida, unsigned int start, unsigned int end,
}
spin_unlock_irqrestore(&simple_ida_lock, flags);

- if (unlikely(ret == -EAGAIN))
+ if (unlikely(ret == -EAGAIN)) {
+ if (!ida_pre_get(ida, gfp_mask))
+ return -ENOMEM;
goto again;
+ }

return ret;
}
--
2.1.4