* [minor fix] radixtree: regulate tag get return value
@ 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* [minor fix] radixtree: regulate tag get return value
@ 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 ` Wu Fengguang
@ 2006-06-04 9:11 ` Andrew Morton
2006-06-04 11:17 ` Wu Fengguang
2006-06-04 11:26 ` Wu Fengguang
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* 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, 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, 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
* [PATCH] radixtree: normalize radix_tree_tag_get() return value
@ 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
* [PATCH] radixtree: normalize radix_tree_tag_get() return value
@ 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 thread
end 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 --
2006-06-04 8:45 [minor fix] radixtree: regulate tag get return value Wu Fengguang
2006-06-04 8:45 ` Wu Fengguang
2006-06-04 9:11 ` Andrew Morton
2006-06-04 11:17 ` Wu Fengguang
2006-06-04 11:17 ` Wu Fengguang
2006-06-04 20:18 ` Andrew Morton
2006-06-04 11:26 ` [PATCH] radixtree: normalize radix_tree_tag_get() " Wu Fengguang
2006-06-04 11:26 ` Wu Fengguang
-- strict thread matches above, loose matches on Subject: below --
2006-06-05 3:07 [minor fix] radixtree: regulate tag get " Chuck Ebbert
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.