mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Rasmus Villemoes <linux@rasmusvillemoes.dk>
To: Tejun Heo <tj@kernel.org>, Andrew Morton <akpm@linux-foundation.org>
Cc: linux-kernel@vger.kernel.org,
	Lai Jiangshan <jiangshanlai@gmail.com>,
	Jens Axboe <axboe@kernel.dk>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>
Subject: [RFC 03/10] lib/idr.c: only fill ida->idr when needed
Date: Thu,  8 Dec 2016 02:22:58 +0100	[thread overview]
Message-ID: <1481160187-9652-4-git-send-email-linux@rasmusvillemoes.dk> (raw)
In-Reply-To: <1481160187-9652-1-git-send-email-linux@rasmusvillemoes.dk>

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@rasmusvillemoes.dk>
---
 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

  parent reply	other threads:[~2016-12-08  1:29 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-08  1:22 [RFC 00/10] implement alternative and much simpler id allocator Rasmus Villemoes
2016-12-08  1:22 ` [RFC 01/10] lib/idr.c: reused free bitmaps are already clear Rasmus Villemoes
2016-12-08  1:22 ` [RFC 02/10] lib/idr.c: delete useless condition Rasmus Villemoes
2016-12-08  1:22 ` Rasmus Villemoes [this message]
2016-12-08  1:22 ` [RFC 04/10] lib/tida.c: a very simple integer id allocator Rasmus Villemoes
2016-12-08  1:23 ` [RFC 05/10] kernel/workqueue.c: replace id allocator ida with tida Rasmus Villemoes
2016-12-08  1:23 ` [RFC 06/10] block: use tida as small id allocator Rasmus Villemoes
2016-12-08  3:56   ` Jens Axboe
2016-12-08 11:02     ` Greg Kroah-Hartman
2016-12-08  1:23 ` [RFC 07/10] drivers/base/platform.c: use simpler " Rasmus Villemoes
2016-12-08  1:23 ` [RFC 08/10] lib/tida.c: introduce tida_get_above Rasmus Villemoes
2016-12-08  1:23 ` [RFC 09/10] drm: use simpler id allocator Rasmus Villemoes
2016-12-08  1:23 ` [RFC 10/10] fs/devpts: use tida for id allocation Rasmus Villemoes
2016-12-09 13:49 ` [RFC 00/10] implement alternative and much simpler id allocator Tejun Heo
2016-12-09 22:01 ` Andrew Morton
2016-12-12 17:09   ` Tejun Heo
2016-12-12 17:35     ` Matthew Wilcox
2016-12-12 18:05       ` Tejun Heo
2016-12-16 19:14   ` Matthew Wilcox
2016-12-16 20:32     ` Rasmus Villemoes
2016-12-16 21:09       ` Matthew Wilcox
2016-12-17 13:28       ` Matthew Wilcox
2016-12-22 23:46         ` Rasmus Villemoes
2016-12-23 17:03           ` Matthew Wilcox
2016-12-18  2:42       ` Matthew Wilcox

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1481160187-9652-4-git-send-email-linux@rasmusvillemoes.dk \
    --to=linux@rasmusvillemoes.dk \
    --cc=akpm@linux-foundation.org \
    --cc=axboe@kernel.dk \
    --cc=gregkh@linuxfoundation.org \
    --cc=jiangshanlai@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome