mm gang: you goofed!

mm gang: you goofed!

[Hugh Dickins]

I've not actually seen truncate_inode_pages' "I goofed!" message
(except when I added further debug to cover lstart != 0 truncation,
which fsx is keen on), but I think it could occur even in that case.

It seems that __lookup() can easily return 0, even though there's
more to find, if it starts partway into a RADIX_TREE_MAP_SIZE
array, in which the only occupied slots are before it starts.

Patch below seems to fix it, but I bet you can improve upon it.

Might this relate to Trond/Chuck's NFS invalidation woes?
It's certainly the main contributor to my tmpfs problems.

I look forward to your "erk" - or will you delight us with
some other exclamation?!

Hugh

--- 2.5.36-mm1/lib/radix-tree.c	Tue Sep 17 11:04:08 2002
+++ linux/lib/radix-tree.c	Wed Sep 18 19:46:10 2002
@@ -231,8 +231,6 @@ __lookup(struct radix_tree_root *root, v
 	unsigned int height = root->height;
 	struct radix_tree_node *slot;
 
-	if (index > max_index)
-		return 0;
 	shift = (height-1) * RADIX_TREE_MAP_SHIFT;
 	slot = root->rnode;
 
@@ -307,9 +305,11 @@ radix_tree_gang_lookup(struct radix_tree
 		unsigned int nr_found;
 		unsigned long next_index;	/* Index of next search */
 
+		if (cur_index > max_index)
+			break;
 		nr_found = __lookup(root, results + ret, cur_index,
 				max_items - ret, &next_index, max_index);
-		if (nr_found == 0)
+		if (nr_found == 0 && !(cur_index & RADIX_TREE_MAP_MASK))
 			break;
 		ret += nr_found;
 		cur_index = next_index;

Re: mm gang: you goofed!

[Andrew Morton]

Hugh Dickins wrote:
> 
> ...
> It seems that __lookup() can easily return 0, even though there's
> more to find, if it starts partway into a RADIX_TREE_MAP_SIZE
> array, in which the only occupied slots are before it starts.

erk.  That sounds right.

I don't think the max_index test matters much - if we ever
hit that index then something has gone horridly wrong, because
it's out-of-bounds for the tree height.  But whatever.

> Patch below seems to fix it

But your patch only fixes the symptoms of the bug.  You should
fix the real bug.  But to do that you'd need to find me, so this
will have to do for now.

> but I bet you can improve upon it.

Well the colour scheme is a bit gaudy.
 
> Might this relate to Trond/Chuck's NFS invalidation woes?

Nope; this code is only in the mm crash-test-dummy tree at present.
And it's currently at the "wow, it worked first time" stage.  I need
to get down and write some special test code for it, and to run
fsx-linux.  Possibly fsx-linux has sufficient coverage.

> It's certainly the main contributor to my tmpfs problems.

Sorry about that.
 
> I look forward to your "erk" - or will you delight us with
> some other exclamation?!

gack?