On Tue, Feb 04, 2014 at 12:49:34PM -0500, Brian Foster wrote:
> Define the AGI fields for the finobt root/level and add magic
> numbers. Update the btree code to add support for the new
> XFS_BTNUM_FINOBT inode btree.
>
> The finobt root block is reserved immediately following the inobt
> root block in the AG. Update XFS_PREALLOC_BLOCKS() to determine the
> starting AG data block based on whether finobt support is enabled.
>
> Signed-off-by: Brian Foster <bfoster@xxxxxxxxxx>
> ---
> fs/xfs/xfs_ag.h | 32 +++++++++++++++----------
> fs/xfs/xfs_btree.c | 6 +++--
> fs/xfs/xfs_btree.h | 3 +++
> fs/xfs/xfs_format.h | 14 ++++++++++-
> fs/xfs/xfs_ialloc.c | 37 +++++++++++++++++++++++++----
> fs/xfs/xfs_ialloc_btree.c | 60 +++++++++++++++++++++++++++++++++++++++++++++--
> fs/xfs/xfs_log_recover.c | 2 ++
> fs/xfs/xfs_stats.c | 1 +
> fs/xfs/xfs_stats.h | 18 +++++++++++++-
> fs/xfs/xfs_types.h | 2 +-
> 10 files changed, 150 insertions(+), 25 deletions(-)
>
> diff --git a/fs/xfs/xfs_ag.h b/fs/xfs/xfs_ag.h
> index 3fc1098..5d3011f 100644
> --- a/fs/xfs/xfs_ag.h
> +++ b/fs/xfs/xfs_ag.h
> @@ -164,22 +164,28 @@ typedef struct xfs_agi {
> __be32 agi_pad32;
> __be64 agi_lsn; /* last write sequence */
>
> + __be32 agi_free_root; /* root of the free inode btree */
> + __be32 agi_free_level;/* levels in free inode btree */
> +
> /* structure must be padded to 64 bit alignment */
> } xfs_agi_t;
Can you add comments to the agi structure that indicate where the
contiguous logging regions start and end?
.....
> #ifdef DEBUG
> @@ -1515,14 +1517,39 @@ xfs_ialloc_log_agi(
> ASSERT(agi->agi_magicnum == cpu_to_be32(XFS_AGI_MAGIC));
> #endif
> /*
> - * Compute byte offsets for the first and last fields.
> + * The growth of the agi buffer over time now requires that we interpret
> + * the buffer as two logical regions delineated at the end of the unlinked
> + * list. This is due to the size of the hash table and its location in the
> + * middle of the agi.
> + *
> + * For example, a request to log a field before agi_unlinked and a field
> + * after agi_unlinked could cause us to log the entire hash table and use
> + * an excessive amount of log space. To avoid this behavior, log the
> + * region up through agi_unlinked in one call and the region after
> + * agi_unlinked through the end of the structure in another.
> */
This comment should be at the head of the function as it describes
the constraints the function works under, rather than explaining
particular piece of the code.
....
> @@ -334,11 +384,17 @@ xfs_inobt_init_cursor(
>
> cur->bc_tp = tp;
> cur->bc_mp = mp;
> - cur->bc_nlevels = be32_to_cpu(agi->agi_level);
> cur->bc_btnum = btnum;
> + if (btnum == XFS_BTNUM_INO) {
> + cur->bc_nlevels = be32_to_cpu(agi->agi_level);
> + cur->bc_ops = &xfs_inobt_ops;
> + } else {
> + cur->bc_nlevels = be32_to_cpu(agi->agi_free_level);
> + cur->bc_ops = &xfs_finobt_ops;
ASSERT(btnum == XFS_BTNUM_FINO);
....
Otherwise looks pretty good.
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
