xfs
[Top] [All Lists]

Re: [PATCH 1/3] libxfs: reintroduce old xfs_repair radix-tree code

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 1/3] libxfs: reintroduce old xfs_repair radix-tree code
From: Alex Elder <aelder@xxxxxxx>
Date: Wed, 09 Feb 2011 12:05:13 -0600
Cc: xfs@xxxxxxxxxxx
In-reply-to: <1294649091-27174-2-git-send-email-david@xxxxxxxxxxxxx>
References: <1294649091-27174-1-git-send-email-david@xxxxxxxxxxxxx> <1294649091-27174-2-git-send-email-david@xxxxxxxxxxxxx>
Reply-to: aelder@xxxxxxx
On Mon, 2011-01-10 at 19:44 +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
> 
> The current kernel code uses radix trees more widely than the
> previous code, so for the next sync we need radix tree support in
> libxfs. Pull the old radix tree code out the xfs_repair git history
> and move it into libxfs to simplify the kernel code sync.
> 
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>

OK, I actually reviewed this code, even though it had
already been present in the source tree prior to commit:
379397bf9... ("repair: use a btree instead of a radix tree
for the prefetch queue").

And I have some suggestions, and I have at least one
thing that I think is a bug.

I also notice that this code apparently formed the
basis of the kernel's implementation.  That's good.
It's probably worth reviewing the kernel version's
history to see if there are any bug fixes that ought
to be brought back into this code (and vice-versa).


All that being said, I think the right thing to do
is to include this change as-is as a commit.  It
includes both "radix-tree.c" and "radix-tree.h" as
identical copies of what was removed (though each
now resides in a different directory from before),
thereby preserving the provenance of the code.

Then, after it's committed, I can offer my suggested
changes, or even just implement and propose them
myself.

So unless you disagree with this approach I think
it's fine to commit it as you originally posted it.

Reviewed-by: Alex Elder <aelder@xxxxxxx>



<Prev in Thread] Current Thread [Next in Thread>