| Message ID | 20191214180559.GN99875@magnolia (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mkfs: print newline if discard fails | expand |
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"); }
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; }