On Tue 29 Apr at 16:33:06 +0200 Nadia.Derbey@bull.net said:

quoted text
> Index: linux-2.6.25-mm1/include/linux/ridr.h
> ===================================================================
> --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6.25-mm1/include/linux/ridr.h	2008-04-29 13:08:00.000000000 +0200
> @@ -0,0 +1,50 @@
> +/*
> + * include/linux/ridr.h
> + *
> + * Small id to pointer translation service avoiding fixed sized
> + * tables. RCU-based implmentation of IDRs.
> + */
                        ^^^^^^^
s/implmentation/reimplementation

Additional comment here might be nice here since this amounts to a mostly
copy of idr.h and its backing implementation, with all the associated
maintenance headaches if both are in tree.

quoted text
> +/*
> + * The idr_layer part should stay at the beginning of the structure
> + */
> +struct ridr_layer {
> +	struct idr_layer idr;
> +	struct rcu_head	 rcu_head;
> +};

Why does it have to be at the beginning?  So an ridr_layer can be used as
an idr_layer?  This doesn't help review.  As previous commenters noted,
review would be a lot easier if this was incremental on idr.

quoted text
> Index: linux-2.6.25-mm1/lib/Makefile
> ===================================================================
> --- linux-2.6.25-mm1.orig/lib/Makefile	2008-04-25 15:29:00.000000000 +0200
> +++ linux-2.6.25-mm1/lib/Makefile	2008-04-25 15:57:39.000000000 +0200
> @@ -6,7 +6,7 @@ lib-y := ctype.o string.o vsprintf.o cmd
>  	 rbtree.o radix-tree.o dump_stack.o \
>  	 idr.o int_sqrt.o extable.o prio_tree.o \
>  	 sha1.o irq_regs.o reciprocal_div.o argv_split.o \
> -	 proportions.o prio_heap.o ratelimit.o
> +	 proportions.o prio_heap.o ratelimit.o ridr.o

I vaguely recall past discussion of preferred insertion point, but
lacking something in CodingStyle and especially given the redundancy
here it could be nice to add ridr.o next to idr.o.

quoted text
> ===================================================================
> --- linux-2.6.25-mm1.orig/lib/idr.c	2008-04-25 17:22:43.000000000 +0200
> +++ linux-2.6.25-mm1/lib/idr.c	2008-04-29 13:08:05.000000000 +0200
> @@ -342,7 +342,7 @@ static void sub_remove(struct idr *idp, 
>  	while ((shift > 0) && p) {
>  		n = (id >> shift) & IDR_MASK;
>  		__clear_bit(n, &p->bitmap);
> -		*++paa = &p->ary[n];
> +		*++paa = (struct idr_layer **) &p->ary[n];
>  		p = p->ary[n];
>  		shift -= IDR_BITS;
>  	}

For shifty use of an ridr_layer as an idr_layer?

quoted text
> Index: linux-2.6.25-mm1/include/linux/idr.h
> ===================================================================
> --- linux-2.6.25-mm1.orig/include/linux/idr.h	2008-04-25 17:22:43.000000000 +0200
> +++ linux-2.6.25-mm1/include/linux/idr.h	2008-04-29 13:08:00.000000000 +0200
> @@ -48,9 +48,9 @@
>  #define IDR_FREE_MAX MAX_LEVEL + MAX_LEVEL
> 
>  struct idr_layer {
> -	unsigned long		 bitmap; /* A zero bit means "space here" */
> -	struct idr_layer	*ary[1<<IDR_BITS];
> -	int			 count;	 /* When zero, we can release it */
> +	unsigned long	 bitmap; /* A zero bit means "space here" */
> +	void		*ary[1<<IDR_BITS];
> +	int		 count;	 /* When zero, we can release it */
>  };
> 
>  struct idr {
> 

This hunk is white space only...

-- 
Tim Pepper  <lnxninja@linux.vnet.ibm.com>
IBM Linux Technology Center
--
unsubscribe notice
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/