All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: akpm@linux-foundation.org
Cc: rusty@rustcorp.com.au, linux-kernel@vger.kernel.org,
	Tejun Heo <tj@kernel.org>
Subject: [PATCH 01/14] idr: make idr_destroy() imply idr_remove_all()
Date: Fri, 25 Jan 2013 17:30:59 -0800	[thread overview]
Message-ID: <1359163872-1949-2-git-send-email-tj@kernel.org> (raw)
In-Reply-To: <1359163872-1949-1-git-send-email-tj@kernel.org>

idr is silly in quite a few ways, one of which is how it's supposed to
be destroyed - idr_destroy() doesn't release IDs and doesn't even
whine if the idr isn't empty.  If the caller forgets idr_remove_all(),
it simply leaks memory.

Even ida gets this wrong and leaks memory on destruction.  There is
absoltely no reason not to call idr_remove_all() from idr_destroy().
Nobody is abusing idr_destroy() for shrinking free layer buffer and
continues to use idr after idr_destroy(), so it's safe to do
remove_all from destroy.

In the whole kernel, there is only one place where idr_remove_all() is
legitimiately used without following idr_destroy() while there are
quite a few places where the caller forgets either idr_remove_all() or
idr_destroy() leaking memory.

This patch makes idr_destroy() call idr_destroy_all() and updates the
function description accordingly.

Signed-off-by: Tejun Heo <tj@kernel.org>
---
 lib/idr.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/lib/idr.c b/lib/idr.c
index 6482390..1e47832 100644
--- a/lib/idr.c
+++ b/lib/idr.c
@@ -436,15 +436,6 @@ EXPORT_SYMBOL(idr_remove);
 /**
  * idr_remove_all - remove all ids from the given idr tree
  * @idp: idr handle
- *
- * idr_destroy() only frees up unused, cached idp_layers, but this
- * function will remove all id mappings and leave all idp_layers
- * unused.
- *
- * A typical clean-up sequence for objects stored in an idr tree will
- * use idr_for_each() to free all objects, if necessay, then
- * idr_remove_all() to remove all ids, and idr_destroy() to free
- * up the cached idr_layers.
  */
 void idr_remove_all(struct idr *idp)
 {
@@ -484,9 +475,20 @@ EXPORT_SYMBOL(idr_remove_all);
 /**
  * idr_destroy - release all cached layers within an idr tree
  * @idp: idr handle
+ *
+ * Free all id mappings and all idp_layers.  After this function, @idp is
+ * completely unused and can be freed / recycled.  The caller is
+ * responsible for ensuring that no one else accesses @idp during or after
+ * idr_destroy().
+ *
+ * A typical clean-up sequence for objects stored in an idr tree will use
+ * idr_for_each() to free all objects, if necessay, then idr_destroy() to
+ * free up the id mappings and cached idr_layers.
  */
 void idr_destroy(struct idr *idp)
 {
+	idr_remove_all(idp);
+
 	while (idp->id_free_cnt) {
 		struct idr_layer *p = get_from_free_list(idp);
 		kmem_cache_free(idr_layer_cache, p);
-- 
1.8.1


  reply	other threads:[~2013-01-26  1:31 UTC|newest]

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-26  1:30 [PATCHSET] idr: deprecate idr_remove_all() Tejun Heo
2013-01-26  1:30 ` Tejun Heo [this message]
2013-01-26  1:31 ` [PATCH 02/14] atm/nicstar: don't use idr_remove_all() Tejun Heo
2013-01-27  6:43   ` David Miller
2013-02-04 17:52   ` [PATCH v2 " Tejun Heo
2013-02-04 18:10     ` chas williams - CONTRACTOR
2013-02-04 18:35       ` Tejun Heo
2013-01-26  1:31 ` [PATCH 03/14] block/loop: " Tejun Heo
2013-01-26  1:31 ` [PATCH 04/14] firewire: " Tejun Heo
2013-01-26  9:07   ` Stefan Richter
2013-01-26  1:31 ` [PATCH 05/14] drm: " Tejun Heo
2013-01-26  1:31   ` Tejun Heo
2013-01-26  1:31 ` [PATCH 06/14] dm: " Tejun Heo
2013-01-26  1:31 ` [PATCH 07/14] remoteproc: " Tejun Heo
2013-01-26  1:31 ` [PATCH 08/14] rpmsg: " Tejun Heo
2013-01-26  1:31 ` [Cluster-devel] [PATCH 09/14] dlm: use idr_for_each_entry() in recover_idr_clear() error path Tejun Heo
2013-01-26  1:31   ` Tejun Heo
2013-01-28 15:55   ` [Cluster-devel] " David Teigland
2013-01-28 15:55     ` David Teigland
2013-01-26  1:31 ` [Cluster-devel] [PATCH 10/14] dlm: don't use idr_remove_all() Tejun Heo
2013-01-26  1:31   ` Tejun Heo
2013-01-28 15:57   ` [Cluster-devel] " David Teigland
2013-01-28 15:57     ` David Teigland
2013-01-29 15:13     ` [Cluster-devel] " David Teigland
2013-01-29 15:13       ` David Teigland
2013-01-30 21:24       ` [Cluster-devel] " David Teigland
2013-01-30 21:24         ` David Teigland
2013-01-31 23:53         ` [Cluster-devel] " Tejun Heo
2013-01-31 23:53           ` Tejun Heo
2013-02-01  0:18           ` [Cluster-devel] " Tejun Heo
2013-02-01  0:18             ` Tejun Heo
2013-02-01 17:44             ` [Cluster-devel] " David Teigland
2013-02-01 17:44               ` David Teigland
2013-02-01 18:00               ` [Cluster-devel] " Tejun Heo
2013-02-01 18:00                 ` Tejun Heo
2013-02-02 23:10                 ` [Cluster-devel] [PATCH] idr: fix a subtle bug in idr_get_next() Tejun Heo
2013-02-02 23:10                   ` Tejun Heo
2013-02-02 23:11                   ` [Cluster-devel] " Tejun Heo
2013-02-02 23:11                     ` Tejun Heo
2013-02-03  2:15                     ` [Cluster-devel] " Randy Dunlap
2013-02-03  2:15                       ` Randy Dunlap
2013-02-03 17:53                       ` [Cluster-devel] " Hugh Dickins
2013-02-03 17:53                         ` Hugh Dickins
2013-02-05 15:36                     ` [Cluster-devel] " David Teigland
2013-02-05 15:36                       ` David Teigland
2013-02-04  3:39                   ` [Cluster-devel] " Li Zefan
2013-02-04  3:39                     ` Li Zefan
2013-02-04 17:44                     ` [Cluster-devel] " Tejun Heo
2013-02-04 17:44                       ` Tejun Heo
2013-01-26  1:31 ` [PATCH 11/14] nfs: idr_destroy() no longer needs idr_remove_all() Tejun Heo
2013-01-29 22:58   ` J. Bruce Fields
2013-01-30  2:18     ` Myklebust, Trond
2013-02-01  0:08       ` Tejun Heo
2013-01-26  1:31 ` [PATCH 12/14] inotify: don't use idr_remove_all() Tejun Heo
     [not found] ` <1359163872-1949-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2013-01-26  1:31   ` [PATCH 13/14] cgroup: " Tejun Heo
2013-01-26  1:31     ` Tejun Heo
2013-01-26  1:31 ` [PATCH 14/14] idr: deprecate idr_remove_all() Tejun Heo
2013-01-26 13:48   ` Arnd Bergmann
2013-01-26 14:24     ` Tejun Heo

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=1359163872-1949-2-git-send-email-tj@kernel.org \
    --to=tj@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rusty@rustcorp.com.au \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.