On Sun, 2011-07-10 at 16:49 -0400, Christoph Hellwig wrote:
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
OK, I guess this is fine. I have a suggestion below
but I see nothing truly wrong with what you have.
Reviewed-by: Alex Elder <aelder@xxxxxxx>
> Index: xfs/fs/xfs/xfs_dir2_leaf.c
> ===================================================================
> --- xfs.orig/fs/xfs/xfs_dir2_leaf.c 2011-07-09 12:07:49.518499801 +0200
> +++ xfs/fs/xfs/xfs_dir2_leaf.c 2011-07-09 12:19:48.171796748 +0200
> @@ -148,6 +148,36 @@ xfs_dir2_block_to_leaf(
> return 0;
> }
>
> +STATIC void
> +xfs_dir2_leaf_find_stale(
> + struct xfs_dir2_leaf *leaf,
> + int index,
> + int *lowstale,
> + int *highstale)
> +{
> + /*
> + * Find the first stale entry before our index, if any.
> + */
> + for (*lowstale = index - 1;
> + *lowstale >= 0 &&
> + leaf->ents[*lowstale].address !=
> + cpu_to_be32(XFS_DIR2_NULL_DATAPTR);
> + --*lowstale)
> + continue;
> +
> + /*
> + * Find the first stale entry at or after our index, if any.
> + * Stop if the answer would be worse than lowstale.
Stop if the result would require moving more
entries than using lowstale.
(I realize you didn't change this comment, you just moved
it into this helper function.)
Actually it seems like this searching of the stale
entries and moving things around among them could
be broken into a few even finer-grained utility
routines. It just seems like what this code is
doing is simpler than what the code complexity
suggests (though I haven't really looked at this
stuff much before).
> + */
> + for (*highstale = index;
> + *highstale < be16_to_cpu(leaf->hdr.count) &&
> + leaf->ents[*highstale].address !=
> + cpu_to_be32(XFS_DIR2_NULL_DATAPTR) &&
> + (*lowstale < 0 || index - *lowstale > *highstale - index);
> + ++*highstale)
> + continue;
> +}
> +
> struct xfs_dir2_leaf_entry *
> xfs_dir2_leaf_find_entry(
> xfs_dir2_leaf_t *leaf, /* leaf structure */
. . .
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
