mkfs: print newline if discard fails
diff mbox series

Message ID 20191214180559.GN99875@magnolia
State New
Headers show
Series
  • mkfs: print newline if discard fails
Related show

Commit Message

Darrick J. Wong Dec. 14, 2019, 6:05 p.m. UTC
From: Darrick J. Wong <darrick.wong@oracle.com>

Make sure the "Discarding..." gets a newline after it no matter how we
exit the function.  Don't bother with any printing it even a small
discard request fails.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 mkfs/xfs_mkfs.c |   12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

Comments

Eric Sandeen Dec. 14, 2019, 6:34 p.m. UTC | #1
On 12/14/19 12:05 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Make sure the "Discarding..." gets a newline after it no matter how we
> exit the function.  Don't bother with any printing it even a small
> discard request fails.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  mkfs/xfs_mkfs.c |   12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 4bfdebf6..948a5a77 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -1251,6 +1251,14 @@ discard_blocks(dev_t dev, uint64_t nsectors, int quiet)
>  	fd = libxfs_device_to_fd(dev);
>  	if (fd <= 0)
>  		return;
> +
> +	/*
> +	 * Try discarding the first 64k; if that fails, don't bother printing
> +	 * any messages at all.
> +	 */
> +	if (platform_discard_blocks(fd, offset, 65536))
> +		return;

or trying to discard at all for that matter ;)  (i.e. this does more than suppress
messages)

I think this patch does 2 things... and that comment is a little confusing.

Also you discard the first 64k twice which is ok but...?

>  	if (!quiet) {
>  		printf("(ks...");
>  		fflush(stdout);
> @@ -1267,8 +1275,10 @@ discard_blocks(dev_t dev, uint64_t nsectors, int quiet)
>  		 * not necessary for the mkfs functionality but just an
>  		 * optimization. However we should stop on error.
>  		 */
> -		if (platform_discard_blocks(fd, offset, tmp_step))
> +		if (platform_discard_blocks(fd, offset, tmp_step)) {

also we don't want to do this if (quiet) ;)

> +			printf("\n");
>  			return;
> +		}
>  
>  		offset += tmp_step;
>  	}
> 

How about this, though I'd split it into 2 patches, 1 to catch the newline
if discard fails (and !quiet), and another to only print status if/after
the first discard succeeds


diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 4bfdebf6..9f07f042 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -1244,6 +1244,7 @@ discard_blocks(dev_t dev, uint64_t nsectors, int quiet)
 {
 	int		fd;
 	uint64_t	offset = 0;
+	bool		messaged = false;
 	/* Discard the device 2G at a time */
 	const uint64_t	step = 2ULL << 30;
 	const uint64_t	count = BBTOB(nsectors);
@@ -1251,10 +1252,6 @@ discard_blocks(dev_t dev, uint64_t nsectors, int quiet)
 	fd = libxfs_device_to_fd(dev);
 	if (fd <= 0)
 		return;
-	if (!quiet) {
-		printf("Discarding blocks...");
-		fflush(stdout);
-	}
 
 	/* The block discarding happens in smaller batches so it can be
 	 * interrupted prematurely
@@ -1267,12 +1264,20 @@ discard_blocks(dev_t dev, uint64_t nsectors, int quiet)
 		 * not necessary for the mkfs functionality but just an
 		 * optimization. However we should stop on error.
 		 */
-		if (platform_discard_blocks(fd, offset, tmp_step))
+		if (platform_discard_blocks(fd, offset, tmp_step) == 0) {
+			if (!messaged && !quiet) {
+				printf("Discarding blocks...");
+				fflush(stdout);
+				messaged = true;
+			}
+		} else if (messaged && !quiet) {
+			printf("\n");
 			return;
+		}
 
 		offset += tmp_step;
 	}
-	if (!quiet)
+	if (messaged && !quiet)
 		printf("Done.\n");
 }

Patch
diff mbox series

diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 4bfdebf6..948a5a77 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -1251,6 +1251,14 @@  discard_blocks(dev_t dev, uint64_t nsectors, int quiet)
 	fd = libxfs_device_to_fd(dev);
 	if (fd <= 0)
 		return;
+
+	/*
+	 * Try discarding the first 64k; if that fails, don't bother printing
+	 * any messages at all.
+	 */
+	if (platform_discard_blocks(fd, offset, 65536))
+		return;
+
 	if (!quiet) {
 		printf("Discarding blocks...");
 		fflush(stdout);
@@ -1267,8 +1275,10 @@  discard_blocks(dev_t dev, uint64_t nsectors, int quiet)
 		 * not necessary for the mkfs functionality but just an
 		 * optimization. However we should stop on error.
 		 */
-		if (platform_discard_blocks(fd, offset, tmp_step))
+		if (platform_discard_blocks(fd, offset, tmp_step)) {
+			printf("\n");
 			return;
+		}
 
 		offset += tmp_step;
 	}