On Mon, Aug 26, 2024 at 02:38:27PM -0700, Darrick J. Wong wrote: > 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@xxxxxxxxxx> > > > > > > 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@xxxxxxxxxx> > > > --- > > > 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. Ok, can we union the i_projid field (both in memory and in the on-disk structure) so that dual use of the field is well documented by the code? -Dave. -- Dave Chinner david@xxxxxxxxxxxxx