On Fri, Jul 25, 2014 at 08:41:12AM +1000, Dave Chinner wrote:
> 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?
>
xfs_inobt_rec_exists() sounds good.
> > + 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.
>
Assuming variable sparse chunk granularity, I don't really know the
start ino of the record that potentially covers the new inode chunk.
Given that, we use the smallest possible start ino that could include
this chunk and search forward from there. As you've noted below, I
wasn't relying on failure here to detect the scenario where there is no
existing record.
Brian
> > + 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
>
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
