Re: [PATCH 2/2] mkfs: Show progress during block discard

[Dave Chinner <david@fromorbit.com>]

On Thu, Nov 21, 2019 at 10:44:45PM +0100, Pavel Reichl wrote:
> Signed-off-by: Pavel Reichl <preichl@redhat.com>
> ---
>  mkfs/xfs_mkfs.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index a02d6f66..07b8bd78 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -1248,6 +1248,7 @@ discard_blocks(dev_t dev, uint64_t nsectors)
>  	const uint64_t	step		= (uint64_t)2<<30;
>  	/* Sector size is 512 bytes */
>  	const uint64_t	count		= nsectors << 9;
> +	uint64_t	prev_done	= (uint64_t) ~0;
>  
>  	fd = libxfs_device_to_fd(dev);
>  	if (fd <= 0)
> @@ -1255,6 +1256,7 @@ discard_blocks(dev_t dev, uint64_t nsectors)
>  
>  	while (offset < count) {
>  		uint64_t	tmp_step = step;
> +		uint64_t	done = offset * 100 / count;

That will overflow on a EB-scale (2^60 bytes) filesystems, won't it?

>  
>  		if ((offset + step) > count)
>  			tmp_step = count - offset;
> @@ -1268,7 +1270,13 @@ discard_blocks(dev_t dev, uint64_t nsectors)
>  			return;
>  
>  		offset += tmp_step;
> +
> +		if (prev_done != done) {
> +			prev_done = done;
> +			fprintf(stderr, _("Discarding: %2lu%% done\n"), done);
> +		}
>  	}
> +	fprintf(stderr, _("Discarding is done.\n"));

Hmmm - this output doesn't get suppressed when the "quiet" (-q)
option is used. mkfs is supposed to be silent when this option is
specified.

I also suspect that it breaks a few fstests, too, as a some of them
capture and filter mkfs output. They'll need filters to drop these
new messages.

FWIW, a 100 lines of extra mkfs output is going to cause workflow
issues. I know it will cause me problems, because I often mkfs 500TB
filesystems tens of times a day on a discard enabled device. This
extra output will scroll all the context of the previous test run
I'm about to compare against off my terminal screen and so now I
will have to scroll the terminal to look at the results of
back-to-back runs. IOWs, I'm going to immediately want to turn this
output off and have it stay off permanently.

Hence I think that, by default, just outputting a single "Discard in
progress" line before starting the discard would be sufficient
indication of what mkfs is currently doing. If someone wants more
verbose progress output, then we should probably introduce a
"verbose" CLI option to go along with the "quiet" option that
suppresses all output.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com