xfs
[Top] [All Lists]

Re: [PATCH 08/18] xfs: create helper to manage record overlap for sparse

To: Brian Foster <bfoster@xxxxxxxxxx>
Subject: Re: [PATCH 08/18] xfs: create helper to manage record overlap for sparse inode chunks
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Fri, 25 Jul 2014 08:41:12 +1000
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1406211788-63206-9-git-send-email-bfoster@xxxxxxxxxx>
References: <1406211788-63206-1-git-send-email-bfoster@xxxxxxxxxx> <1406211788-63206-9-git-send-email-bfoster@xxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Thu, Jul 24, 2014 at 10:22:58AM -0400, Brian Foster wrote:
> Create xfs_spchunk_has_record() to receive the parameters of a new
> sparse inode chunk allocation and identify whether a record exists that
> is capable of tracking this sparse chunk.
> 
> Signed-off-by: Brian Foster <bfoster@xxxxxxxxxx>
> ---
>  fs/xfs/libxfs/xfs_ialloc.c | 55 
> ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
> 
> diff --git a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c
> index 27d3437..be57b51 100644
> --- a/fs/xfs/libxfs/xfs_ialloc.c
> +++ b/fs/xfs/libxfs/xfs_ialloc.c
> @@ -351,6 +351,61 @@ xfs_ialloc_inode_init(
>  }
>  
>  /*
> + * Determine whether part of a sparse inode chunk that has just been 
> allocated
> + * is covered by an existing inobt record.
> + */
> +STATIC int
> +xfs_spchunk_has_record(

not sure I like the "spchunk" naming. I see that and I have no idea
what subsystem it belongs to. It's actually an inobt lookup
function, and doesn't really have anything to do with sparse chunks.
So maybe xfs_inobt_rec_exists or xfs_inobt_lookup_exact?

> +     struct xfs_mount                *mp,
> +     struct xfs_trans                *tp,
> +     struct xfs_buf                  *agbp,
> +     xfs_agino_t                     newino,
> +     xfs_agino_t                     count,
> +     xfs_btnum_t                     btnum,
> +     struct xfs_inobt_rec_incore     *orec)
> +{
> +     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                     previno;
> +     int                             error;
> +     int                             i;
> +     struct xfs_inobt_rec_incore     rec;
> +
> +     orec->ir_startino = NULLAGINO;
> +
> +     cur = xfs_inobt_init_cursor(mp, tp, agbp, agno, btnum);
> +
> +     previno = newino + count - XFS_INODES_PER_CHUNK;
> +     error = xfs_inobt_lookup(cur, previno, XFS_LOOKUP_GE, &i);

You want XFS_LOOKUP_EQ, yes? i.e. XFS_LOOKUP_GE won't fail if the
exact record for the inode chunk does not exist - it will return the
next one in the btree.

> +     if (error)
> +             goto error;
> +     if (i == 0)
> +             goto out;
> +
> +     error = xfs_inobt_get_rec(cur, &rec, &i);
> +     if (error)
> +             goto error;
> +     XFS_WANT_CORRUPTED_GOTO(i == 1, error);
> +
> +     if (rec.ir_startino > newino)
> +             goto out;

And so this check would not be necessary...

> +
> +     ASSERT(rec.ir_startino <= newino &&
> +            rec.ir_startino + XFS_INODES_PER_CHUNK > newino);
> +     ASSERT(rec.ir_freecount + count <= XFS_INODES_PER_CHUNK);
> +
> +     *orec = rec;

/* struct copy */

-Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx

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