xfs
[Top] [All Lists]

Re: [RFC PATCH 08/11] xfs: insert newly allocated inode chunks into the

To: Brian Foster <bfoster@xxxxxxxxxx>
Subject: Re: [RFC PATCH 08/11] xfs: insert newly allocated inode chunks into the finobt
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Thu, 5 Sep 2013 12:10:35 +1000
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1378232708-57156-9-git-send-email-bfoster@xxxxxxxxxx>
References: <1378232708-57156-1-git-send-email-bfoster@xxxxxxxxxx> <1378232708-57156-9-git-send-email-bfoster@xxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Tue, Sep 03, 2013 at 02:25:05PM -0400, Brian Foster wrote:
> A newly allocated inode chunk, by definition, has at least one
> free inode, so a record is always inserted into the finobt.
> 
> Create the xfs_inobt_insert() helper from existing code to insert
> a record in an inobt based on the provided BTNUM. Update
> xfs_ialloc_ag_alloc() to invoke the helper for the existing
> XFS_BTNUM_INO tree and XFS_BTNUM_FINO tree, if enabled.

Factoring is good...

> Signed-off-by: Brian Foster <bfoster@xxxxxxxxxx>
> ---
>  fs/xfs/xfs_ialloc.c | 79 
> +++++++++++++++++++++++++++++++++++++----------------
>  1 file changed, 56 insertions(+), 23 deletions(-)
> 
> diff --git a/fs/xfs/xfs_ialloc.c b/fs/xfs/xfs_ialloc.c
> index 5ced506..e64a728 100644
> --- a/fs/xfs/xfs_ialloc.c
> +++ b/fs/xfs/xfs_ialloc.c
> @@ -152,6 +152,52 @@ xfs_check_agi_freecount(
>  #endif
>  
>  /*
> + * Insert records describing a newly allocated inode chunk into the inobt.
> + */
> +STATIC int
> +xfs_inobt_insert(
> +     struct xfs_mount        *mp,
> +     struct xfs_trans        *tp,
> +     struct xfs_buf          *agbp,
> +     xfs_agino_t             newino,
> +     xfs_agino_t             newlen,
> +     xfs_btnum_t             btnum)
> +{
> +     struct xfs_btree_cur    *cur;
> +     struct xfs_agi          *agi = XFS_BUF_TO_AGI(agbp);
> +     xfs_agnumber_t          agno = be32_to_cpu(agi->agi_seqno);
> +     xfs_agino_t             thisino;
> +     int                     i;
> +     int                     error;
> +
> +     cur = xfs_inobt_init_cursor(mp, tp, agbp, agno, btnum);
> +
> +     for (thisino = newino;
> +          thisino < newino + newlen;
> +          thisino += XFS_INODES_PER_CHUNK) {
> +             cur->bc_rec.i.ir_startino = thisino;
> +             cur->bc_rec.i.ir_freecount = XFS_INODES_PER_CHUNK;
> +             cur->bc_rec.i.ir_free = XFS_INOBT_ALL_FREE;
> +             error = xfs_btree_lookup(cur, XFS_LOOKUP_EQ, &i);
> +             if (error) {
> +                     xfs_btree_del_cursor(cur, XFS_BTREE_ERROR);
> +                     return error;
> +             }

I'm wondering if we'd do better to pass freecount/free to
xfs_inobt_lookup() and call that instead. i.e. we don't need to
expose the btree cursor internals here...

Maybe, also, move this function up to the top of the file with the
other xfs_inobt_* functions.

Otherwise it all looks OK.

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

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