[lkml]   [2013]   [Jan]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
Subject[PATCHSET] idr: deprecate idr_remove_all()

(Andrew, I think this one is best routed through -mm. Please read on)

idr is one of the areas with much higher concentration of bad
interface and implementation decisions. This patchset removes one of
those oddities - idr_remove_all(). idr needs two steps for
destruction - idr_remove_all() followed by idr_destroy().
idr_remove_all() releases all IDs in use but doesn't release buffered
idr_layers. idr_destroy() frees buffered idr_layers() but doesn't
bother with in-use idr_layers.

For added fun, calling idr_remove() on all allocated IDs doesn't
necessarily free all in-use idr_layers, so idr_for_each_entry()
idr_remove(); followed by idr_destroy() may still leak memory.

This confuses people. Some correctly use both. Many forget to call
idr_remove_all() and others forget idr_destroy() and they all leak
memory. Even ida - something tightly coupled w/ idr - forgets to do
idr_remove_all() (although it's my fault).

This is just a bad interface. While remove_all in itself might not be
that bad, there is only one legitimate user of idr_remove_all() which
can be converted to idr_remove() relatively easily, so I think it'd be
better to deprecate and later unexport it than keeping it around.

This patchset contains the following 14 patches.


0001 makes idr_destroy() imply idr_remove_all(). 0002-0013 remove
uses of idr_remove_all(). 0014 marks idr_remove_all() deprecated.
The patches are on top of the current linus#master 66e2d3e8c2 and also
applies on top of the current -mm. It's available in the following
git branch.

git:// deprecate-idr_remove_all

As changes to most are trivial and have dependency on the first patch,
I think it would be best to route these together. The only
non-trivial change is 0009 and 0010 which converts idr_for_each() to
idr_for_each_entry() and then replaces idr_remove_all() with
idr_remove() inside the for_each_entry loop. Defintely wanna get acks
from dlm people.

Andrew, once people agree with the series, can you please route these
through -mm?

diffstat follows. Thanks.

drivers/atm/nicstar.c | 1 -
drivers/block/loop.c | 1 -
drivers/firewire/core-cdev.c | 1 -
drivers/gpu/drm/drm_context.c | 2 +-
drivers/gpu/drm/drm_crtc.c | 1 -
drivers/gpu/drm/drm_drv.c | 1 -
drivers/gpu/drm/drm_gem.c | 2 --
drivers/gpu/drm/exynos/exynos_drm_ipp.c | 4 ----
drivers/gpu/drm/sis/sis_drv.c | 1 -
drivers/gpu/drm/via/via_map.c | 1 -
drivers/md/dm.c | 1 -
drivers/remoteproc/remoteproc_core.c | 1 -
drivers/rpmsg/virtio_rpmsg_bus.c | 1 -
fs/dlm/lockspace.c | 1 -
fs/dlm/recover.c | 25 +++++++++++--------------
fs/nfs/client.c | 1 -
fs/notify/inotify/inotify_fsnotify.c | 1 -
include/linux/idr.h | 14 +++++++++++++-
kernel/cgroup.c | 4 +---
lib/idr.c | 28 +++++++++++++---------------
20 files changed, 39 insertions(+), 53 deletions(-)


 \ /
  Last update: 2013-01-26 03:21    [W:0.145 / U:35.968 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site