* [minor fix] radixtree: regulate tag get return value [not found] <20060604084548.GA5609@mail.ustc.edu.cn> @ 2006-06-04 8:45 ` Wu Fengguang 2006-06-04 9:11 ` Andrew Morton 0 siblings, 1 reply; 6+ messages in thread From: Wu Fengguang @ 2006-06-04 8:45 UTC (permalink / raw) To: Andrew Morton; +Cc: linux-kernel, nickpiggin Andrew, this small patch makes the radixtree tester program from http://www.zip.com.au/~akpm/linux/patches/stuff/rtth.tar.gz run OK, with the latest radix tree code in linux-2.6.17-rc5-mm2. It regulates the return value to 0/1 for functions radix_tree_tag_get() and radix_tree_tagged(). Signed-off-by: Wu Fengguang <wfg@mail.ustc.edu.cn> --- --- linux.orig/lib/radix-tree.c +++ linux/lib/radix-tree.c @@ -156,7 +156,7 @@ static inline void tag_clear(struct radi static inline int tag_get(struct radix_tree_node *node, unsigned int tag, int offset) { - return test_bit(offset, node->tags[tag]); + return !! test_bit(offset, node->tags[tag]); } static inline void root_tag_set(struct radix_tree_root *root, unsigned int tag) @@ -177,7 +177,7 @@ static inline void root_tag_clear_all(st static inline int root_tag_get(struct radix_tree_root *root, unsigned int tag) { - return root->gfp_mask & (1 << (tag + __GFP_BITS_SHIFT)); + return !! (root->gfp_mask & (1 << (tag + __GFP_BITS_SHIFT))); } /* ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [minor fix] radixtree: regulate tag get return value 2006-06-04 8:45 ` [minor fix] radixtree: regulate tag get return value Wu Fengguang @ 2006-06-04 9:11 ` Andrew Morton [not found] ` <20060604111754.GA5984@mail.ustc.edu.cn> [not found] ` <20060604112636.GB5984@mail.ustc.edu.cn> 0 siblings, 2 replies; 6+ messages in thread From: Andrew Morton @ 2006-06-04 9:11 UTC (permalink / raw) To: Wu Fengguang; +Cc: linux-kernel, nickpiggin On Sun, 4 Jun 2006 16:45:48 +0800 Wu Fengguang <wfg@mail.ustc.edu.cn> wrote: > Andrew, this small patch makes the radixtree tester program from > http://www.zip.com.au/~akpm/linux/patches/stuff/rtth.tar.gz > run OK, with the latest radix tree code in linux-2.6.17-rc5-mm2. > > It regulates the return value to 0/1 for functions > radix_tree_tag_get() and radix_tree_tagged(). > Well yes. But it slows down the kernel. It would be better to fix rtth. > --- > > --- linux.orig/lib/radix-tree.c > +++ linux/lib/radix-tree.c > @@ -156,7 +156,7 @@ static inline void tag_clear(struct radi > static inline int tag_get(struct radix_tree_node *node, unsigned int tag, > int offset) > { > - return test_bit(offset, node->tags[tag]); > + return !! test_bit(offset, node->tags[tag]); > } test_bit() is (sadly) defined to return 0 or 1. Did this really make a difference? > static inline void root_tag_set(struct radix_tree_root *root, unsigned int tag) > @@ -177,7 +177,7 @@ static inline void root_tag_clear_all(st > > static inline int root_tag_get(struct radix_tree_root *root, unsigned int tag) > { > - return root->gfp_mask & (1 << (tag + __GFP_BITS_SHIFT)); > + return !! (root->gfp_mask & (1 << (tag + __GFP_BITS_SHIFT))); > } > ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <20060604111754.GA5984@mail.ustc.edu.cn>]
* Re: [minor fix] radixtree: regulate tag get return value [not found] ` <20060604111754.GA5984@mail.ustc.edu.cn> @ 2006-06-04 11:17 ` Wu Fengguang 2006-06-04 20:18 ` Andrew Morton 0 siblings, 1 reply; 6+ messages in thread From: Wu Fengguang @ 2006-06-04 11:17 UTC (permalink / raw) To: Andrew Morton; +Cc: linux-kernel, nickpiggin On Sun, Jun 04, 2006 at 02:11:05AM -0700, Andrew Morton wrote: > On Sun, 4 Jun 2006 16:45:48 +0800 > Wu Fengguang <wfg@mail.ustc.edu.cn> wrote: > > > Andrew, this small patch makes the radixtree tester program from > > http://www.zip.com.au/~akpm/linux/patches/stuff/rtth.tar.gz > > run OK, with the latest radix tree code in linux-2.6.17-rc5-mm2. > > > > It regulates the return value to 0/1 for functions > > radix_tree_tag_get() and radix_tree_tagged(). > > > > Well yes. But it slows down the kernel. It would be better to fix rtth. OK. I'll send an updated patch to directly fix the for-rtth radix_tree_tag_get(). > > --- > > > > --- linux.orig/lib/radix-tree.c > > +++ linux/lib/radix-tree.c > > @@ -156,7 +156,7 @@ static inline void tag_clear(struct radi > > static inline int tag_get(struct radix_tree_node *node, unsigned int tag, > > int offset) > > { > > - return test_bit(offset, node->tags[tag]); > > + return !! test_bit(offset, node->tags[tag]); > > } > > test_bit() is (sadly) defined to return 0 or 1. Did this really make a difference? Interesting. I got the following gdb outputs. Note that tag_get() returns -1 and root_tag_get() returns 1048576. (gdb) n 399 while (height > 0) { (gdb) n 402 offset = (index >> shift) & RADIX_TREE_MAP_MASK; (gdb) 403 if (!tag_get(slot, tag, offset)) (gdb) 404 tag_set(slot, tag, offset); (gdb) p tag_get(slot, tag, offset) $14 = 0 (gdb) n 405 slot = slot->slots[offset]; (gdb) p tag_get(slot, tag, offset) $15 = -1 (gdb) n 406 BUG_ON(slot == NULL); (gdb) n 407 shift -= RADIX_TREE_MAP_SHIFT; (gdb) n 408 height--; (gdb) n 399 while (height > 0) { (gdb) n 412 if (slot && !root_tag_get(root, tag)) (gdb) p root_tag_get(root, tag) $16 = 0 (gdb) n 413 root_tag_set(root, tag); (gdb) n 415 return slot; (gdb) p root_tag_get(root, tag) $17 = 1048576 > > static inline void root_tag_set(struct radix_tree_root *root, unsigned int tag) > > @@ -177,7 +177,7 @@ static inline void root_tag_clear_all(st > > > > static inline int root_tag_get(struct radix_tree_root *root, unsigned int tag) > > { > > - return root->gfp_mask & (1 << (tag + __GFP_BITS_SHIFT)); > > + return !! (root->gfp_mask & (1 << (tag + __GFP_BITS_SHIFT))); > > } > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [minor fix] radixtree: regulate tag get return value 2006-06-04 11:17 ` Wu Fengguang @ 2006-06-04 20:18 ` Andrew Morton 0 siblings, 0 replies; 6+ messages in thread From: Andrew Morton @ 2006-06-04 20:18 UTC (permalink / raw) To: Wu Fengguang; +Cc: linux-kernel, nickpiggin On Sun, 4 Jun 2006 19:17:54 +0800 Wu Fengguang <wfg@mail.ustc.edu.cn> wrote: > > test_bit() is (sadly) defined to return 0 or 1. Did this really make a difference? > > Interesting. I got the following gdb outputs. Note that tag_get() > returns -1 and root_tag_get() returns 1048576. > > (gdb) n > 399 while (height > 0) { > (gdb) n > 402 offset = (index >> shift) & RADIX_TREE_MAP_MASK; > (gdb) > 403 if (!tag_get(slot, tag, offset)) > (gdb) > 404 tag_set(slot, tag, offset); > (gdb) p tag_get(slot, tag, offset) > $14 = 0 > (gdb) n > 405 slot = slot->slots[offset]; > (gdb) p tag_get(slot, tag, offset) > $15 = -1 You trust gdb more than I do ;) It's doing a pretty tricky thing there. test_bit() returns (1 & (expr)) - it _has_ to return 0 or 1. ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <20060604112636.GB5984@mail.ustc.edu.cn>]
* [PATCH] radixtree: normalize radix_tree_tag_get() return value [not found] ` <20060604112636.GB5984@mail.ustc.edu.cn> @ 2006-06-04 11:26 ` Wu Fengguang 0 siblings, 0 replies; 6+ messages in thread From: Wu Fengguang @ 2006-06-04 11:26 UTC (permalink / raw) To: Andrew Morton; +Cc: linux-kernel, nickpiggin In radix_tree_tag_get(), return normalized value of 0/1, as indicated by its comment. Signed-off-by: Wu Fengguang <wfg@mail.ustc.edu.cn> --- --- linux-2.6.17-rc5-mm2.orig/lib/radix-tree.c +++ linux-2.6.17-rc5-mm2/lib/radix-tree.c @@ -531,7 +531,7 @@ int radix_tree_tag_get(struct radix_tree int ret = tag_get(slot, tag, offset); BUG_ON(ret && saw_unset_tag); - return ret; + return !! ret; } slot = slot->slots[offset]; shift -= RADIX_TREE_MAP_SHIFT; ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [minor fix] radixtree: regulate tag get return value
@ 2006-06-05 3:07 Chuck Ebbert
0 siblings, 0 replies; 6+ messages in thread
From: Chuck Ebbert @ 2006-06-05 3:07 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-kernel, Wu Fengguang
In-Reply-To: <20060604131824.e2d1c934.akpm@osdl.org>
On Sun, 4 Jun 2006 13:18:24 -0700, Andrew Morton wrote:
> test_bit() returns (1 & (expr)) - it _has_ to return 0 or 1.
On i386, test_bit(nr, addr) will return 0 or 1 only when nr is
a constant -- it uses (expr != 0).
If nr is a variable it will return either 0 or -1 because it uses:
__asm__ __volatile__(
"btl %2,%1\n\tsbbl %0,%0"
:"=r" (oldbit)
:"m" (*addr),"Ir" (nr));
return oldbit;
btl %2,%1 sets the carry flag if the selected bit is set.
sbbl %0,%0 subtracts a register from itself and then subtracts one
from the result if the carry flag is set.
--
Chuck
^ permalink raw reply [flat|nested] 6+ messages in threadend of thread, other threads:[~2006-06-05 3:12 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20060604084548.GA5609@mail.ustc.edu.cn>
2006-06-04 8:45 ` [minor fix] radixtree: regulate tag get return value Wu Fengguang
2006-06-04 9:11 ` Andrew Morton
[not found] ` <20060604111754.GA5984@mail.ustc.edu.cn>
2006-06-04 11:17 ` Wu Fengguang
2006-06-04 20:18 ` Andrew Morton
[not found] ` <20060604112636.GB5984@mail.ustc.edu.cn>
2006-06-04 11:26 ` [PATCH] radixtree: normalize radix_tree_tag_get() " Wu Fengguang
2006-06-05 3:07 [minor fix] radixtree: regulate tag get " Chuck Ebbert
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox