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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Aug 26, 2024 at 06:56:58PM -0700, Darrick J. Wong wrote:
> On Tue, Aug 27, 2024 at 10:58:59AM +1000, Dave Chinner wrote:
> > 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?
> 
> Sounds good to me.  Does
> 
> union {
> 	xfs_prid_t	i_projid;
> 	uint32_t	i_metagroup;
> };
> 
> sound ok?

Yup.

-- 
Dave Chinner
david@xxxxxxxxxxxxx




[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux