Re: [PATCH 5/3] xfs: replace inode fork size macros with functions

["Darrick J. Wong" <djwong@kernel.org>]

On Wed, Jul 06, 2022 at 10:25:55AM +1000, Dave Chinner wrote:
> On Tue, Jul 05, 2022 at 04:19:24PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > Replace the shouty macros here with typechecked helper functions.
> > 
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> 
> looks good.
> 
> Reviewed-by: Dave Chinner <dchinner@redhat.com>
> 
> .....
> > diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
> > index 2badbf9bb80d..7ff828504b3c 100644
> > --- a/fs/xfs/xfs_inode.h
> > +++ b/fs/xfs/xfs_inode.h
> > @@ -102,6 +102,41 @@ xfs_ifork_ptr(
> >  	}
> >  }
> >  
> > +static inline unsigned int xfs_inode_fork_boff(struct xfs_inode *ip)
> > +{
> > +	return ip->i_forkoff << 3;
> > +}
> > +
> > +static inline unsigned int xfs_inode_data_fork_size(struct xfs_inode *ip)
> > +{
> > +	if (xfs_inode_has_attr_fork(ip))
> > +		return xfs_inode_fork_boff(ip);
> > +
> > +	return XFS_LITINO(ip->i_mount);
> > +}
> > +
> > +static inline unsigned int xfs_inode_attr_fork_size(struct xfs_inode *ip)
> > +{
> > +	if (xfs_inode_has_attr_fork(ip))
> > +		return XFS_LITINO(ip->i_mount) - xfs_inode_fork_boff(ip);
> > +	return 0;
> > +}
> > +
> > +static inline unsigned int
> > +xfs_inode_fork_size(
> > +	struct xfs_inode	*ip,
> > +	int			whichfork)
> > +{
> > +	switch (whichfork) {
> > +	case XFS_DATA_FORK:
> > +		return xfs_inode_data_fork_size(ip);
> > +	case XFS_ATTR_FORK:
> > +		return xfs_inode_attr_fork_size(ip);
> > +	default:
> > +		return 0;
> > +	}
> > +}
> 
> As an aside, one of the things I noticed when doing the 5.19 libxfs
> sync was that there's some generic xfs_inode stuff in
> fs/xfs/xfs_inode.h that is duplicated in include/xfs_inode.h in
> userspace. I suspect all this new stuff here will end up being
> duplicated, too.
> 
> Hence I'm wondering if these new functions should end up in
> libxfs/xfs_inode_fork.h rather than xfs_inode.h?

They should, but there are a surprising number of files that don't
include xfs_inode.h before xfs_inode_fork.h, which causes a ton of
compilation errors.  I'll respin this series with a gigantic pile
of #include changes, but I wanted to get reviews of the main changes
in this series (patches #2 and #3) before spending time on that.

--D

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