Re: [PATCH 6/9] Make idr_find rcu-safe

From: Paul E. McKenney
Date: Fri May 30 2008 - 04:24:52 EST


On Wed, May 07, 2008 at 01:35:59PM +0200, Nadia.Derbey@xxxxxxxx wrote:
> [PATCH 06/09]
>
> This is a patch that makes idr_find rcu-safe: it can now be called inside an
> rcu_read critical section.

Reviewed-by: Paul E. McKenney <paulmck@xxxxxxxxxxxxxxxxxx>

> Signed-off-by: Nadia Derbey <Nadia.Derbey@xxxxxxxx>
>
> ---
> include/linux/idr.h | 16 ++++++++++++++++
> lib/idr.c | 11 ++++++-----
> 2 files changed, 22 insertions(+), 5 deletions(-)
>
> Index: linux-2.6.25-mm1/lib/idr.c
> ===================================================================
> --- linux-2.6.25-mm1.orig/lib/idr.c 2008-05-06 18:06:58.000000000 +0200
> +++ linux-2.6.25-mm1/lib/idr.c 2008-05-07 10:38:15.000000000 +0200
> @@ -459,7 +459,8 @@ EXPORT_SYMBOL(idr_destroy);
> * return indicates that @id is not valid or you passed %NULL in
> * idr_get_new().
> *
> - * The caller must serialize idr_find() vs idr_get_new() and idr_remove().
> + * This function can be called under rcu_read_lock(), given that the leaf
> + * pointers lifetimes are correctly managed.
> */
> void *idr_find(struct idr *idp, int id)
> {
> @@ -467,7 +468,7 @@ void *idr_find(struct idr *idp, int id)
> struct idr_layer *p;
>
> n = idp->layers * IDR_BITS;
> - p = idp->top;
> + p = rcu_dereference(idp->top);
>
> /* Mask off upper bits we don't use for the search. */
> id &= MAX_ID_MASK;
> @@ -477,7 +478,7 @@ void *idr_find(struct idr *idp, int id)
>
> while (n > 0 && p) {
> n -= IDR_BITS;
> - p = p->ary[(id >> n) & IDR_MASK];
> + p = rcu_dereference(p->ary[(id >> n) & IDR_MASK]);
> }
> return((void *)p);
> }
> @@ -510,7 +511,7 @@ int idr_for_each(struct idr *idp,
> struct idr_layer **paa = &pa[0];
>
> n = idp->layers * IDR_BITS;
> - p = idp->top;
> + p = rcu_dereference(idp->top);
> max = 1 << n;
>
> id = 0;
> @@ -518,7 +519,7 @@ int idr_for_each(struct idr *idp,
> while (n > 0 && p) {
> n -= IDR_BITS;
> *paa++ = p;
> - p = p->ary[(id >> n) & IDR_MASK];
> + p = rcu_dereference(p->ary[(id >> n) & IDR_MASK]);
> }
>
> if (p) {
> Index: linux-2.6.25-mm1/include/linux/idr.h
> ===================================================================
> --- linux-2.6.25-mm1.orig/include/linux/idr.h 2008-05-06 17:38:42.000000000 +0200
> +++ linux-2.6.25-mm1/include/linux/idr.h 2008-05-07 10:41:22.000000000 +0200
> @@ -79,6 +79,22 @@ struct idr {
>
> #define _idr_rc_to_errno(rc) ((rc) == -1 ? -EAGAIN : -ENOSPC)
>
> +/**
> + * idr synchronization (stolen from radix-tree.h)
> + *
> + * idr_find() is able to be called locklessly, using RCU. The caller must
> + * ensure calls to this function are made within rcu_read_lock() regions.
> + * Other readers (lock-free or otherwise) and modifications may be running
> + * concurrently.
> + *
> + * It is still required that the caller manage the synchronization and
> + * lifetimes of the items. So if RCU lock-free lookups are used, typically
> + * this would mean that the items have their own locks, or are amenable to
> + * lock-free access; and that the items are freed by RCU (or only freed after
> + * having been deleted from the idr tree *and* a synchronize_rcu() grace
> + * period).
> + */
> +
> /*
> * This is what we export.
> */
>
> --
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/