diff mbox series

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

Message ID 20201015072155.1631135-13-david@fromorbit.com (mailing list archive)
State Deferred, archived
Headers show
Series xfsprogs: xfs_buf unification and AIO | expand

Commit Message

Dave Chinner Oct. 15, 2020, 7:21 a.m. UTC 
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(-)

Comments

Darrick J. Wong Oct. 15, 2020, 5:12 p.m. UTC | #1 
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 ?

--D

> + */
> +STATIC int
> +xfs_check_sizes(
> +        struct xfs_mount *mp)
> +{
> +	struct xfs_buf	*bp;
> +	xfs_daddr_t	d;
> +	int		error;
> +
> +	d = (xfs_daddr_t)XFS_FSB_TO_BB(mp, mp->m_sb.sb_dblocks);
> +	if (XFS_BB_TO_FSB(mp, d) != mp->m_sb.sb_dblocks) {
> +		xfs_warn(mp, "filesystem size mismatch detected");
> +		return -EFBIG;
> +	}
> +	error = libxfs_buf_read_uncached(mp->m_ddev_targp,
> +					d - XFS_FSS_TO_BB(mp, 1),
> +					XFS_FSS_TO_BB(mp, 1), 0, &bp, NULL);
> +	if (error) {
> +		xfs_warn(mp, "last sector read failed");
> +		return error;
> +	}
> +	libxfs_buf_relse(bp);
> +
> +	if (mp->m_logdev_targp == mp->m_ddev_targp)
> +		return 0;
> +
> +	d = (xfs_daddr_t)XFS_FSB_TO_BB(mp, mp->m_sb.sb_logblocks);
> +	if (XFS_BB_TO_FSB(mp, d) != mp->m_sb.sb_logblocks) {
> +		xfs_warn(mp, "log size mismatch detected");
> +		return -EFBIG;
> +	}
> +	error = libxfs_buf_read_uncached(mp->m_logdev_targp,
> +					d - XFS_FSB_TO_BB(mp, 1),
> +					XFS_FSB_TO_BB(mp, 1), 0, &bp, NULL);
> +	if (error) {
> +		xfs_warn(mp, "log device read failed");
> +		return error;
> +	}
> +	libxfs_buf_relse(bp);
> +	return 0;
> +}
> +
>  /*
>   * Mount structure initialization, provides a filled-in xfs_mount_t
>   * such that the numerous XFS_* macros can be used.  If dev is zero,
> @@ -673,7 +720,6 @@ libxfs_mount(
>  {
>  	struct xfs_buf		*bp;
>  	struct xfs_sb		*sbp;
> -	xfs_daddr_t		d;
>  	bool			debugger = (flags & LIBXFS_MOUNT_DEBUGGER);
>  	int			error;
>  
> @@ -704,16 +750,6 @@ libxfs_mount(
>  	xfs_rmapbt_compute_maxlevels(mp);
>  	xfs_refcountbt_compute_maxlevels(mp);
>  
> -	/*
> -	 * Check that the data (and log if separate) are an ok size.
> -	 */
> -	d = (xfs_daddr_t) XFS_FSB_TO_BB(mp, mp->m_sb.sb_dblocks);
> -	if (XFS_BB_TO_FSB(mp, d) != mp->m_sb.sb_dblocks) {
> -		fprintf(stderr, _("%s: size check failed\n"), progname);
> -		if (!(flags & LIBXFS_MOUNT_DEBUGGER))
> -			return NULL;
> -	}
> -
>  	/*
>  	 * We automatically convert v1 inodes to v2 inodes now, so if
>  	 * the NLINK bit is not set we can't operate on the filesystem.
> @@ -755,30 +791,9 @@ libxfs_mount(
>  		return mp;
>  
>  	/* device size checks must pass unless we're a debugger. */
> -	error = libxfs_buf_read(mp->m_dev, d - XFS_FSS_TO_BB(mp, 1),
> -			XFS_FSS_TO_BB(mp, 1), 0, &bp, NULL);
> -	if (error) {
> -		fprintf(stderr, _("%s: data size check failed\n"), progname);
> -		if (!debugger)
> -			return NULL;
> -	} else
> -		libxfs_buf_relse(bp);
> -
> -	if (mp->m_logdev_targp->bt_bdev &&
> -	    mp->m_logdev_targp->bt_bdev != mp->m_ddev_targp->bt_bdev) {
> -		d = (xfs_daddr_t) XFS_FSB_TO_BB(mp, mp->m_sb.sb_logblocks);
> -		if (XFS_BB_TO_FSB(mp, d) != mp->m_sb.sb_logblocks ||
> -		    libxfs_buf_read(mp->m_logdev_targp,
> -				d - XFS_FSB_TO_BB(mp, 1), XFS_FSB_TO_BB(mp, 1),
> -				0, &bp, NULL)) {
> -			fprintf(stderr, _("%s: log size checks failed\n"),
> -					progname);
> -			if (!debugger)
> -				return NULL;
> -		}
> -		if (bp)
> -			libxfs_buf_relse(bp);
> -	}
> +	error = xfs_check_sizes(mp);
> +	if (error && !debugger)
> +		return NULL;
>  
>  	/* Initialize realtime fields in the mount structure */
>  	if (rtmount_init(mp, flags)) {
> @@ -795,7 +810,7 @@ libxfs_mount(
>  	 * read the first one and let the user know to check the geometry.
>  	 */
>  	if (sbp->sb_agcount > 1000000) {
> -		error = libxfs_buf_read(mp->m_dev,
> +		error = libxfs_buf_read_uncached(mp->m_ddev_targp,
>  				XFS_AG_DADDR(mp, sbp->sb_agcount - 1, 0), 1,
>  				0, &bp, NULL);
>  		if (error) {
> -- 
> 2.28.0
>
Dave Chinner Oct. 19, 2020, 10:36 p.m. UTC | #2 
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.
diff mbox series

Patch

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);
 		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
+ */
+STATIC int
+xfs_check_sizes(
+        struct xfs_mount *mp)
+{
+	struct xfs_buf	*bp;
+	xfs_daddr_t	d;
+	int		error;
+
+	d = (xfs_daddr_t)XFS_FSB_TO_BB(mp, mp->m_sb.sb_dblocks);
+	if (XFS_BB_TO_FSB(mp, d) != mp->m_sb.sb_dblocks) {
+		xfs_warn(mp, "filesystem size mismatch detected");
+		return -EFBIG;
+	}
+	error = libxfs_buf_read_uncached(mp->m_ddev_targp,
+					d - XFS_FSS_TO_BB(mp, 1),
+					XFS_FSS_TO_BB(mp, 1), 0, &bp, NULL);
+	if (error) {
+		xfs_warn(mp, "last sector read failed");
+		return error;
+	}
+	libxfs_buf_relse(bp);
+
+	if (mp->m_logdev_targp == mp->m_ddev_targp)
+		return 0;
+
+	d = (xfs_daddr_t)XFS_FSB_TO_BB(mp, mp->m_sb.sb_logblocks);
+	if (XFS_BB_TO_FSB(mp, d) != mp->m_sb.sb_logblocks) {
+		xfs_warn(mp, "log size mismatch detected");
+		return -EFBIG;
+	}
+	error = libxfs_buf_read_uncached(mp->m_logdev_targp,
+					d - XFS_FSB_TO_BB(mp, 1),
+					XFS_FSB_TO_BB(mp, 1), 0, &bp, NULL);
+	if (error) {
+		xfs_warn(mp, "log device read failed");
+		return error;
+	}
+	libxfs_buf_relse(bp);
+	return 0;
+}
+
 /*
  * Mount structure initialization, provides a filled-in xfs_mount_t
  * such that the numerous XFS_* macros can be used.  If dev is zero,
@@ -673,7 +720,6 @@  libxfs_mount(
 {
 	struct xfs_buf		*bp;
 	struct xfs_sb		*sbp;
-	xfs_daddr_t		d;
 	bool			debugger = (flags & LIBXFS_MOUNT_DEBUGGER);
 	int			error;
 
@@ -704,16 +750,6 @@  libxfs_mount(
 	xfs_rmapbt_compute_maxlevels(mp);
 	xfs_refcountbt_compute_maxlevels(mp);
 
-	/*
-	 * Check that the data (and log if separate) are an ok size.
-	 */
-	d = (xfs_daddr_t) XFS_FSB_TO_BB(mp, mp->m_sb.sb_dblocks);
-	if (XFS_BB_TO_FSB(mp, d) != mp->m_sb.sb_dblocks) {
-		fprintf(stderr, _("%s: size check failed\n"), progname);
-		if (!(flags & LIBXFS_MOUNT_DEBUGGER))
-			return NULL;
-	}
-
 	/*
 	 * We automatically convert v1 inodes to v2 inodes now, so if
 	 * the NLINK bit is not set we can't operate on the filesystem.
@@ -755,30 +791,9 @@  libxfs_mount(
 		return mp;
 
 	/* device size checks must pass unless we're a debugger. */
-	error = libxfs_buf_read(mp->m_dev, d - XFS_FSS_TO_BB(mp, 1),
-			XFS_FSS_TO_BB(mp, 1), 0, &bp, NULL);
-	if (error) {
-		fprintf(stderr, _("%s: data size check failed\n"), progname);
-		if (!debugger)
-			return NULL;
-	} else
-		libxfs_buf_relse(bp);
-
-	if (mp->m_logdev_targp->bt_bdev &&
-	    mp->m_logdev_targp->bt_bdev != mp->m_ddev_targp->bt_bdev) {
-		d = (xfs_daddr_t) XFS_FSB_TO_BB(mp, mp->m_sb.sb_logblocks);
-		if (XFS_BB_TO_FSB(mp, d) != mp->m_sb.sb_logblocks ||
-		    libxfs_buf_read(mp->m_logdev_targp,
-				d - XFS_FSB_TO_BB(mp, 1), XFS_FSB_TO_BB(mp, 1),
-				0, &bp, NULL)) {
-			fprintf(stderr, _("%s: log size checks failed\n"),
-					progname);
-			if (!debugger)
-				return NULL;
-		}
-		if (bp)
-			libxfs_buf_relse(bp);
-	}
+	error = xfs_check_sizes(mp);
+	if (error && !debugger)
+		return NULL;
 
 	/* Initialize realtime fields in the mount structure */
 	if (rtmount_init(mp, flags)) {
@@ -795,7 +810,7 @@  libxfs_mount(
 	 * read the first one and let the user know to check the geometry.
 	 */
 	if (sbp->sb_agcount > 1000000) {
-		error = libxfs_buf_read(mp->m_dev,
+		error = libxfs_buf_read_uncached(mp->m_ddev_targp,
 				XFS_AG_DADDR(mp, sbp->sb_agcount - 1, 0), 1,
 				0, &bp, NULL);
 		if (error) {