All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: akpm@linux-foundation.org
Cc: linux-kernel@vger.kernel.org, Tejun Heo <tj@kernel.org>
Subject: [PATCH 02/77] idr: make idr_destroy() imply idr_remove_all()
Date: Wed,  6 Feb 2013 11:39:34 -0800	[thread overview]
Message-ID: <1360179649-22465-3-git-send-email-tj@kernel.org> (raw)
In-Reply-To: <1360179649-22465-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 ca5aa00..b8602e0 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


  parent reply	other threads:[~2013-02-06 19:49 UTC|newest]

Thread overview: 134+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-06 19:39 [PATCHSET] idr: deprecate idr_remova_all() and add idr_alloc() Tejun Heo
2013-02-06 19:39 ` [PATCH 01/77] idr: fix a subtle bug in idr_get_next() Tejun Heo
2013-02-06 19:39 ` Tejun Heo [this message]
2013-02-06 19:39 ` [PATCH 03/77] atm/nicstar: don't use idr_remove_all() Tejun Heo
2013-02-06 19:39 ` [PATCH 04/77] block/loop: " Tejun Heo
2013-02-06 19:39 ` [PATCH 05/77] firewire: " Tejun Heo
2013-02-06 19:39 ` [PATCH 06/77] drm: " Tejun Heo
2013-02-06 19:39   ` Tejun Heo
2013-02-06 19:39 ` [PATCH 07/77] dm: " Tejun Heo
2013-02-06 19:39 ` [PATCH 08/77] remoteproc: " Tejun Heo
2013-02-06 19:39 ` [PATCH 09/77] rpmsg: " Tejun Heo
2013-02-06 19:39 ` [PATCH 10/77] dlm: use idr_for_each_entry() in recover_idr_clear() error path Tejun Heo
2013-02-06 19:39   ` [Cluster-devel] " Tejun Heo
2013-02-06 19:39 ` [PATCH 11/77] dlm: don't use idr_remove_all() Tejun Heo
2013-02-06 19:39   ` [Cluster-devel] " Tejun Heo
2013-02-06 19:39 ` [PATCH 12/77] nfs: idr_destroy() no longer needs idr_remove_all() Tejun Heo
2013-02-06 19:39 ` [PATCH 13/77] inotify: don't use idr_remove_all() Tejun Heo
     [not found] ` <1360179649-22465-1-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2013-02-06 19:39   ` [PATCH 14/77] cgroup: " Tejun Heo
2013-02-06 19:39     ` Tejun Heo
     [not found]     ` <1360179649-22465-15-git-send-email-tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2013-02-07  1:29       ` Li Zefan
2013-02-07  1:29     ` Li Zefan
2013-02-07  1:29       ` Li Zefan
2013-02-06 19:40   ` [PATCH 42/77] IB/ehca: convert to idr_alloc() Tejun Heo
2013-02-06 19:40     ` Tejun Heo
2013-02-06 19:40   ` [PATCH 71/77] cgroup: " Tejun Heo
2013-02-06 19:40     ` Tejun Heo
2013-02-06 19:39 ` [PATCH 15/77] idr: deprecate idr_remove_all() Tejun Heo
2013-02-06 19:39 ` [PATCH 16/77] idr: cosmetic updates to struct / initializer definitions Tejun Heo
2013-02-06 19:39 ` [PATCH 17/77] idr: relocate idr_for_each_entry() and reorganize id[r|a]_get_new() Tejun Heo
2013-02-06 19:39 ` [PATCH 18/77] idr: remove _idr_rc_to_errno() hack Tejun Heo
2013-02-06 19:39 ` [PATCH 19/77] idr: refactor idr_get_new_above() Tejun Heo
2013-02-06 19:39 ` [PATCH 20/77] idr: implement idr_preload[_end]() and idr_alloc() Tejun Heo
2013-02-07 19:53   ` [PATCH v3 " Tejun Heo
2013-02-06 19:39 ` [PATCH 21/77] block: fix synchronization and limit check in blk_alloc_devt() Tejun Heo
2013-02-06 22:24   ` Andrew Morton
2013-02-06 22:27     ` Tejun Heo
2013-02-06 22:32       ` Andrew Morton
2013-02-06 22:33         ` Tejun Heo
2013-02-06 19:39 ` [PATCH 22/77] block: convert to idr_alloc() Tejun Heo
2013-02-06 19:39 ` [PATCH 23/77] block/loop: " Tejun Heo
2013-02-07 18:25   ` [PATCH 22.5/77] block/loop: fix error return value in loop_add() Tejun Heo
2013-02-07 18:26   ` [PATCH v2 23/77] block/loop: convert to idr_alloc() Tejun Heo
2013-02-06 19:39 ` [PATCH 24/77] atm/nicstar: " Tejun Heo
2013-02-06 19:39 ` [PATCH 25/77] drbd: " Tejun Heo
2013-02-06 19:39 ` [PATCH 26/77] dca: " Tejun Heo
2013-02-06 19:39 ` [PATCH 27/77] dmaengine: " Tejun Heo
2013-02-06 19:40 ` [PATCH 28/77] firewire: add minor number range check to fw_device_init() Tejun Heo
2013-02-06 19:40 ` [PATCH 29/77] firewire: convert to idr_alloc() Tejun Heo
2013-02-06 19:40 ` [PATCH 30/77] gpio: " Tejun Heo
2013-02-06 19:40 ` [PATCH 31/77] drm: " Tejun Heo
2013-02-06 19:40   ` Tejun Heo
2013-02-06 19:40 ` [PATCH 32/77] drm/exynos: " Tejun Heo
2013-02-06 19:40 ` [PATCH 33/77] drm/i915: " Tejun Heo
2013-02-06 19:40 ` [PATCH 34/77] drm/sis: " Tejun Heo
2013-02-06 19:40   ` Tejun Heo
2013-02-06 19:40 ` [PATCH 35/77] drm/via: " Tejun Heo
2013-02-06 19:40   ` Tejun Heo
2013-02-06 19:40 ` [PATCH 36/77] drm/vmwgfx: " Tejun Heo
2013-02-06 19:40 ` [PATCH 37/77] i2c: " Tejun Heo
2013-02-06 19:40   ` Tejun Heo
2013-02-07 15:28   ` Mark Brown
2013-02-07 16:32     ` Tejun Heo
2013-02-07 16:32       ` Tejun Heo
2013-02-07 16:39       ` Mark Brown
2013-02-07 16:39         ` Mark Brown
2013-02-07 16:55         ` [PATCH v2] " Tejun Heo
2013-02-07 16:55           ` Tejun Heo
2013-02-07 18:52           ` Mark Brown
2013-02-08 12:10           ` Mark Brown
2013-02-08 12:10             ` Mark Brown
2013-02-10 11:47           ` Wolfram Sang
2013-02-10 11:47             ` Wolfram Sang
2013-02-12 17:34             ` [PATCH -mm] i2c: style cleanups after idr_alloc() conversion Tejun Heo
2013-02-12 17:34               ` Tejun Heo
2013-02-12 17:36               ` Tejun Heo
2013-02-12 17:36                 ` Tejun Heo
2013-02-12 18:00                 ` Jean Delvare
2013-02-12 18:00                   ` Jean Delvare
2013-02-13 20:42               ` Wolfram Sang
2013-02-13 20:42                 ` Wolfram Sang
2013-02-06 19:40 ` [PATCH 38/77] IB/core: convert to idr_alloc() Tejun Heo
2013-02-06 19:40 ` [PATCH 39/77] IB/amso1100: " Tejun Heo
2013-02-06 19:40 ` [PATCH 40/77] IB/cxgb3: " Tejun Heo
2013-02-06 19:40 ` [PATCH 41/77] IB/cxgb4: " Tejun Heo
2013-02-06 19:40 ` [PATCH 43/77] IB/ipath: " Tejun Heo
2013-02-06 19:40 ` [PATCH 44/77] IB/mlx4: " Tejun Heo
2013-02-06 19:40 ` [PATCH 45/77] IB/ocrdma: " Tejun Heo
2013-02-06 19:40 ` [PATCH 46/77] IB/qib: " Tejun Heo
2013-02-06 19:40 ` [PATCH 47/77] dm: " Tejun Heo
2013-02-06 19:40   ` Tejun Heo
2013-02-06 19:40 ` [PATCH 48/77] memstick: " Tejun Heo
2013-02-06 19:40 ` [PATCH 49/77] mfd: " Tejun Heo
2013-02-06 19:40 ` [PATCH 50/77] misc/c2port: " Tejun Heo
2013-02-06 19:40 ` [PATCH 51/77] misc/tifm_core: " Tejun Heo
2013-02-06 19:40 ` [PATCH 52/77] mmc: " Tejun Heo
2013-02-06 19:40 ` [PATCH 53/77] mtd: " Tejun Heo
2013-02-06 19:40   ` Tejun Heo
2013-02-06 19:40 ` [PATCH 54/77] macvtap: " Tejun Heo
2013-02-06 19:40 ` [PATCH 55/77] ppp: " Tejun Heo
2013-02-06 19:40   ` Tejun Heo
2013-02-06 19:40 ` [PATCH 56/77] power: " Tejun Heo
2013-02-06 19:40 ` [PATCH 57/77] pps: " Tejun Heo
2013-02-06 19:40 ` [PATCH 58/77] remoteproc: " Tejun Heo
2013-02-06 19:40 ` [PATCH 59/77] rpmsg: " Tejun Heo
2013-02-06 19:40 ` [PATCH 60/77] scsi/bfa: " Tejun Heo
2013-02-06 19:40 ` [PATCH 61/77] scsi: " Tejun Heo
2013-02-06 19:40 ` [PATCH 62/77] target/iscsi: " Tejun Heo
2013-02-06 19:40 ` [PATCH 63/77] scsi/lpfc: " Tejun Heo
2013-02-11 22:47   ` James Smart
2013-02-11 22:47     ` James Smart
2013-02-06 19:40 ` [PATCH 64/77] thermal: " Tejun Heo
2013-02-06 19:40 ` [PATCH 65/77] uio: " Tejun Heo
2013-02-06 19:40 ` [PATCH 66/77] vfio: " Tejun Heo
2013-02-06 19:40 ` [PATCH 67/77] dlm: " Tejun Heo
2013-03-11 19:29   ` David Teigland
2013-03-11 20:28     ` Tejun Heo
2013-03-12 15:17       ` David Teigland
2013-03-12 21:22         ` [PATCH] idr: idr_alloc() shouldn't trigger lowmem warning when preloaded Tejun Heo
2013-02-06 19:40 ` [PATCH 68/77] inotify: convert to idr_alloc() Tejun Heo
2013-02-06 19:40 ` [PATCH 69/77] ocfs2: " Tejun Heo
2013-02-06 19:40 ` [PATCH 70/77] ipc: " Tejun Heo
2013-02-07 19:43   ` [PATCH v2 " Tejun Heo
2013-02-06 19:40 ` [PATCH 72/77] events: " Tejun Heo
2013-02-06 19:40 ` [PATCH 73/77] posix-timers: " Tejun Heo
2013-02-06 19:40 ` [PATCH 74/77] net/9p: " Tejun Heo
2013-02-06 19:40 ` [PATCH 75/77] mac80211: " Tejun Heo
2013-02-06 19:40 ` [PATCH 76/77] sctp: " Tejun Heo
2013-02-06 19:40   ` Tejun Heo
2013-02-06 20:07   ` Vlad Yasevich
2013-02-06 20:07     ` Vlad Yasevich
2013-02-07 14:49   ` Neil Horman
2013-02-07 14:49     ` Neil Horman
2013-02-06 19:40 ` [PATCH 77/77] nfs4client: " Tejun Heo
2013-02-08  4:04 ` [PATCHSET] idr: deprecate idr_remova_all() and add idr_alloc() Dave Airlie

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=1360179649-22465-3-git-send-email-tj@kernel.org \
    --to=tj@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.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 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.