Re: [PATCH 13/24] xfs: add a lockdep class key for rtgroup inodes

Darrick J. Wong
2024-08-26T21:38:28Z

On Mon, Aug 26, 2024 at 09:58:05AM +1000, Dave Chinner wrote:
> On Thu, Aug 22, 2024 at 05:18:02PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> >
> > Add a dynamic lockdep class key for rtgroup inodes. This will enable
> > lockdep to deduce inconsistencies in the rtgroup metadata ILOCK locking
> > order. Each class can have 8 subclasses, and for now we will only have
> > 2 inodes per group. This enables rtgroup order and inode order checks
> > when nesting ILOCKs.
> >
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> > fs/xfs/libxfs/xfs_rtgroup.c | 52 +++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 52 insertions(+)
> >
> >
> > diff --git a/fs/xfs/libxfs/xfs_rtgroup.c b/fs/xfs/libxfs/xfs_rtgroup.c
> > index 51f04cad5227c..ae6d67c673b1a 100644
> > --- a/fs/xfs/libxfs/xfs_rtgroup.c
> > +++ b/fs/xfs/libxfs/xfs_rtgroup.c
> > @@ -243,3 +243,55 @@ xfs_rtgroup_trans_join(
> > if (rtglock_flags & XFS_RTGLOCK_BITMAP)
> > xfs_rtbitmap_trans_join(tp);
> > }
> > +
> > +#ifdef CONFIG_PROVE_LOCKING
> > +static struct lock_class_key xfs_rtginode_lock_class;
> > +
> > +static int
> > +xfs_rtginode_ilock_cmp_fn(
> > + const struct lockdep_map *m1,
> > + const struct lockdep_map *m2)
> > +{
> > + const struct xfs_inode *ip1 =
> > + container_of(m1, struct xfs_inode, i_lock.dep_map);
> > + const struct xfs_inode *ip2 =
> > + container_of(m2, struct xfs_inode, i_lock.dep_map);
> > +
> > + if (ip1->i_projid < ip2->i_projid)
> > + return -1;
> > + if (ip1->i_projid > ip2->i_projid)
> > + return 1;
> > + return 0;
> > +}
>
> What's the project ID of the inode got to do with realtime groups?

Each rtgroup metadata file stores its group number in i_projid so that
mount can detect if there's a corruption in /rtgroup and we just opened
the bitmap from the wrong group.

We can also use lockdep to detect code that locks rtgroup metadata in
the wrong order. Potentially we could use this _cmp_fn to enforce that
we always ILOCK in the order bitmap -> summary -> rmap -> refcount based
on i_metatype.

--D

> -Dave.
> --
> Dave Chinner
> david@fromorbit.com
>