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

Dave Chinner
2020-10-19T22:36:58Z

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