Re: [PATCH 12/27] xfsprogs: convert use-once buffer reads to uncached IO

From: Dave Chinner <david@fromorbit.com>
To: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 12/27] xfsprogs: convert use-once buffer reads to uncached IO
Date: Tue, 20 Oct 2020 09:36:51 +1100	[thread overview]
Message-ID: <20201019223651.GN7391@dread.disaster.area> (raw)
In-Reply-To: <20201015171239.GV9832@magnolia>

On Thu, Oct 15, 2020 at 10:12:39AM -0700, Darrick J. Wong wrote:
> On Thu, Oct 15, 2020 at 06:21:40PM +1100, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> > 
> > Signed-off-by: Dave Chinner <dchinner@redhat.com>
> > ---
> >  db/init.c     |  2 +-
> >  libxfs/init.c | 93 ++++++++++++++++++++++++++++++---------------------
> >  2 files changed, 55 insertions(+), 40 deletions(-)
> > 
> > diff --git a/db/init.c b/db/init.c
> > index 19f0900a862b..f797df8a768b 100644
> > --- a/db/init.c
> > +++ b/db/init.c
> > @@ -153,7 +153,7 @@ init(
> >  	 */
> >  	if (sbp->sb_rootino != NULLFSINO &&
> >  	    xfs_sb_version_haslazysbcount(&mp->m_sb)) {
> > -		int error = -libxfs_initialize_perag_data(mp, sbp->sb_agcount);
> > +		error = -libxfs_initialize_perag_data(mp, sbp->sb_agcount);
> 
> Er... this and the xfs_check_sizes hoisting below don't have anything to
> do with uncached io conversion...?
> 
> >  		if (error) {
> >  			fprintf(stderr,
> >  	_("%s: cannot init perag data (%d). Continuing anyway.\n"),
> > diff --git a/libxfs/init.c b/libxfs/init.c
> > index fe784940c299..fc30f92d6fb2 100644
> > --- a/libxfs/init.c
> > +++ b/libxfs/init.c
> > @@ -419,7 +419,7 @@ done:
> >   */
> >  static int
> >  rtmount_init(
> > -	xfs_mount_t	*mp,	/* file system mount structure */
> > +	struct xfs_mount *mp,
> >  	int		flags)
> >  {
> >  	struct xfs_buf	*bp;	/* buffer for last block of subvolume */
> > @@ -473,8 +473,9 @@ rtmount_init(
> >  			(unsigned long long) mp->m_sb.sb_rblocks);
> >  		return -1;
> >  	}
> > -	error = libxfs_buf_read(mp->m_rtdev, d - XFS_FSB_TO_BB(mp, 1),
> > -			XFS_FSB_TO_BB(mp, 1), 0, &bp, NULL);
> > +	error = libxfs_buf_read_uncached(mp->m_rtdev_targp,
> > +					d - XFS_FSB_TO_BB(mp, 1),
> > +					XFS_FSB_TO_BB(mp, 1), 0, &bp, NULL);
> >  	if (error) {
> >  		fprintf(stderr, _("%s: realtime size check failed\n"),
> >  			progname);
> > @@ -657,6 +658,52 @@ libxfs_buftarg_init(
> >  	mp->m_rtdev_targp = libxfs_buftarg_alloc(mp, rtdev);
> >  }
> >  
> > +/*
> > + * Check that the data (and log if separate) is an ok size.
> > + *
> > + * XXX: copied from kernel, needs to be moved to shared code
> 
> Ah, because you want to share this function with the kernel.
> 
> Hmm... what do you think about putting it in libxfs/xfs_sb.c ?

It's not really superblock functionality - it's buftarg
functionality - but I don't see anywhere else that is even vaguely
appropriate. And the buftarg implementation is not intendeed to be
shared code, so it doesn't fit there, either. :/

So, yeah, maybe that is the only suitable place it can be put. I'll
split it out into it's own patch.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com