On Thu, Jun 08, 2017 at 09:02:26AM -0400, Brian Foster wrote:
> On Fri, Jun 02, 2017 at 02:25:08PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> >
> > Provide a way to calculate the highest hash value of a leaf1 block.
> > This will be used by the directory scrubbing code to check the sanity
> > of hashes in leaf1 directory blocks.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> > ---
> > fs/xfs/libxfs/xfs_dir2_node.c | 28 ++++++++++++++++++++++++++++
> > fs/xfs/libxfs/xfs_dir2_priv.h | 2 ++
> > 2 files changed, 30 insertions(+)
> >
> >
> > diff --git a/fs/xfs/libxfs/xfs_dir2_node.c b/fs/xfs/libxfs/xfs_dir2_node.c
> > index bbd1238..15c1881 100644
> > --- a/fs/xfs/libxfs/xfs_dir2_node.c
> > +++ b/fs/xfs/libxfs/xfs_dir2_node.c
> > @@ -524,6 +524,34 @@ xfs_dir2_free_hdr_check(
> > #endif /* DEBUG */
> >
> > /*
> > + * Return the last hash value in the leaf1.
> > + * Stale entries are ok.
> > + */
> > +xfs_dahash_t /* hash value */
> > +xfs_dir2_leaf1_lasthash(
> > + struct xfs_inode *dp,
> > + struct xfs_buf *bp, /* leaf buffer */
> > + int *count) /* count of entries in leaf */
> > +{
> > + struct xfs_dir2_leaf *leaf = bp->b_addr;
> > + struct xfs_dir2_leaf_entry *ents;
> > + struct xfs_dir3_icleaf_hdr leafhdr;
> > +
> > + dp->d_ops->leaf_hdr_from_disk(&leafhdr, leaf);
> > +
> > + ASSERT(leafhdr.magic == XFS_DIR2_LEAF1_MAGIC ||
> > + leafhdr.magic == XFS_DIR3_LEAF1_MAGIC);
> > +
>
> It looks like the assert is the only difference between this function
> and xfs_dir2_leafn_lasthash(). It seems like overkill to me to duplicate
> just for that. How about we fix up the assert to cover the additional
> magics (and maybe rename _leafn_lasthash() to _leaf_lasthash() if
> appropriate)?
>
> Actually, taking a closer look, ->leaf_hdr_from_disk() already asserts
> on the appropriate LEAF1/LEAFN magic based on the callback that is
> specified. ISTM that we could also just kill the _lasthash() assert.
The ASSERTs in the _lasthash functions and in leaf_hdr_from_disk aren't
testing quite the same things. The asserts in _dir2_leaf[1n]_lasthash
check that we actually passed it a leaf1 or leafn block, respectively.
The asserts in _dir[23]_leaf_hdr_from_disk check that we actually fed it
a dir2 or dir3 leaf* block without caring whether it's leaf1 or leafn.
That's why I didn't just get rid of the assert and rename the function
xfs_dir2_leaf_lasthash().
I suppose we could just make a single parent function that takes the two
magics it wants to see and have _dir2_leaf[1n]_lasthash call the parent
function with the magic numbers they want to check. How does that
sound?
--D
>
> Brian
>
> > + if (count)
> > + *count = leafhdr.count;
> > + if (!leafhdr.count)
> > + return 0;
> > +
> > + ents = dp->d_ops->leaf_ents_p(leaf);
> > + return be32_to_cpu(ents[leafhdr.count - 1].hashval);
> > +}
> > +
> > +/*
> > * Return the last hash value in the leaf.
> > * Stale entries are ok.
> > */
> > diff --git a/fs/xfs/libxfs/xfs_dir2_priv.h b/fs/xfs/libxfs/xfs_dir2_priv.h
> > index 576f2d2..c09bca1 100644
> > --- a/fs/xfs/libxfs/xfs_dir2_priv.h
> > +++ b/fs/xfs/libxfs/xfs_dir2_priv.h
> > @@ -95,6 +95,8 @@ extern bool xfs_dir3_leaf_check_int(struct xfs_mount *mp, struct xfs_inode *dp,
> > /* xfs_dir2_node.c */
> > extern int xfs_dir2_leaf_to_node(struct xfs_da_args *args,
> > struct xfs_buf *lbp);
> > +extern xfs_dahash_t xfs_dir2_leaf1_lasthash(struct xfs_inode *dp,
> > + struct xfs_buf *bp, int *count);
> > extern xfs_dahash_t xfs_dir2_leafn_lasthash(struct xfs_inode *dp,
> > struct xfs_buf *bp, int *count);
> > extern int xfs_dir2_leafn_lookup_int(struct xfs_buf *bp,
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
