On Wed, Jul 13, 2011 at 04:49:36PM +1000, Dave Chinner wrote:
> > > + --*lowstale)
> > > + continue;
>
> Only thing I was conerned about was the indenting on these loops.
> Something like this:
>
> for (*lowstale = index - 1;
> *lowstale >= 0 &&
> leaf->ents[*lowstale].address !=
> cpu_to_be32(XFS_DIR2_NULL_DATAPTR);
> --*lowstale)
> continue;
>
> means that at a glance it is easy to separate the loop control
> statements from the body of the loop just by indentation.
I tried to avoid changing anything here, but now that other people
like me hate these uglies I think I have to ite the bullet and
actually untangle those loops. The version below is what I'm submitting
to testing now:
From: Christoph Hellwig <hch@xxxxxx>
Subject: xfs: factor out xfs_dir2_leaf_find_stale
Signed-off-by: Christoph Hellwig <hch@xxxxxx>
Reviewed-by: Alex Elder <aelder@xxxxxxx>
Reviewed-by: Dave Chinner <dchinner@xxxxxxxxxx>
Index: xfs/fs/xfs/xfs_dir2_leaf.c
===================================================================
--- xfs.orig/fs/xfs/xfs_dir2_leaf.c 2011-07-13 09:00:10.333246566 +0200
+++ xfs/fs/xfs/xfs_dir2_leaf.c 2011-07-13 09:08:52.217085945 +0200
@@ -148,6 +148,38 @@ 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; --*lowstale) {
+ if (leaf->ents[*lowstale].address ==
+ cpu_to_be32(XFS_DIR2_NULL_DATAPTR))
+ break;
+ }
+
+ /*
+ * Find the first stale entry at or after our index, if any.
+ * Stop if the result would require moving more entries than using
+ * lowstale.
+ */
+ for (*highstale = index;
+ *highstale < be16_to_cpu(leaf->hdr.count);
+ ++*highstale) {
+ if (leaf->ents[*highstale].address ==
+ cpu_to_be32(XFS_DIR2_NULL_DATAPTR))
+ break;
+ if (*lowstale >= 0 && index - *lowstale <= *highstale - index)
+ break;
+ }
+}
+
struct xfs_dir2_leaf_entry *
xfs_dir2_leaf_find_entry(
xfs_dir2_leaf_t *leaf, /* leaf structure */
@@ -190,32 +222,8 @@ xfs_dir2_leaf_find_entry(
* If we didn't compact before, we need to find the nearest stale
* entries before and after our insertion point.
*/
- if (compact == 0) {
- /*
- * Find the first stale entry before the insertion point,
- * if any.
- */
- for (lowstale = index - 1;
- lowstale >= 0 &&
- leaf->ents[lowstale].address !=
- cpu_to_be32(XFS_DIR2_NULL_DATAPTR);
- lowstale--)
- continue;
-
- /*
- * Find the next stale entry at or after the insertion point,
- * if any. Stop if we go so far that the lowstale entry
- * would be better.
- */
- 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 - 1 >= highstale - index);
- highstale++)
- continue;
- }
+ if (compact == 0)
+ xfs_dir2_leaf_find_stale(leaf, index, &lowstale, &highstale);
/*
* If the low one is better, use it.
@@ -689,26 +697,9 @@ xfs_dir2_leaf_compact_x1(
leaf = bp->data;
ASSERT(be16_to_cpu(leaf->hdr.stale) > 1);
index = *indexp;
- /*
- * 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.
- */
- 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;
+
+ xfs_dir2_leaf_find_stale(leaf, index, &lowstale, &highstale);
+
/*
* Pick the better of lowstale and highstale.
*/
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
