[PATCH 028/119] xfs: define the on-disk rmap btree format

[Darrick J. Wong]

On Fri, Jul 08, 2016 at 09:14:55AM +1000, Dave Chinner wrote:
> On Thu, Jul 07, 2016 at 12:18:13PM -0700, Darrick J. Wong wrote:
> > On Thu, Jul 07, 2016 at 02:41:56PM -0400, Brian Foster wrote:
> > > > +	if (!xfs_sb_version_hasrmapbt(&mp->m_sb))
> > > > +		return false;
> > > > +	if (!xfs_btree_sblock_v5hdr_verify(bp))
> > > > +		return false;
> > > > +
> > > > +	level = be16_to_cpu(block->bb_level);
> > > > +	if (pag && pag->pagf_init) {
> > > > +		if (level >= pag->pagf_levels[XFS_BTNUM_RMAPi])
> > > > +			return false;
> > > > +	} else if (level >= mp->m_rmap_maxlevels)
> > > > +		return false;
> > > 
> > > It looks like the above (level >= mp->m_rmap_maxlevels) check could be
> > > independent (rather than an 'else). Otherwise looks good:
> > 
> > Hmmm.... at first I wondered, "Shouldn't we have already checked that
> > pag->pagf_levels[XFS_BTNUM_RMAPi] <= mp->m_rmap_maxlevels?"  But then I
> > realized that no, we don't do that anywhere.  Nor does the bnobt/cntbt
> > verifier.  Am I missing something?

(Yes, I am.)

> It should have been ranged checked when the AGF is first read in
> (i.e. in the verifier), in ASSERTS every time xfs_alloc_read_agf()
> is called after initialisation, and then every time the verifier is
> run on write of the AGF.

You're right.  I missed that. :(

> > I did see that we at least check the AGF/AGI levels to make sure they don't
> > overflow XFS_BTREE_MAXLEVELS, so we're probably fine here.
> 
> Precisely - if the AGF verifier doesn't have a max level check in it
> for the rmapbt, then we need to add one there.

There's a check there, so we're fine.

--D

> 
> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david at fromorbit.com