[PATCH 09/10] idr: Remove unneeded idr locking, idr_preload() usage

[PATCH 09/10] idr: Remove unneeded idr locking, idr_preload() usage

[Kent Overstreet]

Our new idr implementation does its own locking, instead of forcing it
onto the callers like the old implementation.

Many of the existing idr users need locking for more than just
idr_alloc()/idr_remove()/idr_find() - they're taking refcounts and such
under their locks and we can't touch those.

But a significant number of users had locks that protected nothing more
than the idr data structures itself - those we can get rid of.

Note that we have to be careful when removing locks; in some places,
locks appear to only be protecting idr_alloc()/idr_remove() calls but
they're also used by other code that needs to ensure the idr isn't
modified while it's doing something else - so ideally we want to delete
the lock that protected the idr, or else we have to carefully audit all
the other places it's used.

There's also a fair number of places where things were being done under
the idr lock unnecessarily; drivers/dca/dca-sysfs.c is a good example.
dca->id is set to the id idr_alloc() returns under the lock - but
there's no idr_find() calls under the lock, and dca->id isn't touched in
the idr_remove() paths. So the lock can be safely deleted.

The really nice thing about deleting this unnecessary locking is that it
lets us trivially delete a lot of now unnecessary idr_preload() - with
idr doing its own locking, we can pass GFP_KERNEL to idr_alloc() just
fine.

Signed-off-by: Kent Overstreet <koverstreet@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Tejun Heo <tj@kernel.org>
Cc: David Airlie <airlied@linux.ie>
Cc: Tom Tucker <tom@opengridcomputing.com>
Cc: Steve Wise <swise@opengridcomputing.com>
Cc: Roland Dreier <roland@kernel.org>
Cc: Sean Hefty <sean.hefty@intel.com>
Cc: Hal Rosenstock <hal.rosenstock@gmail.com>
Cc: Alasdair Kergon <agk@redhat.com>
Cc: dm-devel@redhat.com
Cc: Samuel Ortiz <sameo@linux.intel.com>
Cc: Alex Dubov <oakad@yahoo.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Chris Ball <cjb@laptop.org>
Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
Cc: Willem Riede <osst@riede.org>
Cc: "Kai Mäkisara" <Kai.Makisara@kolumbus.fi>
Cc: "Nicholas A. Bellinger" <nab@linux-iscsi.org>
Cc: Li Zefan <lizefan@huawei.com>
Cc: Vlad Yasevich <vyasevich@gmail.com>
Cc: Neil Horman <nhorman@tuxdriver.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Alon Levy <alevy@redhat.com>
Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Christoph Hellwig <hch@lst.de>
Cc: linux-kernel@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: linux-rdma@vger.kernel.org
Cc: linux-mmc@vger.kernel.org
Cc: linux-scsi@vger.kernel.org
Cc: osst-users@lists.sourceforge.net
Cc: target-devel@vger.kernel.org
Cc: containers@lists.linux-foundation.org
Cc: cgroups@vger.kernel.org
Cc: linux-sctp@vger.kernel.org
Cc: netdev@vger.kernel.org
---
 drivers/dca/dca-sysfs.c                   | 18 +++---------------
 drivers/gpu/drm/qxl/qxl_cmd.c             |  4 +---
 drivers/gpu/drm/qxl/qxl_drv.h             |  1 -
 drivers/gpu/drm/qxl/qxl_kms.c             |  1 -
 drivers/gpu/drm/qxl/qxl_release.c         | 19 +++++--------------
 drivers/infiniband/hw/amso1100/c2.h       |  1 -
 drivers/infiniband/hw/amso1100/c2_qp.c    | 20 ++------------------
 drivers/md/dm.c                           | 22 ++++------------------
 drivers/memstick/core/memstick.c          | 17 +++--------------
 drivers/mfd/rtsx_pcr.c                    | 13 +++----------
 drivers/misc/c2port/core.c                | 11 +----------
 drivers/misc/tifm_core.c                  | 15 +++------------
 drivers/mmc/core/host.c                   | 13 ++-----------
 drivers/scsi/ch.c                         | 14 ++------------
 drivers/scsi/st.c                         | 13 +------------
 drivers/target/iscsi/iscsi_target.c       | 17 ++++-------------
 drivers/target/iscsi/iscsi_target.h       |  1 -
 drivers/target/iscsi/iscsi_target_login.c | 12 ++----------
 include/linux/cgroup.h                    |  1 -
 include/net/sctp/sctp.h                   |  1 -
 kernel/cgroup.c                           |  9 +--------
 kernel/workqueue.c                        | 15 ++-------------
 net/9p/util.c                             | 15 +--------------
 net/sctp/associola.c                      | 14 ++------------
 net/sctp/protocol.c                       |  1 -
 net/sctp/socket.c                         |  2 --
 26 files changed, 42 insertions(+), 228 deletions(-)

diff --git a/drivers/dca/dca-sysfs.c b/drivers/dca/dca-sysfs.c
index effda66..6be5fbd 100644
--- a/drivers/dca/dca-sysfs.c
+++ b/drivers/dca/dca-sysfs.c
@@ -31,7 +31,6 @@
 
 static struct class *dca_class;
 static struct idr dca_idr;
-static spinlock_t dca_idr_lock;
 
 int dca_sysfs_add_req(struct dca_provider *dca, struct device *dev, int slot)
 {
@@ -55,23 +54,15 @@ int dca_sysfs_add_provider(struct dca_provider *dca, struct device *dev)
 	struct device *cd;
 	int ret;
 
-	idr_preload(GFP_KERNEL);
-	spin_lock(&dca_idr_lock);
-
-	ret = idr_alloc(&dca_idr, dca, GFP_NOWAIT);
-	if (ret >= 0)
-		dca->id = ret;
-
-	spin_unlock(&dca_idr_lock);
-	idr_preload_end();
+	ret = idr_alloc(&dca_idr, dca, GFP_KERNEL);
 	if (ret < 0)
 		return ret;
 
+	dca->id = ret;
+
 	cd = device_create(dca_class, dev, MKDEV(0, 0), NULL, "dca%d", dca->id);
 	if (IS_ERR(cd)) {
-		spin_lock(&dca_idr_lock);
 		idr_remove(&dca_idr, dca->id);
-		spin_unlock(&dca_idr_lock);
 		return PTR_ERR(cd);
 	}
 	dca->cd = cd;
@@ -82,15 +73,12 @@ void dca_sysfs_remove_provider(struct dca_provider *dca)
 {
 	device_unregister(dca->cd);
 	dca->cd = NULL;
-	spin_lock(&dca_idr_lock);
 	idr_remove(&dca_idr, dca->id);
-	spin_unlock(&dca_idr_lock);
 }
 
 int __init dca_sysfs_init(void)
 {
 	idr_init(&dca_idr);
-	spin_lock_init(&dca_idr_lock);
 
 	dca_class = class_create(THIS_MODULE, "dca");
 	if (IS_ERR(dca_class)) {
diff --git a/drivers/gpu/drm/qxl/qxl_cmd.c b/drivers/gpu/drm/qxl/qxl_cmd.c
index 2f99a68..6a2a100 100644
--- a/drivers/gpu/drm/qxl/qxl_cmd.c
+++ b/drivers/gpu/drm/qxl/qxl_cmd.c
@@ -448,11 +448,9 @@ int qxl_surface_id_alloc(struct qxl_device *qdev,
 	int idr_ret;
 	int count = 0;
 again:
-	idr_preload(GFP_ATOMIC);
 	spin_lock(&qdev->surf_id_idr_lock);
-	idr_ret = idr_alloc_range(&qdev->surf_id_idr, NULL, 1, 0, GFP_NOWAIT);
+	idr_ret = idr_alloc_range(&qdev->surf_id_idr, NULL, 1, 0, GFP_ATOMIC);
 	spin_unlock(&qdev->surf_id_idr_lock);
-	idr_preload_end();
 	if (idr_ret < 0)
 		return idr_ret;
 	handle = idr_ret;
diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index 43d06ab..3ccf3a2 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -277,7 +277,6 @@ struct qxl_device {
 	uint64_t	va_slot_mask;
 
 	struct idr	release_idr;
-	spinlock_t release_idr_lock;
 	struct mutex	async_io_mutex;
 	unsigned int last_sent_io_cmd;
 
diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c
index e27ce2a..4923661 100644
--- a/drivers/gpu/drm/qxl/qxl_kms.c
+++ b/drivers/gpu/drm/qxl/qxl_kms.c
@@ -190,7 +190,6 @@ int qxl_device_init(struct qxl_device *qdev,
 			GFP_KERNEL);
 
 	idr_init(&qdev->release_idr);
-	spin_lock_init(&qdev->release_idr_lock);
 
 	idr_init(&qdev->surf_id_idr);
 	spin_lock_init(&qdev->surf_id_idr_lock);
diff --git a/drivers/gpu/drm/qxl/qxl_release.c b/drivers/gpu/drm/qxl/qxl_release.c
index 61c1b0c..0aff5f3 100644
--- a/drivers/gpu/drm/qxl/qxl_release.c
+++ b/drivers/gpu/drm/qxl/qxl_release.c
@@ -43,7 +43,6 @@ qxl_release_alloc(struct qxl_device *qdev, int type,
 		  struct qxl_release **ret)
 {
 	struct qxl_release *release;
-	int handle;
 	size_t size = sizeof(*release);
 	int idr_ret;
 
@@ -57,20 +56,16 @@ qxl_release_alloc(struct qxl_device *qdev, int type,
 	release->release_offset = 0;
 	release->surface_release_id = 0;
 
-	idr_preload(GFP_KERNEL);
-	spin_lock(&qdev->release_idr_lock);
-	idr_ret = idr_alloc_range(&qdev->release_idr, release, 1, 0, GFP_NOWAIT);
-	spin_unlock(&qdev->release_idr_lock);
-	idr_preload_end();
-	handle = idr_ret;
+	idr_ret = idr_alloc_range(&qdev->release_idr, release,
+				  1, 0, GFP_KERNEL);
 	if (idr_ret < 0)
 		goto release_fail;
 	*ret = release;
-	QXL_INFO(qdev, "allocated release %lld\n", handle);
-	release->id = handle;
+	QXL_INFO(qdev, "allocated release %lld\n", idr_ret);
+	release->id = idr_ret;
 release_fail:
 
-	return handle;
+	return idr_ret;
 }
 
 void
@@ -92,9 +87,7 @@ qxl_release_free(struct qxl_device *qdev,
 		qxl_fence_remove_release(&release->bos[i]->fence, release->id);
 		qxl_bo_unref(&release->bos[i]);
 	}
-	spin_lock(&qdev->release_idr_lock);
 	idr_remove(&qdev->release_idr, release->id);
-	spin_unlock(&qdev->release_idr_lock);
 	kfree(release);
 }
 
@@ -266,9 +259,7 @@ struct qxl_release *qxl_release_from_id_locked(struct qxl_device *qdev,
 {
 	struct qxl_release *release;
 
-	spin_lock(&qdev->release_idr_lock);
 	release = idr_find(&qdev->release_idr, id);
-	spin_unlock(&qdev->release_idr_lock);
 	if (!release) {
 		DRM_ERROR("failed to find id in release_idr\n");
 		return NULL;
diff --git a/drivers/infiniband/hw/amso1100/c2.h b/drivers/infiniband/hw/amso1100/c2.h
index d619d7358..73f2e96 100644
--- a/drivers/infiniband/hw/amso1100/c2.h
+++ b/drivers/infiniband/hw/amso1100/c2.h
@@ -264,7 +264,6 @@ struct c2_pd_table {
 
 struct c2_qp_table {
 	struct idr idr;
-	spinlock_t lock;
 };
 
 struct c2_element {
diff --git a/drivers/infiniband/hw/amso1100/c2_qp.c b/drivers/infiniband/hw/amso1100/c2_qp.c
index 86708de..cb80aa9 100644
--- a/drivers/infiniband/hw/amso1100/c2_qp.c
+++ b/drivers/infiniband/hw/amso1100/c2_qp.c
@@ -380,36 +380,21 @@ static int destroy_qp(struct c2_dev *c2dev, struct c2_qp *qp)
 
 static int c2_alloc_qpn(struct c2_dev *c2dev, struct c2_qp *qp)
 {
-	int ret;
-
-	idr_preload(GFP_KERNEL);
-	spin_lock_irq(&c2dev->qp_table.lock);
-
-	ret = idr_alloc_cyclic(&c2dev->qp_table.idr, qp, 0, 0, GFP_NOWAIT);
+	int ret = idr_alloc_cyclic(&c2dev->qp_table.idr, qp, 0, 0, GFP_KERNEL);
 	if (ret >= 0)
 		qp->qpn = ret;
 
-	spin_unlock_irq(&c2dev->qp_table.lock);
-	idr_preload_end();
 	return ret < 0 ? ret : 0;
 }
 
 static void c2_free_qpn(struct c2_dev *c2dev, int qpn)
 {
-	spin_lock_irq(&c2dev->qp_table.lock);
 	idr_remove(&c2dev->qp_table.idr, qpn);
-	spin_unlock_irq(&c2dev->qp_table.lock);
 }
 
 struct c2_qp *c2_find_qpn(struct c2_dev *c2dev, int qpn)
 {
-	unsigned long flags;
-	struct c2_qp *qp;
-
-	spin_lock_irqsave(&c2dev->qp_table.lock, flags);
-	qp = idr_find(&c2dev->qp_table.idr, qpn);
-	spin_unlock_irqrestore(&c2dev->qp_table.lock, flags);
-	return qp;
+	return idr_find(&c2dev->qp_table.idr, qpn);
 }
 
 int c2_alloc_qp(struct c2_dev *c2dev,
@@ -1014,7 +999,6 @@ out:
 
 void c2_init_qp_table(struct c2_dev *c2dev)
 {
-	spin_lock_init(&c2dev->qp_table.lock);
 	idr_init(&c2dev->qp_table.idr);
 }
 
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 8ba4fdb..e24b704 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -1775,9 +1775,7 @@ static int dm_any_congested(void *congested_data, int bdi_bits)
  *---------------------------------------------------------------*/
 static void free_minor(int minor)
 {
-	spin_lock(&_minor_lock);
 	idr_remove(&_minor_idr, minor);
-	spin_unlock(&_minor_lock);
 }
 
 /*
@@ -1790,13 +1788,8 @@ static int specific_minor(int minor)
 	if (minor >= (1 << MINORBITS))
 		return -EINVAL;
 
-	idr_preload(GFP_KERNEL);
-	spin_lock(&_minor_lock);
-
-	r = idr_alloc_range(&_minor_idr, MINOR_ALLOCED, minor, minor + 1, GFP_NOWAIT);
-
-	spin_unlock(&_minor_lock);
-	idr_preload_end();
+	r = idr_alloc_range(&_minor_idr, MINOR_ALLOCED, minor,
+			    minor + 1, GFP_KERNEL);
 	if (r < 0)
 		return r == -ENOSPC ? -EBUSY : r;
 	return 0;
@@ -1806,13 +1799,8 @@ static int next_free_minor(int *minor)
 {
 	int r;
 
-	idr_preload(GFP_KERNEL);
-	spin_lock(&_minor_lock);
-
-	r = idr_alloc_range(&_minor_idr, MINOR_ALLOCED, 0, 1 << MINORBITS, GFP_NOWAIT);
-
-	spin_unlock(&_minor_lock);
-	idr_preload_end();
+	r = idr_alloc_range(&_minor_idr, MINOR_ALLOCED,
+			    0, 1 << MINORBITS, GFP_KERNEL);
 	if (r < 0)
 		return r;
 	*minor = r;
@@ -1921,9 +1909,7 @@ static struct mapped_device *alloc_dev(int minor)
 	md->flush_bio.bi_rw = WRITE_FLUSH;
 
 	/* Populate the mapping, nobody knows we exist yet */
-	spin_lock(&_minor_lock);
 	old_md = idr_replace(&_minor_idr, md, minor);
-	spin_unlock(&_minor_lock);
 
 	BUG_ON(old_md != MINOR_ALLOCED);
 
diff --git a/drivers/memstick/core/memstick.c b/drivers/memstick/core/memstick.c
index 70fb07c..b7544a3 100644
--- a/drivers/memstick/core/memstick.c
+++ b/drivers/memstick/core/memstick.c
@@ -26,7 +26,6 @@ module_param(cmd_retries, uint, 0644);
 
 static struct workqueue_struct *workqueue;
 static DEFINE_IDR(memstick_host_idr);
-static DEFINE_SPINLOCK(memstick_host_lock);
 
 static int memstick_dev_match(struct memstick_dev *card,
 			      struct memstick_device_id *id)
@@ -512,25 +511,17 @@ int memstick_add_host(struct memstick_host *host)
 {
 	int rc;
 
-	idr_preload(GFP_KERNEL);
-	spin_lock(&memstick_host_lock);
-
-	rc = idr_alloc(&memstick_host_idr, host, GFP_NOWAIT);
-	if (rc >= 0)
-		host->id = rc;
-
-	spin_unlock(&memstick_host_lock);
-	idr_preload_end();
+	rc = idr_alloc(&memstick_host_idr, host, GFP_KERNEL);
 	if (rc < 0)
 		return rc;
 
+	host->id = rc;
+
 	dev_set_name(&host->dev, "memstick%u", host->id);
 
 	rc = device_add(&host->dev);
 	if (rc) {
-		spin_lock(&memstick_host_lock);
 		idr_remove(&memstick_host_idr, host->id);
-		spin_unlock(&memstick_host_lock);
 		return rc;
 	}
 
@@ -554,9 +545,7 @@ void memstick_remove_host(struct memstick_host *host)
 	host->set_param(host, MEMSTICK_POWER, MEMSTICK_POWER_OFF);
 	mutex_unlock(&host->lock);
 
-	spin_lock(&memstick_host_lock);
 	idr_remove(&memstick_host_idr, host->id);
-	spin_unlock(&memstick_host_lock);
 	device_del(&host->dev);
 }
 EXPORT_SYMBOL(memstick_remove_host);
diff --git a/drivers/mfd/rtsx_pcr.c b/drivers/mfd/rtsx_pcr.c
index 905513d..ad53e45 100644
--- a/drivers/mfd/rtsx_pcr.c
+++ b/drivers/mfd/rtsx_pcr.c
@@ -40,7 +40,6 @@ module_param(msi_en, bool, S_IRUGO | S_IWUSR);
 MODULE_PARM_DESC(msi_en, "Enable MSI");
 
 static DEFINE_IDR(rtsx_pci_idr);
-static DEFINE_SPINLOCK(rtsx_pci_lock);
 
 static struct mfd_cell rtsx_pcr_cells[] = {
 	[RTSX_SD_CARD] = {
@@ -1096,16 +1095,12 @@ static int rtsx_pci_probe(struct pci_dev *pcidev,
 	}
 	handle->pcr = pcr;
 
-	idr_preload(GFP_KERNEL);
-	spin_lock(&rtsx_pci_lock);
-	ret = idr_alloc(&rtsx_pci_idr, pcr, GFP_NOWAIT);
-	if (ret >= 0)
-		pcr->id = ret;
-	spin_unlock(&rtsx_pci_lock);
-	idr_preload_end();
+	ret = idr_alloc(&rtsx_pci_idr, pcr, GFP_KERNEL);
 	if (ret < 0)
 		goto free_handle;
 
+	pcr->id = ret;
+
 	pcr->pci = pcidev;
 	dev_set_drvdata(&pcidev->dev, handle);
 
@@ -1211,9 +1206,7 @@ static void rtsx_pci_remove(struct pci_dev *pcidev)
 	pci_release_regions(pcidev);
 	pci_disable_device(pcidev);
 
-	spin_lock(&rtsx_pci_lock);
 	idr_remove(&rtsx_pci_idr, pcr->id);
-	spin_unlock(&rtsx_pci_lock);
 
 	kfree(pcr->slots);
 	kfree(pcr);
diff --git a/drivers/misc/c2port/core.c b/drivers/misc/c2port/core.c
index 8a1ab10..5cd8c99 100644
--- a/drivers/misc/c2port/core.c
+++ b/drivers/misc/c2port/core.c
@@ -27,7 +27,6 @@
 #define DRIVER_NAME             "c2port"
 #define DRIVER_VERSION          "0.51.0"
 
-static DEFINE_SPINLOCK(c2port_idr_lock);
 static DEFINE_IDR(c2port_idr);
 
 /*
@@ -897,11 +896,7 @@ struct c2port_device *c2port_device_register(char *name,
 	if (unlikely(!c2dev))
 		return ERR_PTR(-ENOMEM);
 
-	idr_preload(GFP_KERNEL);
-	spin_lock_irq(&c2port_idr_lock);
-	ret = idr_alloc(&c2port_idr, c2dev, GFP_NOWAIT);
-	spin_unlock_irq(&c2port_idr_lock);
-	idr_preload_end();
+	ret = idr_alloc(&c2port_idr, c2dev, GFP_KERNEL);
 
 	if (ret < 0)
 		goto error_idr_alloc;
@@ -941,9 +936,7 @@ error_device_create_bin_file:
 	device_destroy(c2port_class, 0);
 
 error_device_create:
-	spin_lock_irq(&c2port_idr_lock);
 	idr_remove(&c2port_idr, c2dev->id);
-	spin_unlock_irq(&c2port_idr_lock);
 
 error_idr_alloc:
 	kfree(c2dev);
@@ -960,9 +953,7 @@ void c2port_device_unregister(struct c2port_device *c2dev)
 	dev_info(c2dev->dev, "C2 port %s removed\n", c2dev->name);
 
 	device_remove_bin_file(c2dev->dev, &c2port_bin_attrs);
-	spin_lock_irq(&c2port_idr_lock);
 	idr_remove(&c2port_idr, c2dev->id);
-	spin_unlock_irq(&c2port_idr_lock);
 
 	device_destroy(c2port_class, c2dev->id);
 
diff --git a/drivers/misc/tifm_core.c b/drivers/misc/tifm_core.c
index c828c27..ee7b181 100644
--- a/drivers/misc/tifm_core.c
+++ b/drivers/misc/tifm_core.c
@@ -20,7 +20,6 @@
 
 static struct workqueue_struct *workqueue;
 static DEFINE_IDR(tifm_adapter_idr);
-static DEFINE_SPINLOCK(tifm_adapter_lock);
 
 static const char *tifm_media_type_name(unsigned char type, unsigned char nt)
 {
@@ -196,22 +195,16 @@ int tifm_add_adapter(struct tifm_adapter *fm)
 {
 	int rc;
 
-	idr_preload(GFP_KERNEL);
-	spin_lock(&tifm_adapter_lock);
-	rc = idr_alloc(&tifm_adapter_idr, fm, GFP_NOWAIT);
-	if (rc >= 0)
-		fm->id = rc;
-	spin_unlock(&tifm_adapter_lock);
-	idr_preload_end();
+	rc = idr_alloc(&tifm_adapter_idr, fm, GFP_KERNEL);
 	if (rc < 0)
 		return rc;
 
+	fm->id = rc;
+
 	dev_set_name(&fm->dev, "tifm%u", fm->id);
 	rc = device_add(&fm->dev);
 	if (rc) {
-		spin_lock(&tifm_adapter_lock);
 		idr_remove(&tifm_adapter_idr, fm->id);
-		spin_unlock(&tifm_adapter_lock);
 	}
 
 	return rc;
@@ -228,9 +221,7 @@ void tifm_remove_adapter(struct tifm_adapter *fm)
 			device_unregister(&fm->sockets[cnt]->dev);
 	}
 
-	spin_lock(&tifm_adapter_lock);
 	idr_remove(&tifm_adapter_idr, fm->id);
-	spin_unlock(&tifm_adapter_lock);
 	device_del(&fm->dev);
 }
 EXPORT_SYMBOL(tifm_remove_adapter);
diff --git a/drivers/mmc/core/host.c b/drivers/mmc/core/host.c
index 335873c..bf9c68f 100644
--- a/drivers/mmc/core/host.c
+++ b/drivers/mmc/core/host.c
@@ -55,7 +55,6 @@ void mmc_unregister_host_class(void)
 }
 
 static DEFINE_IDR(mmc_host_idr);
-static DEFINE_SPINLOCK(mmc_host_lock);
 
 #ifdef CONFIG_MMC_CLKGATE
 static ssize_t clkgate_delay_show(struct device *dev,
@@ -435,16 +434,11 @@ struct mmc_host *mmc_alloc_host(int extra, struct device *dev)
 
 	/* scanning will be enabled when we're ready */
 	host->rescan_disable = 1;
-	idr_preload(GFP_KERNEL);
-	spin_lock(&mmc_host_lock);
-	err = idr_alloc(&mmc_host_idr, host, GFP_NOWAIT);
-	if (err >= 0)
-		host->index = err;
-	spin_unlock(&mmc_host_lock);
-	idr_preload_end();
+	err = idr_alloc(&mmc_host_idr, host, GFP_KERNEL);
 	if (err < 0)
 		goto free;
 
+	host->index = err;
 	dev_set_name(&host->class_dev, "mmc%d", host->index);
 
 	host->parent = dev;
@@ -552,10 +546,7 @@ EXPORT_SYMBOL(mmc_remove_host);
  */
 void mmc_free_host(struct mmc_host *host)
 {
-	spin_lock(&mmc_host_lock);
 	idr_remove(&mmc_host_idr, host->index);
-	spin_unlock(&mmc_host_lock);
-
 	put_device(&host->class_dev);
 }
 
diff --git a/drivers/scsi/ch.c b/drivers/scsi/ch.c
index c264bed..0693879 100644
--- a/drivers/scsi/ch.c
+++ b/drivers/scsi/ch.c
@@ -115,7 +115,6 @@ typedef struct {
 } scsi_changer;
 
 static DEFINE_IDR(ch_index_idr);
-static DEFINE_SPINLOCK(ch_index_lock);
 
 static const struct {
 	unsigned char  sense;
@@ -582,15 +581,12 @@ ch_open(struct inode *inode, struct file *file)
 	int minor = iminor(inode);
 
 	mutex_lock(&ch_mutex);
-	spin_lock(&ch_index_lock);
 	ch = idr_find(&ch_index_idr, minor);
 
 	if (NULL == ch || scsi_device_get(ch->device)) {
-		spin_unlock(&ch_index_lock);
 		mutex_unlock(&ch_mutex);
 		return -ENXIO;
 	}
-	spin_unlock(&ch_index_lock);
 
 	file->private_data = ch;
 	mutex_unlock(&ch_mutex);
@@ -905,12 +901,8 @@ static int ch_probe(struct device *dev)
 	if (NULL == ch)
 		return -ENOMEM;
 
-	idr_preload(GFP_KERNEL);
-	spin_lock(&ch_index_lock);
-	ret = idr_alloc_range(&ch_index_idr, ch, 0, CH_MAX_DEVS + 1, GFP_NOWAIT);
-	spin_unlock(&ch_index_lock);
-	idr_preload_end();
-
+	ret = idr_alloc_range(&ch_index_idr, ch, 0,
+			      CH_MAX_DEVS + 1, GFP_KERNEL);
 	if (ret < 0) {
 		if (ret == -ENOSPC)
 			ret = -ENODEV;
@@ -951,9 +943,7 @@ static int ch_remove(struct device *dev)
 {
 	scsi_changer *ch = dev_get_drvdata(dev);
 
-	spin_lock(&ch_index_lock);
 	idr_remove(&ch_index_idr, ch->minor);
-	spin_unlock(&ch_index_lock);
 
 	device_destroy(ch_sysfs_class, MKDEV(SCSI_CHANGER_MAJOR,ch->minor));
 	kfree(ch->dt);
diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
index 743fd72..2f13bf8 100644
--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -214,7 +214,6 @@ static void scsi_tape_release(struct kref *);
 #define to_scsi_tape(obj) container_of(obj, struct scsi_tape, kref)
 
 static DEFINE_MUTEX(st_ref_mutex);
-static DEFINE_SPINLOCK(st_index_lock);
 static DEFINE_SPINLOCK(st_use_lock);
 static DEFINE_IDR(st_index_idr);
 
@@ -235,7 +234,6 @@ static struct scsi_tape *scsi_tape_get(int dev)
 	struct scsi_tape *STp = NULL;
 
 	mutex_lock(&st_ref_mutex);
-	spin_lock(&st_index_lock);
 
 	STp = idr_find(&st_index_idr, dev);
 	if (!STp) goto out;
@@ -254,7 +252,6 @@ out_put:
 	kref_put(&STp->kref, scsi_tape_release);
 	STp = NULL;
 out:
-	spin_unlock(&st_index_lock);
 	mutex_unlock(&st_ref_mutex);
 	return STp;
 }
@@ -4182,11 +4179,7 @@ static int st_probe(struct device *dev)
 	    tpnt->blksize_changed = 0;
 	mutex_init(&tpnt->lock);
 
-	idr_preload(GFP_KERNEL);
-	spin_lock(&st_index_lock);
-	error = idr_alloc_range(&st_index_idr, tpnt, 0, ST_MAX_TAPES + 1, GFP_NOWAIT);
-	spin_unlock(&st_index_lock);
-	idr_preload_end();
+	error = idr_alloc_range(&st_index_idr, tpnt, 0, ST_MAX_TAPES + 1, GFP_KERNEL);
 	if (error < 0) {
 		pr_warn("st: idr allocation failed: %d\n", error);
 		goto out_put_queue;
@@ -4212,9 +4205,7 @@ static int st_probe(struct device *dev)
 
 out_remove_devs:
 	remove_cdevs(tpnt);
-	spin_lock(&st_index_lock);
 	idr_remove(&st_index_idr, tpnt->index);
-	spin_unlock(&st_index_lock);
 out_put_queue:
 	blk_put_queue(disk->queue);
 out_put_disk:
@@ -4238,9 +4229,7 @@ static int st_remove(struct device *dev)
 	mutex_lock(&st_ref_mutex);
 	kref_put(&tpnt->kref, scsi_tape_release);
 	mutex_unlock(&st_ref_mutex);
-	spin_lock(&st_index_lock);
 	idr_remove(&st_index_idr, index);
-	spin_unlock(&st_index_lock);
 	return 0;
 }
 
diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
index b06d2a8..a4777a4 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -59,7 +59,6 @@ static DEFINE_SPINLOCK(np_lock);
 static struct idr tiqn_idr;
 struct idr sess_idr;
 struct mutex auth_id_lock;
-spinlock_t sess_idr_lock;
 
 struct iscsit_global *iscsit_global;
 
@@ -147,22 +146,17 @@ struct iscsi_tiqn *iscsit_add_tiqn(unsigned char *buf)
 
 	tiqn->tiqn_state = TIQN_STATE_ACTIVE;
 
-	idr_preload(GFP_KERNEL);
-	spin_lock(&tiqn_lock);
-
-	ret = idr_alloc(&tiqn_idr, NULL, GFP_NOWAIT);
+	ret = idr_alloc(&tiqn_idr, NULL, GFP_KERNEL);
 	if (ret < 0) {
 		pr_err("idr_alloc() failed for tiqn->tiqn_index\n");
-		spin_unlock(&tiqn_lock);
-		idr_preload_end();
 		kfree(tiqn);
 		return ERR_PTR(ret);
 	}
 	tiqn->tiqn_index = ret;
-	list_add_tail(&tiqn->tiqn_list, &g_tiqn_list);
 
+	spin_lock(&tiqn_lock);
+	list_add_tail(&tiqn->tiqn_list, &g_tiqn_list);
 	spin_unlock(&tiqn_lock);
-	idr_preload_end();
 
 	pr_debug("CORE[0] - Added iSCSI Target IQN: %s\n", tiqn->tiqn);
 
@@ -201,8 +195,8 @@ void iscsit_del_tiqn(struct iscsi_tiqn *tiqn)
 
 	spin_lock(&tiqn_lock);
 	list_del(&tiqn->tiqn_list);
-	idr_remove(&tiqn_idr, tiqn->tiqn_index);
 	spin_unlock(&tiqn_lock);
+	idr_remove(&tiqn_idr, tiqn->tiqn_index);
 
 	pr_debug("CORE[0] - Deleted iSCSI Target IQN: %s\n",
 			tiqn->tiqn);
@@ -519,7 +513,6 @@ static int __init iscsi_target_init_module(void)
 		return -1;
 	}
 	mutex_init(&auth_id_lock);
-	spin_lock_init(&sess_idr_lock);
 	idr_init(&tiqn_idr);
 	idr_init(&sess_idr);
 
@@ -4417,9 +4410,7 @@ int iscsit_close_session(struct iscsi_session *sess)
 	pr_debug("Decremented number of active iSCSI Sessions on"
 		" iSCSI TPG: %hu to %u\n", tpg->tpgt, tpg->nsessions);
 
-	spin_lock(&sess_idr_lock);
 	idr_remove(&sess_idr, sess->session_index);
-	spin_unlock(&sess_idr_lock);
 
 	kfree(sess->sess_ops);
 	sess->sess_ops = NULL;
diff --git a/drivers/target/iscsi/iscsi_target.h b/drivers/target/iscsi/iscsi_target.h
index a0050b2..a47d7132 100644
--- a/drivers/target/iscsi/iscsi_target.h
+++ b/drivers/target/iscsi/iscsi_target.h
@@ -43,7 +43,6 @@ extern struct kmem_cache *lio_r2t_cache;
 
 extern struct idr sess_idr;
 extern struct mutex auth_id_lock;
-extern spinlock_t sess_idr_lock;
 
 
 #endif   /*** ISCSI_TARGET_H ***/
diff --git a/drivers/target/iscsi/iscsi_target_login.c b/drivers/target/iscsi/iscsi_target_login.c
index 87a82b6..a0b9510 100644
--- a/drivers/target/iscsi/iscsi_target_login.c
+++ b/drivers/target/iscsi/iscsi_target_login.c
@@ -289,14 +289,7 @@ static int iscsi_login_zero_tsih_s1(
 	spin_lock_init(&sess->session_usage_lock);
 	spin_lock_init(&sess->ttt_lock);
 
-	idr_preload(GFP_KERNEL);
-	spin_lock_bh(&sess_idr_lock);
-	ret = idr_alloc(&sess_idr, NULL, GFP_NOWAIT);
-	if (ret >= 0)
-		sess->session_index = ret;
-	spin_unlock_bh(&sess_idr_lock);
-	idr_preload_end();
-
+	ret = idr_alloc(&sess_idr, NULL, GFP_KERNEL);
 	if (ret < 0) {
 		pr_err("idr_alloc() for sess_idr failed\n");
 		iscsit_tx_login_rsp(conn, ISCSI_STATUS_CLS_TARGET_ERR,
@@ -305,6 +298,7 @@ static int iscsi_login_zero_tsih_s1(
 		return -ENOMEM;
 	}
 
+	sess->session_index = ret;
 	sess->creation_time = get_jiffies_64();
 	spin_lock_init(&sess->session_stats_lock);
 	/*
@@ -1315,9 +1309,7 @@ new_sess_out:
 	if (conn->sess->se_sess)
 		transport_free_session(conn->sess->se_sess);
 	if (conn->sess->session_index != 0) {
-		spin_lock_bh(&sess_idr_lock);
 		idr_remove(&sess_idr, conn->sess->session_index);
-		spin_unlock_bh(&sess_idr_lock);
 	}
 	kfree(conn->sess->sess_ops);
 	kfree(conn->sess);
diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index 8bda129..0a5d8ad 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -621,7 +621,6 @@ struct cgroup_subsys {
 	struct list_head sibling;
 	/* used when use_id == true */
 	struct idr idr;
-	spinlock_t id_lock;
 
 	/* list of cftype_sets */
 	struct list_head cftsets;
diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
index cd89510..7f8ce79 100644
--- a/include/net/sctp/sctp.h
+++ b/include/net/sctp/sctp.h
@@ -603,7 +603,6 @@ extern struct proto sctpv6_prot;
 void sctp_put_port(struct sock *sk);
 
 extern struct idr sctp_assocs_id;
-extern spinlock_t sctp_assocs_id_lock;
 
 /* Static inline functions. */
 
diff --git a/kernel/cgroup.c b/kernel/cgroup.c
index 54a591bb..998f6de 100644
--- a/kernel/cgroup.c
+++ b/kernel/cgroup.c
@@ -5143,9 +5143,7 @@ void free_css_id(struct cgroup_subsys *ss, struct cgroup_subsys_state *css)
 
 	rcu_assign_pointer(id->css, NULL);
 	rcu_assign_pointer(css->id, NULL);
-	spin_lock(&ss->id_lock);
 	idr_remove(&ss->idr, id->id);
-	spin_unlock(&ss->id_lock);
 	kfree_rcu(id, rcu_head);
 }
 EXPORT_SYMBOL_GPL(free_css_id);
@@ -5167,12 +5165,8 @@ static struct css_id *get_new_cssid(struct cgroup_subsys *ss, int depth)
 	if (!newid)
 		return ERR_PTR(-ENOMEM);
 
-	idr_preload(GFP_KERNEL);
-	spin_lock(&ss->id_lock);
 	/* Don't use 0. allocates an ID of 1-65535 */
-	ret = idr_alloc_range(&ss->idr, newid, 1, CSS_ID_MAX + 1, GFP_NOWAIT);
-	spin_unlock(&ss->id_lock);
-	idr_preload_end();
+	ret = idr_alloc_range(&ss->idr, newid, 1, CSS_ID_MAX + 1, GFP_KERNEL);
 
 	/* Returns error when there are no free spaces for new ID.*/
 	if (ret < 0)
@@ -5192,7 +5186,6 @@ static int __init_or_module cgroup_init_idr(struct cgroup_subsys *ss,
 {
 	struct css_id *newid;
 
-	spin_lock_init(&ss->id_lock);
 	idr_init(&ss->idr);
 
 	newid = get_new_cssid(ss, 0);
diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index b45694f..be5852c 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -1690,13 +1690,7 @@ static struct worker *create_worker(struct worker_pool *pool)
 	 * ID is needed to determine kthread name.  Allocate ID first
 	 * without installing the pointer.
 	 */
-	idr_preload(GFP_KERNEL);
-	spin_lock_irq(&pool->lock);
-
-	id = idr_alloc(&pool->worker_idr, NULL, GFP_NOWAIT);
-
-	spin_unlock_irq(&pool->lock);
-	idr_preload_end();
+	id = idr_alloc(&pool->worker_idr, NULL, GFP_KERNEL);
 	if (id < 0)
 		goto fail;
 
@@ -1737,18 +1731,13 @@ static struct worker *create_worker(struct worker_pool *pool)
 		worker->flags |= WORKER_UNBOUND;
 
 	/* successful, commit the pointer to idr */
-	spin_lock_irq(&pool->lock);
 	idr_replace(&pool->worker_idr, worker, worker->id);
-	spin_unlock_irq(&pool->lock);
 
 	return worker;
 
 fail:
-	if (id >= 0) {
-		spin_lock_irq(&pool->lock);
+	if (id >= 0)
 		idr_remove(&pool->worker_idr, id);
-		spin_unlock_irq(&pool->lock);
-	}
 	kfree(worker);
 	return NULL;
 }
diff --git a/net/9p/util.c b/net/9p/util.c
index cc31c62..23693d6 100644
--- a/net/9p/util.c
+++ b/net/9p/util.c
@@ -41,7 +41,6 @@
  */
 
 struct p9_idpool {
-	spinlock_t lock;
 	struct idr pool;
 };
 
@@ -58,7 +57,6 @@ struct p9_idpool *p9_idpool_create(void)
 	if (!p)
 		return ERR_PTR(-ENOMEM);
 
-	spin_lock_init(&p->lock);
 	idr_init(&p->pool);
 
 	return p;
@@ -88,16 +86,9 @@ EXPORT_SYMBOL(p9_idpool_destroy);
 int p9_idpool_get(struct p9_idpool *p)
 {
 	int i;
-	unsigned long flags;
-
-	idr_preload(GFP_NOFS);
-	spin_lock_irqsave(&p->lock, flags);
 
 	/* no need to store exactly p, we just need something non-null */
-	i = idr_alloc(&p->pool, p, GFP_NOWAIT);
-
-	spin_unlock_irqrestore(&p->lock, flags);
-	idr_preload_end();
+	i = idr_alloc(&p->pool, p, GFP_NOFS);
 	if (i < 0)
 		return -1;
 
@@ -117,13 +108,9 @@ EXPORT_SYMBOL(p9_idpool_get);
 
 void p9_idpool_put(int id, struct p9_idpool *p)
 {
-	unsigned long flags;
-
 	p9_debug(P9_DEBUG_MUX, " id %d pool %p\n", id, p);
 
-	spin_lock_irqsave(&p->lock, flags);
 	idr_remove(&p->pool, id);
-	spin_unlock_irqrestore(&p->lock, flags);
 }
 EXPORT_SYMBOL(p9_idpool_put);
 
diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index 91cfd8f..d2739e0 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -467,11 +467,8 @@ static void sctp_association_destroy(struct sctp_association *asoc)
 	sctp_endpoint_put(asoc->ep);
 	sock_put(asoc->base.sk);
 
-	if (asoc->assoc_id != 0) {
-		spin_lock_bh(&sctp_assocs_id_lock);
+	if (asoc->assoc_id != 0)
 		idr_remove(&sctp_assocs_id, asoc->assoc_id);
-		spin_unlock_bh(&sctp_assocs_id_lock);
-	}
 
 	WARN_ON(atomic_read(&asoc->rmem_alloc));
 
@@ -1580,21 +1577,14 @@ int sctp_assoc_lookup_laddr(struct sctp_association *asoc,
 /* Set an association id for a given association */
 int sctp_assoc_set_id(struct sctp_association *asoc, gfp_t gfp)
 {
-	bool preload = gfp & __GFP_WAIT;
 	int ret;
 
 	/* If the id is already assigned, keep it. */
 	if (asoc->assoc_id)
 		return 0;
 
-	if (preload)
-		idr_preload(gfp);
-	spin_lock_bh(&sctp_assocs_id_lock);
 	/* 0 is not a valid assoc_id, must be >= 1 */
-	ret = idr_alloc_cyclic(&sctp_assocs_id, asoc, 1, 0, GFP_NOWAIT);
-	spin_unlock_bh(&sctp_assocs_id_lock);
-	if (preload)
-		idr_preload_end();
+	ret = idr_alloc_cyclic(&sctp_assocs_id, asoc, 1, 0, gfp);
 	if (ret < 0)
 		return ret;
 
diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
index eaee00c..4bfe2e8 100644
--- a/net/sctp/protocol.c
+++ b/net/sctp/protocol.c
@@ -71,7 +71,6 @@
 struct sctp_globals sctp_globals __read_mostly;
 
 struct idr sctp_assocs_id;
-DEFINE_SPINLOCK(sctp_assocs_id_lock);
 
 static struct sctp_pf *sctp_pf_inet6_specific;
 static struct sctp_pf *sctp_pf_inet_specific;
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 6abb1ca..3fe9ab4 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -230,9 +230,7 @@ struct sctp_association *sctp_id2assoc(struct sock *sk, sctp_assoc_t id)
 	if (!id || (id == (sctp_assoc_t)-1))
 		return NULL;
 
-	spin_lock_bh(&sctp_assocs_id_lock);
 	asoc = (struct sctp_association *)idr_find(&sctp_assocs_id, (int)id);
-	spin_unlock_bh(&sctp_assocs_id_lock);
 
 	if (!asoc || (asoc->base.sk != sk) || asoc->base.dead)
 		return NULL;
-- 
1.8.3.1

_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/containers

[PATCH 10/10] idr: Rework idr_preload()

[Kent Overstreet]

The old idr_preload() used percpu buffers - since the
bitmap/radix/whatever tree only grew by fixed sized nodes, it only had
to ensure there was a node available in the percpu buffer and disable
preemption. This conveniently meant that you didn't have to pass the idr
you were going to allocate from to it.

With the new ida implementation, that doesn't work anymore - the new ida
code grows its bitmap tree by reallocating the entire thing in power of
two increments. Doh.

So we need a slightly different trick. Note that if all allocations from
an idr start by calling idr_prealloc() and disabling premption, at
most nr_cpus() allocations can happen before someone calls
idr_prealloc() again.

So, we just change idr_prealloc() to resize the ida bitmap tree if
there's less than num_possible_cpus() ids available - conveniently, we
already track the number of allocated ids, and the total number of ids
we can have allocated is just nr_leaf_nodes * BITS_PER_LONG. Easy.

This does require a fairly trivial interface change - we now have to
pass the idr we're going to allocate from (and the starting id we're
going to pass to idr_allocate_range()) to idr_prealloc(), so this patch
updates all the callers.

Signed-off-by: Kent Overstreet <koverstreet@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Tejun Heo <tj@kernel.org>
Cc: Stefan Richter <stefanr@s5r6.in-berlin.de>
Cc: David Airlie <airlied@linux.ie>
Cc: Roland Dreier <roland@kernel.org>
Cc: Sean Hefty <sean.hefty@intel.com>
Cc: Hal Rosenstock <hal.rosenstock@gmail.com>
Cc: Steve Wise <swise@chelsio.com>
Cc: Hoang-Nam Nguyen <hnguyen@de.ibm.com>
Cc: Christoph Raisch <raisch@de.ibm.com>
Cc: Mike Marciniszyn <infinipath@intel.com>
Cc: Doug Gilbert <dgilbert@interlog.com>
Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
Cc: Christine Caulfield <ccaulfie@redhat.com>
Cc: David Teigland <teigland@redhat.com>
Cc: Trond Myklebust <Trond.Myklebust@netapp.com>
Cc: John McCutchan <john@johnmccutchan.com>
Cc: Robert Love <rlove@rlove.org>
Cc: Eric Paris <eparis@parisplace.org>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Thomas Hellstrom <thellstrom@vmware.com>
Cc: Brian Paul <brianp@vmware.com>
Cc: Maarten Lankhorst <maarten.lankhorst@canonical.com>
Cc: Dmitry Torokhov <dtor@vmware.com>
Cc: Erez Shitrit <erezsh@mellanox.co.il>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Haggai Eran <haggaie@mellanox.com>
Cc: Jack Morgenstein <jackm@dev.mellanox.co.il>
Cc: Wolfram Sang <wolfram@the-dreams.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Davidlohr Bueso <davidlohr.bueso@hp.com>
Cc: Rik van Riel <riel@redhat.com>
Cc: Michel Lespinasse <walken@google.com>
Cc: linux1394-devel@lists.sourceforge.net
Cc: linux-kernel@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: linux-rdma@vger.kernel.org
Cc: linux-scsi@vger.kernel.org
Cc: cluster-devel@redhat.com
Cc: linux-nfs@vger.kernel.org
---
 drivers/firewire/core-cdev.c               |  2 +-
 drivers/gpu/drm/drm_gem.c                  |  4 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_resource.c   |  2 +-
 drivers/infiniband/core/cm.c               |  7 +---
 drivers/infiniband/core/sa_query.c         |  2 +-
 drivers/infiniband/core/uverbs_cmd.c       |  2 +-
 drivers/infiniband/hw/cxgb3/iwch.h         |  2 +-
 drivers/infiniband/hw/cxgb4/iw_cxgb4.h     |  2 +-
 drivers/infiniband/hw/ehca/ehca_cq.c       |  2 +-
 drivers/infiniband/hw/ehca/ehca_qp.c       |  2 +-
 drivers/infiniband/hw/ipath/ipath_driver.c |  2 +-
 drivers/infiniband/hw/mlx4/cm.c            |  2 +-
 drivers/infiniband/hw/qib/qib_init.c       |  2 +-
 drivers/scsi/sg.c                          |  2 +-
 fs/dlm/lock.c                              |  2 +-
 fs/dlm/recover.c                           |  2 +-
 fs/nfs/nfs4client.c                        |  2 +-
 fs/notify/inotify/inotify_user.c           |  2 +-
 include/linux/idr.h                        | 37 +----------------
 ipc/util.c                                 |  4 +-
 lib/idr.c                                  | 66 ++++++++++++++++++++++++++++++
 21 files changed, 91 insertions(+), 59 deletions(-)

diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
index 436debf..3c70fbc 100644
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -490,7 +490,7 @@ static int add_client_resource(struct client *client,
 	int ret;
 
 	if (preload)
-		idr_preload(gfp_mask);
+		idr_preload(&client->resource_idr, 0, gfp_mask);
 	spin_lock_irqsave(&client->lock, flags);
 
 	if (client->in_shutdown)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 1c897b9..cf50894 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -273,7 +273,7 @@ drm_gem_handle_create(struct drm_file *file_priv,
 	 * Get the user-visible handle using idr.  Preload and perform
 	 * allocation under our spinlock.
 	 */
-	idr_preload(GFP_KERNEL);
+	idr_preload(&file_priv->object_idr, 1, GFP_KERNEL);
 	spin_lock(&file_priv->table_lock);
 
 	ret = idr_alloc_range(&file_priv->object_idr, obj, 1, 0, GFP_NOWAIT);
@@ -449,7 +449,7 @@ drm_gem_flink_ioctl(struct drm_device *dev, void *data,
 	if (obj == NULL)
 		return -ENOENT;
 
-	idr_preload(GFP_KERNEL);
+	idr_preload(&dev->object_name_idr, 1, GFP_KERNEL);
 	spin_lock(&dev->object_name_lock);
 	if (!obj->name) {
 		ret = idr_alloc_range(&dev->object_name_idr, obj, 1, 0, GFP_NOWAIT);
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
index ccbaed1..9f00706 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
@@ -177,7 +177,7 @@ int vmw_resource_alloc_id(struct vmw_resource *res)
 
 	BUG_ON(res->id != -1);
 
-	idr_preload(GFP_KERNEL);
+	idr_preload(idr, 1, GFP_KERNEL);
 	write_lock(&dev_priv->resource_lock);
 
 	ret = idr_alloc_range(idr, res, 1, 0, GFP_NOWAIT);
diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c
index 86008a9..a11bb5e 100644
--- a/drivers/infiniband/core/cm.c
+++ b/drivers/infiniband/core/cm.c
@@ -383,14 +383,11 @@ static int cm_alloc_id(struct cm_id_private *cm_id_priv)
 {
 	unsigned long flags;
 	int id;
-	static int next_id;
 
-	idr_preload(GFP_KERNEL);
+	idr_preload(&cm.local_id_table, 0, GFP_KERNEL);
 	spin_lock_irqsave(&cm.lock, flags);
 
-	id = idr_alloc_range(&cm.local_id_table, cm_id_priv, next_id, 0, GFP_NOWAIT);
-	if (id >= 0)
-		next_id = max(id + 1, 0);
+	id = idr_alloc_cyclic(&cm.local_id_table, cm_id_priv, 0, 0, GFP_NOWAIT);
 
 	spin_unlock_irqrestore(&cm.lock, flags);
 	idr_preload_end();
diff --git a/drivers/infiniband/core/sa_query.c b/drivers/infiniband/core/sa_query.c
index 509d5a6..9fc181f 100644
--- a/drivers/infiniband/core/sa_query.c
+++ b/drivers/infiniband/core/sa_query.c
@@ -616,7 +616,7 @@ static int send_mad(struct ib_sa_query *query, int timeout_ms, gfp_t gfp_mask)
 	int ret, id;
 
 	if (preload)
-		idr_preload(gfp_mask);
+		idr_preload(&query_idr, 0, gfp_mask);
 	spin_lock_irqsave(&idr_lock, flags);
 
 	id = idr_alloc(&query_idr, query, GFP_NOWAIT);
diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index 775431a..b1dfb30 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -125,7 +125,7 @@ static int idr_add_uobj(struct idr *idr, struct ib_uobject *uobj)
 {
 	int ret;
 
-	idr_preload(GFP_KERNEL);
+	idr_preload(idr, 0, GFP_KERNEL);
 	spin_lock(&ib_uverbs_idr_lock);
 
 	ret = idr_alloc(idr, uobj, GFP_NOWAIT);
diff --git a/drivers/infiniband/hw/cxgb3/iwch.h b/drivers/infiniband/hw/cxgb3/iwch.h
index f28c585..12e5f29 100644
--- a/drivers/infiniband/hw/cxgb3/iwch.h
+++ b/drivers/infiniband/hw/cxgb3/iwch.h
@@ -154,7 +154,7 @@ static inline int insert_handle(struct iwch_dev *rhp, struct idr *idr,
 {
 	int ret;
 
-	idr_preload(GFP_KERNEL);
+	idr_preload(idr, id, GFP_KERNEL);
 	spin_lock_irq(&rhp->lock);
 
 	ret = idr_alloc_range(idr, handle, id, id + 1, GFP_NOWAIT);
diff --git a/drivers/infiniband/hw/cxgb4/iw_cxgb4.h b/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
index 50e5a3f..e6a5fc3 100644
--- a/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
+++ b/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
@@ -262,7 +262,7 @@ static inline int _insert_handle(struct c4iw_dev *rhp, struct idr *idr,
 	int ret;
 
 	if (lock) {
-		idr_preload(GFP_KERNEL);
+		idr_preload(idr, id, GFP_KERNEL);
 		spin_lock_irq(&rhp->lock);
 	}
 
diff --git a/drivers/infiniband/hw/ehca/ehca_cq.c b/drivers/infiniband/hw/ehca/ehca_cq.c
index 0bc5c51..89c02e4 100644
--- a/drivers/infiniband/hw/ehca/ehca_cq.c
+++ b/drivers/infiniband/hw/ehca/ehca_cq.c
@@ -163,7 +163,7 @@ struct ib_cq *ehca_create_cq(struct ib_device *device, int cqe, int comp_vector,
 	adapter_handle = shca->ipz_hca_handle;
 	param.eq_handle = shca->eq.ipz_eq_handle;
 
-	idr_preload(GFP_KERNEL);
+	idr_preload(&ehca_cq_idr, 0, GFP_KERNEL);
 	write_lock_irqsave(&ehca_cq_idr_lock, flags);
 	my_cq->token = idr_alloc_range(&ehca_cq_idr, my_cq, 0, 0x2000000, GFP_NOWAIT);
 	write_unlock_irqrestore(&ehca_cq_idr_lock, flags);
diff --git a/drivers/infiniband/hw/ehca/ehca_qp.c b/drivers/infiniband/hw/ehca/ehca_qp.c
index 758a265..4184133 100644
--- a/drivers/infiniband/hw/ehca/ehca_qp.c
+++ b/drivers/infiniband/hw/ehca/ehca_qp.c
@@ -636,7 +636,7 @@ static struct ehca_qp *internal_create_qp(
 		my_qp->send_cq =
 			container_of(init_attr->send_cq, struct ehca_cq, ib_cq);
 
-	idr_preload(GFP_KERNEL);
+	idr_preload(&ehca_qp_idr, 0, GFP_KERNEL);
 	write_lock_irqsave(&ehca_qp_idr_lock, flags);
 
 	ret = idr_alloc_range(&ehca_qp_idr, my_qp, 0, 0x2000000, GFP_NOWAIT);
diff --git a/drivers/infiniband/hw/ipath/ipath_driver.c b/drivers/infiniband/hw/ipath/ipath_driver.c
index 83a40a5..b241f42 100644
--- a/drivers/infiniband/hw/ipath/ipath_driver.c
+++ b/drivers/infiniband/hw/ipath/ipath_driver.c
@@ -201,7 +201,7 @@ static struct ipath_devdata *ipath_alloc_devdata(struct pci_dev *pdev)
 	}
 	dd->ipath_unit = -1;
 
-	idr_preload(GFP_KERNEL);
+	idr_preload(&unit_table, 0, GFP_KERNEL);
 	spin_lock_irqsave(&ipath_devs_lock, flags);
 
 	ret = idr_alloc(&unit_table, dd, GFP_NOWAIT);
diff --git a/drivers/infiniband/hw/mlx4/cm.c b/drivers/infiniband/hw/mlx4/cm.c
index d1f5f1d..ac089e6 100644
--- a/drivers/infiniband/hw/mlx4/cm.c
+++ b/drivers/infiniband/hw/mlx4/cm.c
@@ -219,7 +219,7 @@ id_map_alloc(struct ib_device *ibdev, int slave_id, u32 sl_cm_id)
 	ent->dev = to_mdev(ibdev);
 	INIT_DELAYED_WORK(&ent->timeout, id_map_ent_timeout);
 
-	idr_preload(GFP_KERNEL);
+	idr_preload(&sriov->pv_id_table, 0, GFP_KERNEL);
 	spin_lock(&to_mdev(ibdev)->sriov.id_map_lock);
 
 	ret = idr_alloc_cyclic(&sriov->pv_id_table, ent, 0, 0, GFP_NOWAIT);
diff --git a/drivers/infiniband/hw/qib/qib_init.c b/drivers/infiniband/hw/qib/qib_init.c
index 503619c..08d9703 100644
--- a/drivers/infiniband/hw/qib/qib_init.c
+++ b/drivers/infiniband/hw/qib/qib_init.c
@@ -1066,7 +1066,7 @@ struct qib_devdata *qib_alloc_devdata(struct pci_dev *pdev, size_t extra)
 		goto bail;
 	}
 
-	idr_preload(GFP_KERNEL);
+	idr_preload(&qib_unit_table, 0, GFP_KERNEL);
 	spin_lock_irqsave(&qib_devs_lock, flags);
 
 	ret = idr_alloc(&qib_unit_table, dd, GFP_NOWAIT);
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 23856c8..d226a64 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -1392,7 +1392,7 @@ static Sg_device *sg_alloc(struct gendisk *disk, struct scsi_device *scsidp)
 		return ERR_PTR(-ENOMEM);
 	}
 
-	idr_preload(GFP_KERNEL);
+	idr_preload(&sg_index_idr, 0, GFP_KERNEL);
 	write_lock_irqsave(&sg_index_lock, iflags);
 
 	error = idr_alloc_range(&sg_index_idr, sdp, 0, SG_MAX_DEVS, GFP_NOWAIT);
diff --git a/fs/dlm/lock.c b/fs/dlm/lock.c
index 85bba95..7dd15dd 100644
--- a/fs/dlm/lock.c
+++ b/fs/dlm/lock.c
@@ -1199,7 +1199,7 @@ static int create_lkb(struct dlm_ls *ls, struct dlm_lkb **lkb_ret)
 	mutex_init(&lkb->lkb_cb_mutex);
 	INIT_WORK(&lkb->lkb_cb_work, dlm_callback_work);
 
-	idr_preload(GFP_NOFS);
+	idr_preload(&ls->ls_lkbidr, 1, GFP_NOFS);
 	spin_lock(&ls->ls_lkbidr_spin);
 	rv = idr_alloc_range(&ls->ls_lkbidr, lkb, 1, 0, GFP_NOWAIT);
 	if (rv >= 0)
diff --git a/fs/dlm/recover.c b/fs/dlm/recover.c
index 2babe5e..757b7a6 100644
--- a/fs/dlm/recover.c
+++ b/fs/dlm/recover.c
@@ -307,7 +307,7 @@ static int recover_idr_add(struct dlm_rsb *r)
 	struct dlm_ls *ls = r->res_ls;
 	int rv;
 
-	idr_preload(GFP_NOFS);
+	idr_preload(&ls->ls_recover_idr, 1, GFP_NOFS);
 	spin_lock(&ls->ls_recover_idr_lock);
 	if (r->res_id) {
 		rv = -1;
diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
index 786aac37..85c904e 100644
--- a/fs/nfs/nfs4client.c
+++ b/fs/nfs/nfs4client.c
@@ -30,7 +30,7 @@ static int nfs_get_cb_ident_idr(struct nfs_client *clp, int minorversion)
 
 	if (clp->rpc_ops->version != 4 || minorversion != 0)
 		return ret;
-	idr_preload(GFP_KERNEL);
+	idr_preload(&nn->cb_ident_idr, 0, GFP_KERNEL);
 	spin_lock(&nn->nfs_client_lock);
 	ret = idr_alloc(&nn->cb_ident_idr, clp, GFP_NOWAIT);
 	if (ret >= 0)
diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c
index 959815c..04302e8 100644
--- a/fs/notify/inotify/inotify_user.c
+++ b/fs/notify/inotify/inotify_user.c
@@ -360,7 +360,7 @@ static int inotify_add_to_idr(struct idr *idr, spinlock_t *idr_lock,
 {
 	int ret;
 
-	idr_preload(GFP_KERNEL);
+	idr_preload(idr, 1, GFP_KERNEL);
 	spin_lock(idr_lock);
 
 	ret = idr_alloc_cyclic(idr, i_mark, 1, 0, GFP_NOWAIT);
diff --git a/include/linux/idr.h b/include/linux/idr.h
index ec789f5..6234ba8 100644
--- a/include/linux/idr.h
+++ b/include/linux/idr.h
@@ -175,6 +175,7 @@ int idr_for_each(struct idr *idr,
 		 int (*fn)(int id, void *p, void *data), void *data);
 void *idr_replace(struct idr *idr, void *ptr, unsigned id);
 void idr_remove(struct idr *idr, unsigned id);
+int idr_preload(struct idr *idr, unsigned start, gfp_t gfp);
 int idr_alloc_range(struct idr *idr, void *ptr, unsigned start,
 		    unsigned end, gfp_t gfp);
 int idr_alloc_cyclic(struct idr *idr, void *ptr, unsigned start,
@@ -195,41 +196,7 @@ static inline int idr_alloc(struct idr *idr, void *ptr, gfp_t gfp)
  */
 static inline void idr_preload_end(void)
 {
-	radix_tree_preload_end();
-}
-
-/**
- * idr_preload - preload for idr_alloc_range()
- * @gfp: allocation mask to use for preloading
- *
- * Preload per-cpu layer buffer for idr_alloc_range().  Can only be used from
- * process context and each idr_preload() invocation should be matched with
- * idr_preload_end().  Note that preemption is disabled while preloaded.
- *
- * The first idr_alloc_range() in the preloaded section can be treated as if it
- * were invoked with @gfp_mask used for preloading.  This allows using more
- * permissive allocation masks for idrs protected by spinlocks.
- *
- * For example, if idr_alloc_range() below fails, the failure can be treated as
- * if idr_alloc_range() were called with GFP_KERNEL rather than GFP_NOWAIT.
- *
- *	idr_preload(GFP_KERNEL);
- *	spin_lock(lock);
- *
- *	id = idr_alloc_range(idr, ptr, start, end, GFP_NOWAIT);
- *
- *	spin_unlock(lock);
- *	idr_preload_end();
- *	if (id < 0)
- *		error;
- */
-static inline void idr_preload(gfp_t gfp)
-{
-	might_sleep_if(gfp & __GFP_WAIT);
-
-	/* Well this is horrible, but idr_preload doesn't return errors */
-	if (radix_tree_preload(gfp))
-		preempt_disable();
+	preempt_enable();
 }
 
 /* radix tree can't store NULL pointers, so we have to translate...  */
diff --git a/ipc/util.c b/ipc/util.c
index 749511d..5988e6b 100644
--- a/ipc/util.c
+++ b/ipc/util.c
@@ -262,7 +262,9 @@ int ipc_addid(struct ipc_ids* ids, struct kern_ipc_perm* new, int size)
 	if (ids->in_use >= size)
 		return -ENOSPC;
 
-	idr_preload(GFP_KERNEL);
+	idr_preload(&ids->ipcs_idr,
+		    (next_id < 0) ? 0 : ipcid_to_idx(next_id),
+		    GFP_KERNEL);
 
 	spin_lock_init(&new->lock);
 	new->deleted = 0;
diff --git a/lib/idr.c b/lib/idr.c
index c2fb8bc..2f04743 100644
--- a/lib/idr.c
+++ b/lib/idr.c
@@ -230,6 +230,23 @@ static inline int __ida_resize(struct ida *ida, unsigned max_id,
 	return 0;
 }
 
+static int ida_preload(struct ida *ida, unsigned start, gfp_t gfp)
+{
+	int ret = 0;
+	unsigned long flags;
+
+	spin_lock_irqsave(&ida->lock, flags);
+
+	while (!ret &&
+	       (ida->nodes - ida->first_leaf * BITS_PER_LONG <
+		start + ida->allocated_ids + num_possible_cpus()))
+		ret = __ida_resize(ida, (unsigned) INT_MAX + 1, gfp, &flags);
+
+	spin_unlock_irqrestore(&ida->lock, flags);
+
+	return ret;
+}
+
 /*
  * Ganged allocation - amortize locking and tree traversal for when we've got
  * another allocator (i.e. a percpu version) acting as a frontend to this code
@@ -940,6 +957,55 @@ void idr_remove(struct idr *idr, unsigned id)
 }
 EXPORT_SYMBOL(idr_remove);
 
+/**
+ * idr_preload - preload for idr_alloc_range()
+ * @idr: idr to ensure has room to allocate an id
+ * @start: value that will be passed to ida_alloc_range()
+ * @gfp: allocation mask to use for preloading
+ *
+ * On success, guarantees that one call of idr_alloc()/idr_alloc_range() won't
+ * fail. Returns with preemption disabled; use idr_preload_end() when
+ * finished.
+ *
+ * It's not required to check for failure if you're still checking for
+ * idr_alloc() failure.
+ *
+ * In order to guarantee idr_alloc() won't fail, all allocations from @idr must
+ * make use of idr_preload().
+ */
+int idr_preload(struct idr *idr, unsigned start, gfp_t gfp)
+{
+	int radix_ret, ida_ret = 0;
+
+	might_sleep_if(gfp & __GFP_WAIT);
+
+	while (1) {
+		radix_ret = radix_tree_preload(gfp);
+
+		/*
+		 * Well this is horrible, but radix_tree_preload() doesn't
+		 * disable preemption if it fails, and idr_preload() users don't
+		 * check for errors
+		 */
+		if (radix_ret)
+			preempt_disable();
+
+		/* if ida_preload with GFP_WAIT failed, don't retry */
+		if (ida_ret)
+			break;
+
+		if (!ida_preload(&idr->ida, start, GFP_NOWAIT) ||
+		    !(gfp & __GFP_WAIT))
+			break;
+
+		radix_tree_preload_end();
+		ida_ret = ida_preload(&idr->ida, start, gfp);
+	}
+
+	return radix_ret ?: ida_ret;
+}
+EXPORT_SYMBOL(idr_preload);
+
 static int idr_insert(struct idr *idr, void *ptr, unsigned id,
 		      gfp_t gfp, unsigned long *flags)
 {
-- 
1.8.3.1

Re: [PATCH 10/10] idr: Rework idr_preload()

[Tejun Heo]

Hello, Kent.

On Tue, Jun 18, 2013 at 05:02:30PM -0700, Kent Overstreet wrote:
> With the new ida implementation, that doesn't work anymore - the new ida
> code grows its bitmap tree by reallocating the entire thing in power of
> two increments. Doh.

So, I'm not sure about the single contiguous array implementation.  It
introduces some nasty characteristics such as possibly too large
contiguous allocation, bursty CPU usages, and loss of the ability to
handle ID allocations clustered in high areas - sure, the old ida
wasn't great on that part either but this can be much worse.

In addition, I'm not sure what this single contiguous allocation buys
us.  Let's say each leaf node size is 4k.  Even with in-array tree
implementation, it should be able to serve 2k IDs per-page, which
would be enough for most use cases and with a bit of caching it can
easily achieve both the performance benefits of array implementation
and the flexibility of allocating each leaf separately.  Even without
caching, the tree depth would normally be much lower than the current
implementation and the performance should be better.  If you're
bothered by having to implement another radix tree for it, it should
be possible to just implement the leaf node logic and use the existing
radix tree for internal nodes, right?

> So we need a slightly different trick. Note that if all allocations from
> an idr start by calling idr_prealloc() and disabling premption, at
> most nr_cpus() allocations can happen before someone calls
> idr_prealloc() again.

But we allow mixing preloaded and normal allocations and users are
allowed to allocate as many IDs they want after preloading.  It should
guarantee that the first allocation after preloading follows the
stronger allocation flag, and the above approach can't guarantee that.
You can declare that if preload is to work, all allocators of the ida
should preload and enforce it but that restriction arises only because
you're using this single array implementation.  It's another drawback
of this particular approach.  There seem to be a lot of cons and I
can't really see what the pros are.

Thanks.

-- 
tejun

Re: [PATCH 10/10] idr: Rework idr_preload()

[Kent Overstreet]

On Wed, Jun 19, 2013 at 01:18:32AM -0700, Tejun Heo wrote:
> Hello, Kent.
> 
> On Tue, Jun 18, 2013 at 05:02:30PM -0700, Kent Overstreet wrote:
> > With the new ida implementation, that doesn't work anymore - the new ida
> > code grows its bitmap tree by reallocating the entire thing in power of
> > two increments. Doh.
> 
> So, I'm not sure about the single contiguous array implementation.  It
> introduces some nasty characteristics such as possibly too large
> contiguous allocation, bursty CPU usages, and loss of the ability to
> handle ID allocations clustered in high areas - sure, the old ida
> wasn't great on that part either but this can be much worse.
> 
> In addition, I'm not sure what this single contiguous allocation buys
> us.  Let's say each leaf node size is 4k.  Even with in-array tree
> implementation, it should be able to serve 2k IDs per-page, which
> would be enough for most use cases and with a bit of caching it can
> easily achieve both the performance benefits of array implementation
> and the flexibility of allocating each leaf separately.  Even without
> caching, the tree depth would normally be much lower than the current
> implementation and the performance should be better.  If you're
> bothered by having to implement another radix tree for it, it should
> be possible to just implement the leaf node logic and use the existing
> radix tree for internal nodes, right?

With respect to performance, strongly disagree. Leaving pointers out of
the interior nodes cuts our space requirements by a factor of _64_ -
that's huge, and with data structures the dominating factors w.r.t.
performance tend to be the amount of memory you touch and the amount of
pointer chasing.

The lack of pointer chasing also means I could add prefetching to the
tree walking code (child nodes are always contiguous in memory) like I
did in bcache's binary search trees - I didn't realize DLM was using
this for so many ids so I'll probably add that.

That means for quite large bitmaps, _all_ the interior nodes will fit
onto a couple cachelines which are all contiguous in memory. That's
_huge_.

Even for 1 million ids - that's 128 kilobytes for all the leaf nodes,
which means all the interior nodes take up 2 kilobytes - which again is
contiguous so we can prefetch far in advance as we walk down the tree.

(If I was optimizing for huge bitmaps I would've picked a bigger splay
factor and the interior nodes would take up even less memory, but I've
been assuming the common case for the bitmap size is less than a page in
which case BITS_PER_LONG should be about right).

Also, idr_find() was slower than radix_tree_lookup() before, as tested
for some aio patches - decoupling the id allocation from the radix tree
gives the id allocator more freedom in its data structures (couldn't
realloc before without breaking RCU lookup).

I'm highly skeptical the bursty CPU usage is going to be a real issue in
practice, as that can happen anywhere we do allocation - the realloc is
going to be trivial compared to what can happen then. What's important
is just the amortized cpu overhead, and in that respect doing the
realloc is definitely a performance win.

Besides all that, the ida/idr reimplementations deleted > 300 lines of
code from idr.[ch]. If you try to add caching to the existing ida code,
it's only going to get more complicated - and trying to maintain the
behaviour where we always allocate the smallest available id is going to
be fun there (the cache has to be kept in sorted order every time you
free an id).

Sparse id allocations/allocations clustered in the high areas isn't a
concern - part of the reason I separated out ida_alloc() from
ida_alloc_range() was to make sure none of the existing code was doing
anything dumb with the starting id.

The only thing that would've been a problem there was idr_alloc_cyclic()
(and the code that was open coding ida_alloc_cyclic() as a performance
hack), but the new ida_alloc_cyclic() handles that - handles it better
than the old version, too.

The only real concern I've come across is the fact that this
implementation currently can't allocate up to INT_MAX ids with the whole
allocation being contiguous - for all the uses I'd looked at I didn't
think this was going to be an issue, but turns out it probably will be
for DLM. So I've got a bit more work to do there.

(I'm still not going to go with anything resembling a radix tree though
- instead of having a flat array, I'll have a an array of pointers to
fixed size array segments once the entire tree exceeds a certain size).

> > So we need a slightly different trick. Note that if all allocations from
> > an idr start by calling idr_prealloc() and disabling premption, at
> > most nr_cpus() allocations can happen before someone calls
> > idr_prealloc() again.
> 
> But we allow mixing preloaded and normal allocations and users are
> allowed to allocate as many IDs they want after preloading.  It should
> guarantee that the first allocation after preloading follows the
> stronger allocation flag, and the above approach can't guarantee that.

None of the existing code nedes that guarantee, though.

> You can declare that if preload is to work, all allocators of the ida
> should preload and enforce it but that restriction arises only because
> you're using this single array implementation.  It's another drawback
> of this particular approach. 

That's what I documented, and I audited all the existing idr_preload()
users (had to anyways). Generally speaking idr allocations are done from
a central location anyways so IMO it's a pretty trivial issue in
practice.

Re: [PATCH 10/10] idr: Rework idr_preload()

[Tejun Heo]

On Wed, Jun 19, 2013 at 04:33:44PM -0700, Kent Overstreet wrote:
> With respect to performance, strongly disagree. Leaving pointers out of
> the interior nodes cuts our space requirements by a factor of _64_ -
> that's huge, and with data structures the dominating factors w.r.t.
> performance tend to be the amount of memory you touch and the amount of
> pointer chasing.
> 
> The lack of pointer chasing also means I could add prefetching to the
> tree walking code (child nodes are always contiguous in memory) like I
> did in bcache's binary search trees - I didn't realize DLM was using
> this for so many ids so I'll probably add that.
>
> That means for quite large bitmaps, _all_ the interior nodes will fit
> onto a couple cachelines which are all contiguous in memory. That's
> _huge_.

Do all that in the leaf node which will be able to serve most use
cases with single leaf node anyway, so you really don't lose anything.

> Even for 1 million ids - that's 128 kilobytes for all the leaf nodes,
> which means all the interior nodes take up 2 kilobytes - which again is
> contiguous so we can prefetch far in advance as we walk down the tree.

So, that's ~30k IDs per page, right?  Let the internal node have 64
fan out, then you'll only have single depth tree for 1mil.  Again,
you're not losing much, if anything, while gaining more predictable
behavior and flexibility.

> Also, idr_find() was slower than radix_tree_lookup() before, as tested
> for some aio patches - decoupling the id allocation from the radix tree
> gives the id allocator more freedom in its data structures (couldn't
> realloc before without breaking RCU lookup).

Yeah, sure.  I don't have any problem implementing idr in terms of
ida.  I do have problems with ida being one contiguous array.

> Besides all that, the ida/idr reimplementations deleted > 300 lines of
> code from idr.[ch]. If you try to add caching to the existing ida code,
> it's only going to get more complicated - and trying to maintain the
> behaviour where we always allocate the smallest available id is going to
> be fun there (the cache has to be kept in sorted order every time you
> free an id).

It's like recursive code.  It looks more elegant and looks okay for
most cases but breaks in corner cases and we end up converting it to
iterative code anyway.  Similar thing.  Long contiguous arrays just
don't work.  We're very likely to break it up eventually anyway.

> (I'm still not going to go with anything resembling a radix tree though
> - instead of having a flat array, I'll have a an array of pointers to
> fixed size array segments once the entire tree exceeds a certain size).

I don't really care how it gets split but firm nack on single
contiguous array.

> > But we allow mixing preloaded and normal allocations and users are
> > allowed to allocate as many IDs they want after preloading.  It should
> > guarantee that the first allocation after preloading follows the
> > stronger allocation flag, and the above approach can't guarantee that.
> 
> None of the existing code nedes that guarantee, though.

Hmmm?  ISTR users mixing preloaded and !preloaded allocations.  Maybe
I'm misremembering.  I don't know.  But still the API is nasty like
hell.  Nobody is gonna notice even if it's being misused.  It's just
gonna have allocation failure after preloading once in a blue moon and
we won't be able to track it down.

> That's what I documented, and I audited all the existing idr_preload()
> users (had to anyways). Generally speaking idr allocations are done from
> a central location anyways so IMO it's a pretty trivial issue in
> practice.

If that has to happen, you need to enforce it.  Trigger WARN if
somebody violates the rule, but really this is just nasty.

Thanks.

-- 
tejun

[PATCH 10/10] idr: Rework idr_preload()

[Kent Overstreet]

The old idr_preload() used percpu buffers - since the
bitmap/radix/whatever tree only grew by fixed sized nodes, it only had
to ensure there was a node available in the percpu buffer and disable
preemption. This conveniently meant that you didn't have to pass the idr
you were going to allocate from to it.

With the new ida implementation, that doesn't work anymore - the new ida
code grows its bitmap tree by reallocating the entire thing in power of
two increments. Doh.

So we need a slightly different trick. Note that if all allocations from
an idr start by calling idr_prealloc() and disabling premption, at
most nr_cpus() allocations can happen before someone calls
idr_prealloc() again.

So, we just change idr_prealloc() to resize the ida bitmap tree if
there's less than num_possible_cpus() ids available - conveniently, we
already track the number of allocated ids, and the total number of ids
we can have allocated is just nr_leaf_nodes * BITS_PER_LONG. Easy.

This does require a fairly trivial interface change - we now have to
pass the idr we're going to allocate from (and the starting id we're
going to pass to idr_allocate_range()) to idr_prealloc(), so this patch
updates all the callers.

Signed-off-by: Kent Overstreet <koverstreet@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Tejun Heo <tj@kernel.org>
Cc: Stefan Richter <stefanr@s5r6.in-berlin.de>
Cc: David Airlie <airlied@linux.ie>
Cc: Roland Dreier <roland@kernel.org>
Cc: Sean Hefty <sean.hefty@intel.com>
Cc: Hal Rosenstock <hal.rosenstock@gmail.com>
Cc: Steve Wise <swise@chelsio.com>
Cc: Hoang-Nam Nguyen <hnguyen@de.ibm.com>
Cc: Christoph Raisch <raisch@de.ibm.com>
Cc: Mike Marciniszyn <infinipath@intel.com>
Cc: Doug Gilbert <dgilbert@interlog.com>
Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
Cc: Christine Caulfield <ccaulfie@redhat.com>
Cc: David Teigland <teigland@redhat.com>
Cc: Trond Myklebust <Trond.Myklebust@netapp.com>
Cc: John McCutchan <john@johnmccutchan.com>
Cc: Robert Love <rlove@rlove.org>
Cc: Eric Paris <eparis@parisplace.org>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Thomas Hellstrom <thellstrom@vmware.com>
Cc: Brian Paul <brianp@vmware.com>
Cc: Maarten Lankhorst <maarten.lankhorst@canonical.com>
Cc: Dmitry Torokhov <dtor@vmware.com>
Cc: Erez Shitrit <erezsh@mellanox.co.il>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Haggai Eran <haggaie@mellanox.com>
Cc: Jack Morgenstein <jackm@dev.mellanox.co.il>
Cc: Wolfram Sang <wolfram@the-dreams.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Davidlohr Bueso <davidlohr.bueso@hp.com>
Cc: Rik van Riel <riel@redhat.com>
Cc: Michel Lespinasse <walken@google.com>
Cc: linux1394-devel@lists.sourceforge.net
Cc: linux-kernel@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: linux-rdma@vger.kernel.org
Cc: linux-scsi@vger.kernel.org
Cc: cluster-devel@redhat.com
Cc: linux-nfs@vger.kernel.org
Signed-off-by: Kent Overstreet <kmo@daterainc.com>
---
 drivers/firewire/core-cdev.c               |  2 +-
 drivers/gpu/drm/drm_gem.c                  |  4 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_resource.c   |  2 +-
 drivers/infiniband/core/cm.c               |  7 +---
 drivers/infiniband/core/sa_query.c         |  2 +-
 drivers/infiniband/core/uverbs_cmd.c       |  2 +-
 drivers/infiniband/hw/cxgb3/iwch.h         |  2 +-
 drivers/infiniband/hw/cxgb4/iw_cxgb4.h     |  2 +-
 drivers/infiniband/hw/ehca/ehca_cq.c       |  2 +-
 drivers/infiniband/hw/ehca/ehca_qp.c       |  2 +-
 drivers/infiniband/hw/ipath/ipath_driver.c |  2 +-
 drivers/infiniband/hw/mlx4/cm.c            |  2 +-
 drivers/infiniband/hw/qib/qib_init.c       |  2 +-
 drivers/scsi/sg.c                          |  2 +-
 fs/dlm/lock.c                              |  2 +-
 fs/dlm/recover.c                           |  2 +-
 fs/nfs/nfs4client.c                        |  2 +-
 fs/notify/inotify/inotify_user.c           |  2 +-
 include/linux/idr.h                        | 37 +----------------
 ipc/util.c                                 |  4 +-
 lib/idr.c                                  | 66 ++++++++++++++++++++++++++++++
 21 files changed, 91 insertions(+), 59 deletions(-)

diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
index 436debf..3c70fbc 100644
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -490,7 +490,7 @@ static int add_client_resource(struct client *client,
 	int ret;
 
 	if (preload)
-		idr_preload(gfp_mask);
+		idr_preload(&client->resource_idr, 0, gfp_mask);
 	spin_lock_irqsave(&client->lock, flags);
 
 	if (client->in_shutdown)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 1c897b9..cf50894 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -273,7 +273,7 @@ drm_gem_handle_create(struct drm_file *file_priv,
 	 * Get the user-visible handle using idr.  Preload and perform
 	 * allocation under our spinlock.
 	 */
-	idr_preload(GFP_KERNEL);
+	idr_preload(&file_priv->object_idr, 1, GFP_KERNEL);
 	spin_lock(&file_priv->table_lock);
 
 	ret = idr_alloc_range(&file_priv->object_idr, obj, 1, 0, GFP_NOWAIT);
@@ -449,7 +449,7 @@ drm_gem_flink_ioctl(struct drm_device *dev, void *data,
 	if (obj == NULL)
 		return -ENOENT;
 
-	idr_preload(GFP_KERNEL);
+	idr_preload(&dev->object_name_idr, 1, GFP_KERNEL);
 	spin_lock(&dev->object_name_lock);
 	if (!obj->name) {
 		ret = idr_alloc_range(&dev->object_name_idr, obj, 1, 0, GFP_NOWAIT);
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
index ccbaed1..9f00706 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
@@ -177,7 +177,7 @@ int vmw_resource_alloc_id(struct vmw_resource *res)
 
 	BUG_ON(res->id != -1);
 
-	idr_preload(GFP_KERNEL);
+	idr_preload(idr, 1, GFP_KERNEL);
 	write_lock(&dev_priv->resource_lock);
 
 	ret = idr_alloc_range(idr, res, 1, 0, GFP_NOWAIT);
diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c
index 86008a9..a11bb5e 100644
--- a/drivers/infiniband/core/cm.c
+++ b/drivers/infiniband/core/cm.c
@@ -383,14 +383,11 @@ static int cm_alloc_id(struct cm_id_private *cm_id_priv)
 {
 	unsigned long flags;
 	int id;
-	static int next_id;
 
-	idr_preload(GFP_KERNEL);
+	idr_preload(&cm.local_id_table, 0, GFP_KERNEL);
 	spin_lock_irqsave(&cm.lock, flags);
 
-	id = idr_alloc_range(&cm.local_id_table, cm_id_priv, next_id, 0, GFP_NOWAIT);
-	if (id >= 0)
-		next_id = max(id + 1, 0);
+	id = idr_alloc_cyclic(&cm.local_id_table, cm_id_priv, 0, 0, GFP_NOWAIT);
 
 	spin_unlock_irqrestore(&cm.lock, flags);
 	idr_preload_end();
diff --git a/drivers/infiniband/core/sa_query.c b/drivers/infiniband/core/sa_query.c
index 509d5a6..9fc181f 100644
--- a/drivers/infiniband/core/sa_query.c
+++ b/drivers/infiniband/core/sa_query.c
@@ -616,7 +616,7 @@ static int send_mad(struct ib_sa_query *query, int timeout_ms, gfp_t gfp_mask)
 	int ret, id;
 
 	if (preload)
-		idr_preload(gfp_mask);
+		idr_preload(&query_idr, 0, gfp_mask);
 	spin_lock_irqsave(&idr_lock, flags);
 
 	id = idr_alloc(&query_idr, query, GFP_NOWAIT);
diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index 775431a..b1dfb30 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -125,7 +125,7 @@ static int idr_add_uobj(struct idr *idr, struct ib_uobject *uobj)
 {
 	int ret;
 
-	idr_preload(GFP_KERNEL);
+	idr_preload(idr, 0, GFP_KERNEL);
 	spin_lock(&ib_uverbs_idr_lock);
 
 	ret = idr_alloc(idr, uobj, GFP_NOWAIT);
diff --git a/drivers/infiniband/hw/cxgb3/iwch.h b/drivers/infiniband/hw/cxgb3/iwch.h
index f28c585..12e5f29 100644
--- a/drivers/infiniband/hw/cxgb3/iwch.h
+++ b/drivers/infiniband/hw/cxgb3/iwch.h
@@ -154,7 +154,7 @@ static inline int insert_handle(struct iwch_dev *rhp, struct idr *idr,
 {
 	int ret;
 
-	idr_preload(GFP_KERNEL);
+	idr_preload(idr, id, GFP_KERNEL);
 	spin_lock_irq(&rhp->lock);
 
 	ret = idr_alloc_range(idr, handle, id, id + 1, GFP_NOWAIT);
diff --git a/drivers/infiniband/hw/cxgb4/iw_cxgb4.h b/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
index 50e5a3f..e6a5fc3 100644
--- a/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
+++ b/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
@@ -262,7 +262,7 @@ static inline int _insert_handle(struct c4iw_dev *rhp, struct idr *idr,
 	int ret;
 
 	if (lock) {
-		idr_preload(GFP_KERNEL);
+		idr_preload(idr, id, GFP_KERNEL);
 		spin_lock_irq(&rhp->lock);
 	}
 
diff --git a/drivers/infiniband/hw/ehca/ehca_cq.c b/drivers/infiniband/hw/ehca/ehca_cq.c
index 0bc5c51..89c02e4 100644
--- a/drivers/infiniband/hw/ehca/ehca_cq.c
+++ b/drivers/infiniband/hw/ehca/ehca_cq.c
@@ -163,7 +163,7 @@ struct ib_cq *ehca_create_cq(struct ib_device *device, int cqe, int comp_vector,
 	adapter_handle = shca->ipz_hca_handle;
 	param.eq_handle = shca->eq.ipz_eq_handle;
 
-	idr_preload(GFP_KERNEL);
+	idr_preload(&ehca_cq_idr, 0, GFP_KERNEL);
 	write_lock_irqsave(&ehca_cq_idr_lock, flags);
 	my_cq->token = idr_alloc_range(&ehca_cq_idr, my_cq, 0, 0x2000000, GFP_NOWAIT);
 	write_unlock_irqrestore(&ehca_cq_idr_lock, flags);
diff --git a/drivers/infiniband/hw/ehca/ehca_qp.c b/drivers/infiniband/hw/ehca/ehca_qp.c
index 758a265..4184133 100644
--- a/drivers/infiniband/hw/ehca/ehca_qp.c
+++ b/drivers/infiniband/hw/ehca/ehca_qp.c
@@ -636,7 +636,7 @@ static struct ehca_qp *internal_create_qp(
 		my_qp->send_cq =
 			container_of(init_attr->send_cq, struct ehca_cq, ib_cq);
 
-	idr_preload(GFP_KERNEL);
+	idr_preload(&ehca_qp_idr, 0, GFP_KERNEL);
 	write_lock_irqsave(&ehca_qp_idr_lock, flags);
 
 	ret = idr_alloc_range(&ehca_qp_idr, my_qp, 0, 0x2000000, GFP_NOWAIT);
diff --git a/drivers/infiniband/hw/ipath/ipath_driver.c b/drivers/infiniband/hw/ipath/ipath_driver.c
index 83a40a5..b241f42 100644
--- a/drivers/infiniband/hw/ipath/ipath_driver.c
+++ b/drivers/infiniband/hw/ipath/ipath_driver.c
@@ -201,7 +201,7 @@ static struct ipath_devdata *ipath_alloc_devdata(struct pci_dev *pdev)
 	}
 	dd->ipath_unit = -1;
 
-	idr_preload(GFP_KERNEL);
+	idr_preload(&unit_table, 0, GFP_KERNEL);
 	spin_lock_irqsave(&ipath_devs_lock, flags);
 
 	ret = idr_alloc(&unit_table, dd, GFP_NOWAIT);
diff --git a/drivers/infiniband/hw/mlx4/cm.c b/drivers/infiniband/hw/mlx4/cm.c
index d1f5f1d..ac089e6 100644
--- a/drivers/infiniband/hw/mlx4/cm.c
+++ b/drivers/infiniband/hw/mlx4/cm.c
@@ -219,7 +219,7 @@ id_map_alloc(struct ib_device *ibdev, int slave_id, u32 sl_cm_id)
 	ent->dev = to_mdev(ibdev);
 	INIT_DELAYED_WORK(&ent->timeout, id_map_ent_timeout);
 
-	idr_preload(GFP_KERNEL);
+	idr_preload(&sriov->pv_id_table, 0, GFP_KERNEL);
 	spin_lock(&to_mdev(ibdev)->sriov.id_map_lock);
 
 	ret = idr_alloc_cyclic(&sriov->pv_id_table, ent, 0, 0, GFP_NOWAIT);
diff --git a/drivers/infiniband/hw/qib/qib_init.c b/drivers/infiniband/hw/qib/qib_init.c
index 503619c..08d9703 100644
--- a/drivers/infiniband/hw/qib/qib_init.c
+++ b/drivers/infiniband/hw/qib/qib_init.c
@@ -1066,7 +1066,7 @@ struct qib_devdata *qib_alloc_devdata(struct pci_dev *pdev, size_t extra)
 		goto bail;
 	}
 
-	idr_preload(GFP_KERNEL);
+	idr_preload(&qib_unit_table, 0, GFP_KERNEL);
 	spin_lock_irqsave(&qib_devs_lock, flags);
 
 	ret = idr_alloc(&qib_unit_table, dd, GFP_NOWAIT);
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 23856c8..d226a64 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -1392,7 +1392,7 @@ static Sg_device *sg_alloc(struct gendisk *disk, struct scsi_device *scsidp)
 		return ERR_PTR(-ENOMEM);
 	}
 
-	idr_preload(GFP_KERNEL);
+	idr_preload(&sg_index_idr, 0, GFP_KERNEL);
 	write_lock_irqsave(&sg_index_lock, iflags);
 
 	error = idr_alloc_range(&sg_index_idr, sdp, 0, SG_MAX_DEVS, GFP_NOWAIT);
diff --git a/fs/dlm/lock.c b/fs/dlm/lock.c
index 85bba95..7dd15dd 100644
--- a/fs/dlm/lock.c
+++ b/fs/dlm/lock.c
@@ -1199,7 +1199,7 @@ static int create_lkb(struct dlm_ls *ls, struct dlm_lkb **lkb_ret)
 	mutex_init(&lkb->lkb_cb_mutex);
 	INIT_WORK(&lkb->lkb_cb_work, dlm_callback_work);
 
-	idr_preload(GFP_NOFS);
+	idr_preload(&ls->ls_lkbidr, 1, GFP_NOFS);
 	spin_lock(&ls->ls_lkbidr_spin);
 	rv = idr_alloc_range(&ls->ls_lkbidr, lkb, 1, 0, GFP_NOWAIT);
 	if (rv >= 0)
diff --git a/fs/dlm/recover.c b/fs/dlm/recover.c
index 2babe5e..757b7a6 100644
--- a/fs/dlm/recover.c
+++ b/fs/dlm/recover.c
@@ -307,7 +307,7 @@ static int recover_idr_add(struct dlm_rsb *r)
 	struct dlm_ls *ls = r->res_ls;
 	int rv;
 
-	idr_preload(GFP_NOFS);
+	idr_preload(&ls->ls_recover_idr, 1, GFP_NOFS);
 	spin_lock(&ls->ls_recover_idr_lock);
 	if (r->res_id) {
 		rv = -1;
diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
index 786aac3..85c904e 100644
--- a/fs/nfs/nfs4client.c
+++ b/fs/nfs/nfs4client.c
@@ -30,7 +30,7 @@ static int nfs_get_cb_ident_idr(struct nfs_client *clp, int minorversion)
 
 	if (clp->rpc_ops->version != 4 || minorversion != 0)
 		return ret;
-	idr_preload(GFP_KERNEL);
+	idr_preload(&nn->cb_ident_idr, 0, GFP_KERNEL);
 	spin_lock(&nn->nfs_client_lock);
 	ret = idr_alloc(&nn->cb_ident_idr, clp, GFP_NOWAIT);
 	if (ret >= 0)
diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c
index 959815c..04302e8 100644
--- a/fs/notify/inotify/inotify_user.c
+++ b/fs/notify/inotify/inotify_user.c
@@ -360,7 +360,7 @@ static int inotify_add_to_idr(struct idr *idr, spinlock_t *idr_lock,
 {
 	int ret;
 
-	idr_preload(GFP_KERNEL);
+	idr_preload(idr, 1, GFP_KERNEL);
 	spin_lock(idr_lock);
 
 	ret = idr_alloc_cyclic(idr, i_mark, 1, 0, GFP_NOWAIT);
diff --git a/include/linux/idr.h b/include/linux/idr.h
index 85355d7..418d87c 100644
--- a/include/linux/idr.h
+++ b/include/linux/idr.h
@@ -177,6 +177,7 @@ int idr_for_each(struct idr *idr,
 		 int (*fn)(int id, void *p, void *data), void *data);
 void *idr_replace(struct idr *idr, void *ptr, unsigned id);
 void idr_remove(struct idr *idr, unsigned id);
+int idr_preload(struct idr *idr, unsigned start, gfp_t gfp);
 int idr_alloc_range(struct idr *idr, void *ptr, unsigned start,
 		    unsigned end, gfp_t gfp);
 int idr_alloc_cyclic(struct idr *idr, void *ptr, unsigned start,
@@ -197,41 +198,7 @@ static inline int idr_alloc(struct idr *idr, void *ptr, gfp_t gfp)
  */
 static inline void idr_preload_end(void)
 {
-	radix_tree_preload_end();
-}
-
-/**
- * idr_preload - preload for idr_alloc_range()
- * @gfp: allocation mask to use for preloading
- *
- * Preload per-cpu layer buffer for idr_alloc_range().  Can only be used from
- * process context and each idr_preload() invocation should be matched with
- * idr_preload_end().  Note that preemption is disabled while preloaded.
- *
- * The first idr_alloc_range() in the preloaded section can be treated as if it
- * were invoked with @gfp_mask used for preloading.  This allows using more
- * permissive allocation masks for idrs protected by spinlocks.
- *
- * For example, if idr_alloc_range() below fails, the failure can be treated as
- * if idr_alloc_range() were called with GFP_KERNEL rather than GFP_NOWAIT.
- *
- *	idr_preload(GFP_KERNEL);
- *	spin_lock(lock);
- *
- *	id = idr_alloc_range(idr, ptr, start, end, GFP_NOWAIT);
- *
- *	spin_unlock(lock);
- *	idr_preload_end();
- *	if (id < 0)
- *		error;
- */
-static inline void idr_preload(gfp_t gfp)
-{
-	might_sleep_if(gfp & __GFP_WAIT);
-
-	/* Well this is horrible, but idr_preload doesn't return errors */
-	if (radix_tree_preload(gfp))
-		preempt_disable();
+	preempt_enable();
 }
 
 /* radix tree can't store NULL pointers, so we have to translate...  */
diff --git a/ipc/util.c b/ipc/util.c
index 749511d..5988e6b 100644
--- a/ipc/util.c
+++ b/ipc/util.c
@@ -262,7 +262,9 @@ int ipc_addid(struct ipc_ids* ids, struct kern_ipc_perm* new, int size)
 	if (ids->in_use >= size)
 		return -ENOSPC;
 
-	idr_preload(GFP_KERNEL);
+	idr_preload(&ids->ipcs_idr,
+		    (next_id < 0) ? 0 : ipcid_to_idx(next_id),
+		    GFP_KERNEL);
 
 	spin_lock_init(&new->lock);
 	new->deleted = 0;
diff --git a/lib/idr.c b/lib/idr.c
index fc7cb1a..6001805 100644
--- a/lib/idr.c
+++ b/lib/idr.c
@@ -294,6 +294,23 @@ err:
 	return -ENOMEM;
 }
 
+static int ida_preload(struct ida *ida, unsigned start, gfp_t gfp)
+{
+	int ret = 0;
+	unsigned long flags;
+
+	spin_lock_irqsave(&ida->lock, flags);
+
+	while (!ret &&
+	       (ida->nodes - ida->first_leaf * BITS_PER_LONG <
+		start + ida->allocated_ids + num_possible_cpus()))
+		ret = __ida_resize(ida, gfp, &flags);
+
+	spin_unlock_irqrestore(&ida->lock, flags);
+
+	return ret;
+}
+
 /*
  * Ganged allocation - amortize locking and tree traversal for when we've got
  * another allocator (i.e. a percpu version) acting as a frontend to this code
@@ -1028,6 +1045,55 @@ void idr_remove(struct idr *idr, unsigned id)
 }
 EXPORT_SYMBOL(idr_remove);
 
+/**
+ * idr_preload - preload for idr_alloc_range()
+ * @idr: idr to ensure has room to allocate an id
+ * @start: value that will be passed to ida_alloc_range()
+ * @gfp: allocation mask to use for preloading
+ *
+ * On success, guarantees that one call of idr_alloc()/idr_alloc_range() won't
+ * fail. Returns with preemption disabled; use idr_preload_end() when
+ * finished.
+ *
+ * It's not required to check for failure if you're still checking for
+ * idr_alloc() failure.
+ *
+ * In order to guarantee idr_alloc() won't fail, all allocations from @idr must
+ * make use of idr_preload().
+ */
+int idr_preload(struct idr *idr, unsigned start, gfp_t gfp)
+{
+	int radix_ret, ida_ret = 0;
+
+	might_sleep_if(gfp & __GFP_WAIT);
+
+	while (1) {
+		radix_ret = radix_tree_preload(gfp);
+
+		/*
+		 * Well this is horrible, but radix_tree_preload() doesn't
+		 * disable preemption if it fails, and idr_preload() users don't
+		 * check for errors
+		 */
+		if (radix_ret)
+			preempt_disable();
+
+		/* if ida_preload with GFP_WAIT failed, don't retry */
+		if (ida_ret)
+			break;
+
+		if (!ida_preload(&idr->ida, start, GFP_NOWAIT) ||
+		    !(gfp & __GFP_WAIT))
+			break;
+
+		radix_tree_preload_end();
+		ida_ret = ida_preload(&idr->ida, start, gfp);
+	}
+
+	return radix_ret ?: ida_ret;
+}
+EXPORT_SYMBOL(idr_preload);
+
 static int idr_insert(struct idr *idr, void *ptr, unsigned id,
 		      gfp_t gfp, unsigned long *flags)
 {
-- 
1.8.3.2


------------------------------------------------------------------------------
This SF.net email is sponsored by Windows:

Build for Windows Store.

http://p.sf.net/sfu/windows-dev2dev

[PATCH 10/10] idr: Rework idr_preload()

[Kent Overstreet]

The old idr_preload() used percpu buffers - since the
bitmap/radix/whatever tree only grew by fixed sized nodes, it only had
to ensure there was a node available in the percpu buffer and disable
preemption. This conveniently meant that you didn't have to pass the idr
you were going to allocate from to it.

With the new ida implementation, that doesn't work anymore - the new ida
code grows its bitmap tree by reallocating the entire thing in power of
two increments. Doh.

So we need a slightly different trick. Note that if all allocations from
an idr start by calling idr_prealloc() and disabling premption, at
most nr_cpus() allocations can happen before someone calls
idr_prealloc() again.

So, we just change idr_prealloc() to resize the ida bitmap tree if
there's less than num_possible_cpus() ids available - conveniently, we
already track the number of allocated ids, and the total number of ids
we can have allocated is just nr_leaf_nodes * BITS_PER_LONG. Easy.

This does require a fairly trivial interface change - we now have to
pass the idr we're going to allocate from (and the starting id we're
going to pass to idr_allocate_range()) to idr_prealloc(), so this patch
updates all the callers.

Signed-off-by: Kent Overstreet <kmo@daterainc.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Tejun Heo <tj@kernel.org>
Cc: Stefan Richter <stefanr@s5r6.in-berlin.de>
Cc: David Airlie <airlied@linux.ie>
Cc: Roland Dreier <roland@kernel.org>
Cc: Sean Hefty <sean.hefty@intel.com>
Cc: Hal Rosenstock <hal.rosenstock@gmail.com>
Cc: Steve Wise <swise@chelsio.com>
Cc: Hoang-Nam Nguyen <hnguyen@de.ibm.com>
Cc: Christoph Raisch <raisch@de.ibm.com>
Cc: Mike Marciniszyn <infinipath@intel.com>
Cc: Doug Gilbert <dgilbert@interlog.com>
Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
Cc: Christine Caulfield <ccaulfie@redhat.com>
Cc: David Teigland <teigland@redhat.com>
Cc: Trond Myklebust <Trond.Myklebust@netapp.com>
Cc: John McCutchan <john@johnmccutchan.com>
Cc: Robert Love <rlove@rlove.org>
Cc: Eric Paris <eparis@parisplace.org>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Thomas Hellstrom <thellstrom@vmware.com>
Cc: Brian Paul <brianp@vmware.com>
Cc: Maarten Lankhorst <maarten.lankhorst@canonical.com>
Cc: Dmitry Torokhov <dtor@vmware.com>
Cc: Erez Shitrit <erezsh@mellanox.co.il>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Haggai Eran <haggaie@mellanox.com>
Cc: Jack Morgenstein <jackm@dev.mellanox.co.il>
Cc: Wolfram Sang <wolfram@the-dreams.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Davidlohr Bueso <davidlohr.bueso@hp.com>
Cc: Rik van Riel <riel@redhat.com>
Cc: Michel Lespinasse <walken@google.com>
Cc: linux1394-devel@lists.sourceforge.net
Cc: linux-kernel@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: linux-rdma@vger.kernel.org
Cc: linux-scsi@vger.kernel.org
Cc: cluster-devel@redhat.com
Cc: linux-nfs@vger.kernel.org
---
 drivers/firewire/core-cdev.c               |  2 +-
 drivers/gpu/drm/drm_gem.c                  |  4 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_resource.c   |  2 +-
 drivers/infiniband/core/cm.c               |  8 +---
 drivers/infiniband/core/sa_query.c         |  2 +-
 drivers/infiniband/core/uverbs_cmd.c       |  2 +-
 drivers/infiniband/hw/cxgb3/iwch.h         |  2 +-
 drivers/infiniband/hw/cxgb4/iw_cxgb4.h     |  2 +-
 drivers/infiniband/hw/ehca/ehca_cq.c       |  2 +-
 drivers/infiniband/hw/ehca/ehca_qp.c       |  2 +-
 drivers/infiniband/hw/ipath/ipath_driver.c |  2 +-
 drivers/infiniband/hw/mlx4/cm.c            |  2 +-
 drivers/infiniband/hw/qib/qib_init.c       |  2 +-
 drivers/scsi/sg.c                          |  2 +-
 fs/dlm/lock.c                              |  2 +-
 fs/dlm/recover.c                           |  2 +-
 fs/nfs/nfs4client.c                        |  2 +-
 fs/notify/inotify/inotify_user.c           |  2 +-
 include/linux/idr.h                        | 37 +----------------
 ipc/util.c                                 |  4 +-
 lib/idr.c                                  | 66 ++++++++++++++++++++++++++++++
 21 files changed, 91 insertions(+), 60 deletions(-)

diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
index ba78d08..08d31da 100644
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -491,7 +491,7 @@ static int add_client_resource(struct client *client,
 	int ret;
 
 	if (preload)
-		idr_preload(gfp_mask);
+		idr_preload(&client->resource_idr, 0, gfp_mask);
 	spin_lock_irqsave(&client->lock, flags);
 
 	if (client->in_shutdown)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index d12ea60..c8ed531 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -269,7 +269,7 @@ drm_gem_handle_create(struct drm_file *file_priv,
 	 * Get the user-visible handle using idr.  Preload and perform
 	 * allocation under our spinlock.
 	 */
-	idr_preload(GFP_KERNEL);
+	idr_preload(&file_priv->object_idr, 1, GFP_KERNEL);
 	spin_lock(&file_priv->table_lock);
 
 	ret = idr_alloc_range(&file_priv->object_idr, obj, 1, 0, GFP_NOWAIT);
@@ -445,7 +445,7 @@ drm_gem_flink_ioctl(struct drm_device *dev, void *data,
 	if (obj == NULL)
 		return -ENOENT;
 
-	idr_preload(GFP_KERNEL);
+	idr_preload(&dev->object_name_idr, 1, GFP_KERNEL);
 	spin_lock(&dev->object_name_lock);
 	if (!obj->name) {
 		ret = idr_alloc_range(&dev->object_name_idr,
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
index 4838238..1078b51 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
@@ -177,7 +177,7 @@ int vmw_resource_alloc_id(struct vmw_resource *res)
 
 	BUG_ON(res->id != -1);
 
-	idr_preload(GFP_KERNEL);
+	idr_preload(idr, 1, GFP_KERNEL);
 	write_lock(&dev_priv->resource_lock);
 
 	ret = idr_alloc_range(idr, res, 1, 0, GFP_NOWAIT);
diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c
index c686690..a11bb5e 100644
--- a/drivers/infiniband/core/cm.c
+++ b/drivers/infiniband/core/cm.c
@@ -383,15 +383,11 @@ static int cm_alloc_id(struct cm_id_private *cm_id_priv)
 {
 	unsigned long flags;
 	int id;
-	static int next_id;
 
-	idr_preload(GFP_KERNEL);
+	idr_preload(&cm.local_id_table, 0, GFP_KERNEL);
 	spin_lock_irqsave(&cm.lock, flags);
 
-	id = idr_alloc_range(&cm.local_id_table, cm_id_priv,
-			     next_id, 0, GFP_NOWAIT);
-	if (id >= 0)
-		next_id = max(id + 1, 0);
+	id = idr_alloc_cyclic(&cm.local_id_table, cm_id_priv, 0, 0, GFP_NOWAIT);
 
 	spin_unlock_irqrestore(&cm.lock, flags);
 	idr_preload_end();
diff --git a/drivers/infiniband/core/sa_query.c b/drivers/infiniband/core/sa_query.c
index ce757fd..b1ed7fd 100644
--- a/drivers/infiniband/core/sa_query.c
+++ b/drivers/infiniband/core/sa_query.c
@@ -616,7 +616,7 @@ static int send_mad(struct ib_sa_query *query, int timeout_ms, gfp_t gfp_mask)
 	int ret, id;
 
 	if (preload)
-		idr_preload(gfp_mask);
+		idr_preload(&query_idr, 0, gfp_mask);
 	spin_lock_irqsave(&idr_lock, flags);
 
 	id = idr_alloc(&query_idr, query, GFP_NOWAIT);
diff --git a/drivers/infiniband/core/uverbs_cmd.c b/drivers/infiniband/core/uverbs_cmd.c
index 9ddc2e0..0f20a27 100644
--- a/drivers/infiniband/core/uverbs_cmd.c
+++ b/drivers/infiniband/core/uverbs_cmd.c
@@ -125,7 +125,7 @@ static int idr_add_uobj(struct idr *idr, struct ib_uobject *uobj)
 {
 	int ret;
 
-	idr_preload(GFP_KERNEL);
+	idr_preload(idr, 0, GFP_KERNEL);
 	spin_lock(&ib_uverbs_idr_lock);
 
 	ret = idr_alloc(idr, uobj, GFP_NOWAIT);
diff --git a/drivers/infiniband/hw/cxgb3/iwch.h b/drivers/infiniband/hw/cxgb3/iwch.h
index f28c585..12e5f29 100644
--- a/drivers/infiniband/hw/cxgb3/iwch.h
+++ b/drivers/infiniband/hw/cxgb3/iwch.h
@@ -154,7 +154,7 @@ static inline int insert_handle(struct iwch_dev *rhp, struct idr *idr,
 {
 	int ret;
 
-	idr_preload(GFP_KERNEL);
+	idr_preload(idr, id, GFP_KERNEL);
 	spin_lock_irq(&rhp->lock);
 
 	ret = idr_alloc_range(idr, handle, id, id + 1, GFP_NOWAIT);
diff --git a/drivers/infiniband/hw/cxgb4/iw_cxgb4.h b/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
index 50e5a3f..e6a5fc3 100644
--- a/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
+++ b/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
@@ -262,7 +262,7 @@ static inline int _insert_handle(struct c4iw_dev *rhp, struct idr *idr,
 	int ret;
 
 	if (lock) {
-		idr_preload(GFP_KERNEL);
+		idr_preload(idr, id, GFP_KERNEL);
 		spin_lock_irq(&rhp->lock);
 	}
 
diff --git a/drivers/infiniband/hw/ehca/ehca_cq.c b/drivers/infiniband/hw/ehca/ehca_cq.c
index a3632ee..3886f43 100644
--- a/drivers/infiniband/hw/ehca/ehca_cq.c
+++ b/drivers/infiniband/hw/ehca/ehca_cq.c
@@ -163,7 +163,7 @@ struct ib_cq *ehca_create_cq(struct ib_device *device, int cqe, int comp_vector,
 	adapter_handle = shca->ipz_hca_handle;
 	param.eq_handle = shca->eq.ipz_eq_handle;
 
-	idr_preload(GFP_KERNEL);
+	idr_preload(&ehca_cq_idr, 0, GFP_KERNEL);
 	write_lock_irqsave(&ehca_cq_idr_lock, flags);
 	my_cq->token = idr_alloc_range(&ehca_cq_idr, my_cq, 0,
 				       0x2000000, GFP_NOWAIT);
diff --git a/drivers/infiniband/hw/ehca/ehca_qp.c b/drivers/infiniband/hw/ehca/ehca_qp.c
index 758a265..4184133 100644
--- a/drivers/infiniband/hw/ehca/ehca_qp.c
+++ b/drivers/infiniband/hw/ehca/ehca_qp.c
@@ -636,7 +636,7 @@ static struct ehca_qp *internal_create_qp(
 		my_qp->send_cq =
 			container_of(init_attr->send_cq, struct ehca_cq, ib_cq);
 
-	idr_preload(GFP_KERNEL);
+	idr_preload(&ehca_qp_idr, 0, GFP_KERNEL);
 	write_lock_irqsave(&ehca_qp_idr_lock, flags);
 
 	ret = idr_alloc_range(&ehca_qp_idr, my_qp, 0, 0x2000000, GFP_NOWAIT);
diff --git a/drivers/infiniband/hw/ipath/ipath_driver.c b/drivers/infiniband/hw/ipath/ipath_driver.c
index 83a40a5..b241f42 100644
--- a/drivers/infiniband/hw/ipath/ipath_driver.c
+++ b/drivers/infiniband/hw/ipath/ipath_driver.c
@@ -201,7 +201,7 @@ static struct ipath_devdata *ipath_alloc_devdata(struct pci_dev *pdev)
 	}
 	dd->ipath_unit = -1;
 
-	idr_preload(GFP_KERNEL);
+	idr_preload(&unit_table, 0, GFP_KERNEL);
 	spin_lock_irqsave(&ipath_devs_lock, flags);
 
 	ret = idr_alloc(&unit_table, dd, GFP_NOWAIT);
diff --git a/drivers/infiniband/hw/mlx4/cm.c b/drivers/infiniband/hw/mlx4/cm.c
index d1f5f1d..ac089e6 100644
--- a/drivers/infiniband/hw/mlx4/cm.c
+++ b/drivers/infiniband/hw/mlx4/cm.c
@@ -219,7 +219,7 @@ id_map_alloc(struct ib_device *ibdev, int slave_id, u32 sl_cm_id)
 	ent->dev = to_mdev(ibdev);
 	INIT_DELAYED_WORK(&ent->timeout, id_map_ent_timeout);
 
-	idr_preload(GFP_KERNEL);
+	idr_preload(&sriov->pv_id_table, 0, GFP_KERNEL);
 	spin_lock(&to_mdev(ibdev)->sriov.id_map_lock);
 
 	ret = idr_alloc_cyclic(&sriov->pv_id_table, ent, 0, 0, GFP_NOWAIT);
diff --git a/drivers/infiniband/hw/qib/qib_init.c b/drivers/infiniband/hw/qib/qib_init.c
index 17adbd10c..e7101b2 100644
--- a/drivers/infiniband/hw/qib/qib_init.c
+++ b/drivers/infiniband/hw/qib/qib_init.c
@@ -1106,7 +1106,7 @@ struct qib_devdata *qib_alloc_devdata(struct pci_dev *pdev, size_t extra)
 	qib_dbg_ibdev_init(&dd->verbs_dev);
 #endif
 
-	idr_preload(GFP_KERNEL);
+	idr_preload(&qib_unit_table, 0, GFP_KERNEL);
 	spin_lock_irqsave(&qib_devs_lock, flags);
 
 	ret = idr_alloc(&qib_unit_table, dd, GFP_NOWAIT);
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 23856c8..d226a64 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -1392,7 +1392,7 @@ static Sg_device *sg_alloc(struct gendisk *disk, struct scsi_device *scsidp)
 		return ERR_PTR(-ENOMEM);
 	}
 
-	idr_preload(GFP_KERNEL);
+	idr_preload(&sg_index_idr, 0, GFP_KERNEL);
 	write_lock_irqsave(&sg_index_lock, iflags);
 
 	error = idr_alloc_range(&sg_index_idr, sdp, 0, SG_MAX_DEVS, GFP_NOWAIT);
diff --git a/fs/dlm/lock.c b/fs/dlm/lock.c
index 75f0421..47edc23 100644
--- a/fs/dlm/lock.c
+++ b/fs/dlm/lock.c
@@ -1199,7 +1199,7 @@ static int create_lkb(struct dlm_ls *ls, struct dlm_lkb **lkb_ret)
 	mutex_init(&lkb->lkb_cb_mutex);
 	INIT_WORK(&lkb->lkb_cb_work, dlm_callback_work);
 
-	idr_preload(GFP_NOFS);
+	idr_preload(&ls->ls_lkbidr, 1, GFP_NOFS);
 	spin_lock(&ls->ls_lkbidr_spin);
 	rv = idr_alloc_range(&ls->ls_lkbidr, lkb, 1, 0, GFP_NOWAIT);
 	if (rv >= 0)
diff --git a/fs/dlm/recover.c b/fs/dlm/recover.c
index 2babe5e..757b7a6 100644
--- a/fs/dlm/recover.c
+++ b/fs/dlm/recover.c
@@ -307,7 +307,7 @@ static int recover_idr_add(struct dlm_rsb *r)
 	struct dlm_ls *ls = r->res_ls;
 	int rv;
 
-	idr_preload(GFP_NOFS);
+	idr_preload(&ls->ls_recover_idr, 1, GFP_NOFS);
 	spin_lock(&ls->ls_recover_idr_lock);
 	if (r->res_id) {
 		rv = -1;
diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
index dd8451d..14ab2da 100644
--- a/fs/nfs/nfs4client.c
+++ b/fs/nfs/nfs4client.c
@@ -30,7 +30,7 @@ static int nfs_get_cb_ident_idr(struct nfs_client *clp, int minorversion)
 
 	if (clp->rpc_ops->version != 4 || minorversion != 0)
 		return ret;
-	idr_preload(GFP_KERNEL);
+	idr_preload(&nn->cb_ident_idr, 0, GFP_KERNEL);
 	spin_lock(&nn->nfs_client_lock);
 	ret = idr_alloc(&nn->cb_ident_idr, clp, GFP_NOWAIT);
 	if (ret >= 0)
diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c
index 60f954a..c6bcf73 100644
--- a/fs/notify/inotify/inotify_user.c
+++ b/fs/notify/inotify/inotify_user.c
@@ -360,7 +360,7 @@ static int inotify_add_to_idr(struct idr *idr, spinlock_t *idr_lock,
 {
 	int ret;
 
-	idr_preload(GFP_KERNEL);
+	idr_preload(idr, 1, GFP_KERNEL);
 	spin_lock(idr_lock);
 
 	ret = idr_alloc_cyclic(idr, i_mark, 1, 0, GFP_NOWAIT);
diff --git a/include/linux/idr.h b/include/linux/idr.h
index 85355d7..418d87c 100644
--- a/include/linux/idr.h
+++ b/include/linux/idr.h
@@ -177,6 +177,7 @@ int idr_for_each(struct idr *idr,
 		 int (*fn)(int id, void *p, void *data), void *data);
 void *idr_replace(struct idr *idr, void *ptr, unsigned id);
 void idr_remove(struct idr *idr, unsigned id);
+int idr_preload(struct idr *idr, unsigned start, gfp_t gfp);
 int idr_alloc_range(struct idr *idr, void *ptr, unsigned start,
 		    unsigned end, gfp_t gfp);
 int idr_alloc_cyclic(struct idr *idr, void *ptr, unsigned start,
@@ -197,41 +198,7 @@ static inline int idr_alloc(struct idr *idr, void *ptr, gfp_t gfp)
  */
 static inline void idr_preload_end(void)
 {
-	radix_tree_preload_end();
-}
-
-/**
- * idr_preload - preload for idr_alloc_range()
- * @gfp: allocation mask to use for preloading
- *
- * Preload per-cpu layer buffer for idr_alloc_range().  Can only be used from
- * process context and each idr_preload() invocation should be matched with
- * idr_preload_end().  Note that preemption is disabled while preloaded.
- *
- * The first idr_alloc_range() in the preloaded section can be treated as if it
- * were invoked with @gfp_mask used for preloading.  This allows using more
- * permissive allocation masks for idrs protected by spinlocks.
- *
- * For example, if idr_alloc_range() below fails, the failure can be treated as
- * if idr_alloc_range() were called with GFP_KERNEL rather than GFP_NOWAIT.
- *
- *	idr_preload(GFP_KERNEL);
- *	spin_lock(lock);
- *
- *	id = idr_alloc_range(idr, ptr, start, end, GFP_NOWAIT);
- *
- *	spin_unlock(lock);
- *	idr_preload_end();
- *	if (id < 0)
- *		error;
- */
-static inline void idr_preload(gfp_t gfp)
-{
-	might_sleep_if(gfp & __GFP_WAIT);
-
-	/* Well this is horrible, but idr_preload doesn't return errors */
-	if (radix_tree_preload(gfp))
-		preempt_disable();
+	preempt_enable();
 }
 
 /* radix tree can't store NULL pointers, so we have to translate...  */
diff --git a/ipc/util.c b/ipc/util.c
index e31ecb8..d6453c1 100644
--- a/ipc/util.c
+++ b/ipc/util.c
@@ -261,7 +261,9 @@ int ipc_addid(struct ipc_ids* ids, struct kern_ipc_perm* new, int size)
 	if (ids->in_use >= size)
 		return -ENOSPC;
 
-	idr_preload(GFP_KERNEL);
+	idr_preload(&ids->ipcs_idr,
+		    (next_id < 0) ? 0 : ipcid_to_idx(next_id),
+		    GFP_KERNEL);
 
 	spin_lock_init(&new->lock);
 	new->deleted = 0;
diff --git a/lib/idr.c b/lib/idr.c
index 89ec59f..fb374c3 100644
--- a/lib/idr.c
+++ b/lib/idr.c
@@ -295,6 +295,23 @@ err:
 	return -ENOMEM;
 }
 
+static int ida_preload(struct ida *ida, unsigned start, gfp_t gfp)
+{
+	int ret = 0;
+	unsigned long flags;
+
+	spin_lock_irqsave(&ida->lock, flags);
+
+	while (!ret &&
+	       (ida->nodes - ida->first_leaf * BITS_PER_LONG <
+		start + ida->allocated_ids + num_possible_cpus()))
+		ret = __ida_resize(ida, gfp, &flags);
+
+	spin_unlock_irqrestore(&ida->lock, flags);
+
+	return ret;
+}
+
 /*
  * Ganged allocation - amortize locking and tree traversal for when we've got
  * another allocator (i.e. a percpu version) acting as a frontend to this code
@@ -1032,6 +1049,55 @@ void idr_remove(struct idr *idr, unsigned id)
 }
 EXPORT_SYMBOL(idr_remove);
 
+/**
+ * idr_preload - preload for idr_alloc_range()
+ * @idr: idr to ensure has room to allocate an id
+ * @start: value that will be passed to ida_alloc_range()
+ * @gfp: allocation mask to use for preloading
+ *
+ * On success, guarantees that one call of idr_alloc()/idr_alloc_range() won't
+ * fail. Returns with preemption disabled; use idr_preload_end() when
+ * finished.
+ *
+ * It's not required to check for failure if you're still checking for
+ * idr_alloc() failure.
+ *
+ * In order to guarantee idr_alloc() won't fail, all allocations from @idr must
+ * make use of idr_preload().
+ */
+int idr_preload(struct idr *idr, unsigned start, gfp_t gfp)
+{
+	int radix_ret, ida_ret = 0;
+
+	might_sleep_if(gfp & __GFP_WAIT);
+
+	while (1) {
+		radix_ret = radix_tree_preload(gfp);
+
+		/*
+		 * Well this is horrible, but radix_tree_preload() doesn't
+		 * disable preemption if it fails, and idr_preload() users don't
+		 * check for errors
+		 */
+		if (radix_ret)
+			preempt_disable();
+
+		/* if ida_preload with GFP_WAIT failed, don't retry */
+		if (ida_ret)
+			break;
+
+		if (!ida_preload(&idr->ida, start, GFP_NOWAIT) ||
+		    !(gfp & __GFP_WAIT))
+			break;
+
+		radix_tree_preload_end();
+		ida_ret = ida_preload(&idr->ida, start, gfp);
+	}
+
+	return radix_ret ?: ida_ret;
+}
+EXPORT_SYMBOL(idr_preload);
+
 static int idr_insert(struct idr *idr, void *ptr, unsigned id,
 		      gfp_t gfp, unsigned long *flags)
 {
-- 
1.8.4.rc1