On 12/14/19 12:05 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>
> ---
> 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");
}
