diff mbox series

[7/8] iogen: upgrade to fallocate

Message ID 164193784690.3008286.8689130816813600863.stgit@magnolia (mailing list archive)
State New, archived
Headers show
Series fstests: random fixes | expand

Commit Message

Darrick J. Wong Jan. 11, 2022, 9:50 p.m. UTC 
From: Darrick J. Wong <djwong@kernel.org>

Update this utility to use fallocate to preallocate/reserve space to a
file so that we're not so dependent on legacy XFS ioctls.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 ltp/iogen.c |   32 ++++++++++++++++++++++----------
 1 file changed, 22 insertions(+), 10 deletions(-)

Comments

Eryu Guan Jan. 16, 2022, 7:01 a.m. UTC | #1 
On Tue, Jan 11, 2022 at 01:50:46PM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Update this utility to use fallocate to preallocate/reserve space to a
> file so that we're not so dependent on legacy XFS ioctls.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  ltp/iogen.c |   32 ++++++++++++++++++++++----------
>  1 file changed, 22 insertions(+), 10 deletions(-)
> 
> 
> diff --git a/ltp/iogen.c b/ltp/iogen.c
> index 2b6644d5..6c2c1534 100644
> --- a/ltp/iogen.c
> +++ b/ltp/iogen.c
> @@ -928,7 +928,15 @@ bozo!
>  		   fd, f.l_whence, (long long)f.l_start, (long long)f.l_len);*/
>  
>  	    /* non-zeroing reservation */
> -#ifdef XFS_IOC_RESVSP
> +#if defined(HAVE_FALLOCATE)
> +	    if (fallocate(fd, 0, 0, nbytes) == -1) {

Seems this is not a identical replacement for XFS_IOC_RESVSP (is that
what we want here?), as fallocate(2) here zeros space and change i_size
as well.

And from xfsctl(3), after XFS_IOC_RESVSP "The blocks are allocated, but
not zeroed, and the file size does not change." And the comments above
indicates "non-zeroing reservation" as well.

If identical replacement is not required, we could drop "non-zeroing"
part, but add FALLOC_FL_KEEP_SIZE?

Thanks,
Eryu

> +		fprintf(stderr,
> +			"iogen%s:  Could not fallocate %d bytes in file %s: %s (%d)\n",
> +			TagName, nbytes, path, SYSERR, errno);
> +		close(fd);
> +		return -1;
> +	    }
> +#elif defined(XFS_IOC_RESVSP)
>  	    if( xfsctl( path, fd, XFS_IOC_RESVSP, &f ) == -1) {
>  		fprintf(stderr,
>  			"iogen%s:  Could not xfsctl(XFS_IOC_RESVSP) %d bytes in file %s: %s (%d)\n",
> @@ -936,8 +944,7 @@ bozo!
>  		close(fd);
>  		return -1;
>  	    }
> -#else
> -#ifdef F_RESVSP
> +#elif defined(F_RESVSP)
>  	    if( fcntl( fd, F_RESVSP, &f ) == -1) {
>  		fprintf(stderr,
>  			"iogen%s:  Could not fcntl(F_RESVSP) %d bytes in file %s: %s (%d)\n",
> @@ -946,8 +953,7 @@ bozo!
>  		return -1;
>  	    }
>  #else
> -bozo!
> -#endif
> +# error Dont know how to reserve space!
>  #endif
>  	}
>  
> @@ -962,7 +968,15 @@ bozo!
>  		    (long long)f.l_len);*/
>  
>  	    /* zeroing reservation */
> -#ifdef XFS_IOC_ALLOCSP
> +#if defined(HAVE_FALLOCATE)
> +	    if (fallocate(fd, 0, sbuf.st_size, nbytes - sbuf.st_size) == -1) {
> +		fprintf(stderr,
> +			"iogen%s:  Could not fallocate %d bytes in file %s: %s (%d)\n",
> +			TagName, nbytes, path, SYSERR, errno);
> +		close(fd);
> +		return -1;
> +	    }
> +#elif defined(XFS_IOC_ALLOCSP)
>  	    if( xfsctl( path, fd, XFS_IOC_ALLOCSP, &f ) == -1) {
>  		fprintf(stderr,
>  			"iogen%s:  Could not xfsctl(XFS_IOC_ALLOCSP) %d bytes in file %s: %s (%d)\n",
> @@ -970,8 +984,7 @@ bozo!
>  		close(fd);
>  		return -1;
>  	    }
> -#else
> -#ifdef F_ALLOCSP
> +#elif defined(F_ALLOCSP)
>  	    if ( fcntl(fd, F_ALLOCSP, &f) < 0) {
>  		fprintf(stderr,
>  			"iogen%s:  Could not fcntl(F_ALLOCSP) %d bytes in file %s: %s (%d)\n",
> @@ -980,8 +993,7 @@ bozo!
>  		return -1;
>  	    }
>  #else
> -bozo!
> -#endif
> +# error Dont know how to (pre)allocate space!
>  #endif
>  	}
>  #endif
Darrick J. Wong Jan. 17, 2022, 5:15 p.m. UTC | #2 
On Sun, Jan 16, 2022 at 03:01:35PM +0800, Eryu Guan wrote:
> On Tue, Jan 11, 2022 at 01:50:46PM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > Update this utility to use fallocate to preallocate/reserve space to a
> > file so that we're not so dependent on legacy XFS ioctls.
> > 
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> >  ltp/iogen.c |   32 ++++++++++++++++++++++----------
> >  1 file changed, 22 insertions(+), 10 deletions(-)
> > 
> > 
> > diff --git a/ltp/iogen.c b/ltp/iogen.c
> > index 2b6644d5..6c2c1534 100644
> > --- a/ltp/iogen.c
> > +++ b/ltp/iogen.c
> > @@ -928,7 +928,15 @@ bozo!
> >  		   fd, f.l_whence, (long long)f.l_start, (long long)f.l_len);*/
> >  
> >  	    /* non-zeroing reservation */
> > -#ifdef XFS_IOC_RESVSP
> > +#if defined(HAVE_FALLOCATE)
> > +	    if (fallocate(fd, 0, 0, nbytes) == -1) {
> 
> Seems this is not a identical replacement for XFS_IOC_RESVSP (is that
> what we want here?), as fallocate(2) here zeros space and change i_size
> as well.

Doh, you're right, the second parameter should follow what the kernel
does and specify FALLOC_FL_KEEP_SIZE.  I'll fix that and resubmit.
Thanks for taking the rest of the series. :)

--D

> And from xfsctl(3), after XFS_IOC_RESVSP "The blocks are allocated, but
> not zeroed, and the file size does not change." And the comments above
> indicates "non-zeroing reservation" as well.
> 
> If identical replacement is not required, we could drop "non-zeroing"
> part, but add FALLOC_FL_KEEP_SIZE?
> 
> Thanks,
> Eryu
> 
> > +		fprintf(stderr,
> > +			"iogen%s:  Could not fallocate %d bytes in file %s: %s (%d)\n",
> > +			TagName, nbytes, path, SYSERR, errno);
> > +		close(fd);
> > +		return -1;
> > +	    }
> > +#elif defined(XFS_IOC_RESVSP)
> >  	    if( xfsctl( path, fd, XFS_IOC_RESVSP, &f ) == -1) {
> >  		fprintf(stderr,
> >  			"iogen%s:  Could not xfsctl(XFS_IOC_RESVSP) %d bytes in file %s: %s (%d)\n",
> > @@ -936,8 +944,7 @@ bozo!
> >  		close(fd);
> >  		return -1;
> >  	    }
> > -#else
> > -#ifdef F_RESVSP
> > +#elif defined(F_RESVSP)
> >  	    if( fcntl( fd, F_RESVSP, &f ) == -1) {
> >  		fprintf(stderr,
> >  			"iogen%s:  Could not fcntl(F_RESVSP) %d bytes in file %s: %s (%d)\n",
> > @@ -946,8 +953,7 @@ bozo!
> >  		return -1;
> >  	    }
> >  #else
> > -bozo!
> > -#endif
> > +# error Dont know how to reserve space!
> >  #endif
> >  	}
> >  
> > @@ -962,7 +968,15 @@ bozo!
> >  		    (long long)f.l_len);*/
> >  
> >  	    /* zeroing reservation */
> > -#ifdef XFS_IOC_ALLOCSP
> > +#if defined(HAVE_FALLOCATE)
> > +	    if (fallocate(fd, 0, sbuf.st_size, nbytes - sbuf.st_size) == -1) {
> > +		fprintf(stderr,
> > +			"iogen%s:  Could not fallocate %d bytes in file %s: %s (%d)\n",
> > +			TagName, nbytes, path, SYSERR, errno);
> > +		close(fd);
> > +		return -1;
> > +	    }
> > +#elif defined(XFS_IOC_ALLOCSP)
> >  	    if( xfsctl( path, fd, XFS_IOC_ALLOCSP, &f ) == -1) {
> >  		fprintf(stderr,
> >  			"iogen%s:  Could not xfsctl(XFS_IOC_ALLOCSP) %d bytes in file %s: %s (%d)\n",
> > @@ -970,8 +984,7 @@ bozo!
> >  		close(fd);
> >  		return -1;
> >  	    }
> > -#else
> > -#ifdef F_ALLOCSP
> > +#elif defined(F_ALLOCSP)
> >  	    if ( fcntl(fd, F_ALLOCSP, &f) < 0) {
> >  		fprintf(stderr,
> >  			"iogen%s:  Could not fcntl(F_ALLOCSP) %d bytes in file %s: %s (%d)\n",
> > @@ -980,8 +993,7 @@ bozo!
> >  		return -1;
> >  	    }
> >  #else
> > -bozo!
> > -#endif
> > +# error Dont know how to (pre)allocate space!
> >  #endif
> >  	}
> >  #endif
diff mbox series

Patch

diff --git a/ltp/iogen.c b/ltp/iogen.c
index 2b6644d5..6c2c1534 100644
--- a/ltp/iogen.c
+++ b/ltp/iogen.c
@@ -928,7 +928,15 @@  bozo!
 		   fd, f.l_whence, (long long)f.l_start, (long long)f.l_len);*/
 
 	    /* non-zeroing reservation */
-#ifdef XFS_IOC_RESVSP
+#if defined(HAVE_FALLOCATE)
+	    if (fallocate(fd, 0, 0, nbytes) == -1) {
+		fprintf(stderr,
+			"iogen%s:  Could not fallocate %d bytes in file %s: %s (%d)\n",
+			TagName, nbytes, path, SYSERR, errno);
+		close(fd);
+		return -1;
+	    }
+#elif defined(XFS_IOC_RESVSP)
 	    if( xfsctl( path, fd, XFS_IOC_RESVSP, &f ) == -1) {
 		fprintf(stderr,
 			"iogen%s:  Could not xfsctl(XFS_IOC_RESVSP) %d bytes in file %s: %s (%d)\n",
@@ -936,8 +944,7 @@  bozo!
 		close(fd);
 		return -1;
 	    }
-#else
-#ifdef F_RESVSP
+#elif defined(F_RESVSP)
 	    if( fcntl( fd, F_RESVSP, &f ) == -1) {
 		fprintf(stderr,
 			"iogen%s:  Could not fcntl(F_RESVSP) %d bytes in file %s: %s (%d)\n",
@@ -946,8 +953,7 @@  bozo!
 		return -1;
 	    }
 #else
-bozo!
-#endif
+# error Dont know how to reserve space!
 #endif
 	}
 
@@ -962,7 +968,15 @@  bozo!
 		    (long long)f.l_len);*/
 
 	    /* zeroing reservation */
-#ifdef XFS_IOC_ALLOCSP
+#if defined(HAVE_FALLOCATE)
+	    if (fallocate(fd, 0, sbuf.st_size, nbytes - sbuf.st_size) == -1) {
+		fprintf(stderr,
+			"iogen%s:  Could not fallocate %d bytes in file %s: %s (%d)\n",
+			TagName, nbytes, path, SYSERR, errno);
+		close(fd);
+		return -1;
+	    }
+#elif defined(XFS_IOC_ALLOCSP)
 	    if( xfsctl( path, fd, XFS_IOC_ALLOCSP, &f ) == -1) {
 		fprintf(stderr,
 			"iogen%s:  Could not xfsctl(XFS_IOC_ALLOCSP) %d bytes in file %s: %s (%d)\n",
@@ -970,8 +984,7 @@  bozo!
 		close(fd);
 		return -1;
 	    }
-#else
-#ifdef F_ALLOCSP
+#elif defined(F_ALLOCSP)
 	    if ( fcntl(fd, F_ALLOCSP, &f) < 0) {
 		fprintf(stderr,
 			"iogen%s:  Could not fcntl(F_ALLOCSP) %d bytes in file %s: %s (%d)\n",
@@ -980,8 +993,7 @@  bozo!
 		return -1;
 	    }
 #else
-bozo!
-#endif
+# error Dont know how to (pre)allocate space!
 #endif
 	}
 #endif