From: Eric Sandeen <sandeen@sandeen.net> To: Pavel Reichl <preichl@redhat.com>, linux-xfs@vger.kernel.org Subject: Re: [PATCH v3] mkfs: Break block discard into chunks of 2 GB Date: Mon, 9 Dec 2019 16:00:17 -0600 Message-ID: <6ad2e204-aa5b-c487-e03e-4a75b01018fa@sandeen.net> (raw) In-Reply-To: <20191128062139.93218-1-preichl@redhat.com> On 11/28/19 12:21 AM, Pavel Reichl wrote: > Some users are not happy about the BLKDISCARD taking too long and at the same > time not being informed about that - so they think that the command actually > hung. > > This commit changes code so that progress reporting is possible and also typing > the ^C will cancel the ongoing BLKDISCARD. Ok I'm going to nitpick this just a little bit more... > Signed-off-by: Pavel Reichl <preichl@redhat.com> > --- > mkfs/xfs_mkfs.c | 50 ++++++++++++++++++++++++++++++++++++------------- > 1 file changed, 37 insertions(+), 13 deletions(-) > > diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c > index 18338a61..0defadb2 100644 > --- a/mkfs/xfs_mkfs.c > +++ b/mkfs/xfs_mkfs.c > @@ -1240,17 +1240,40 @@ done: > } > > static void > -discard_blocks(dev_t dev, uint64_t nsectors) > +discard_blocks(dev_t dev, uint64_t nsectors, int quiet) > { > - int fd; > + int fd; > + uint64_t offset = 0; > + /* Discard the device 2G at a time */ > + const uint64_t step = 2ULL << 30; > + const uint64_t count = BBTOB(nsectors); > > - /* > - * We intentionally ignore errors from the discard ioctl. It is > - * not necessary for the mkfs functionality but just an optimization. > - */ > fd = libxfs_device_to_fd(dev); > - if (fd > 0) > - platform_discard_blocks(fd, 0, nsectors << 9); > + if (fd <= 0) > + return; > + if (!quiet) { > + printf("Discarding blocks...\n"); > + printf("...\n"); > + } Let's change this output so that it prints only a single line, i.e. + if (!quiet) { + printf("Discarding blocks... "); ... + if (!quiet) + printf("Done.\n"); Then in xfstests there's only one line to filter. I think there was a suggestion to test isatty() to see if we're on a terminal, but I have some concern that a hung script will lead to confusion as well if there's no status message. We can add terminal testing in a later patch if that really seems like the right way to go. If you're ok with the above, I can make the change on commit to save another patch round trip, and Reviewed-by: Eric Sandeen <sandeen@redhat.com> > + > + /* The block discarding happens in smaller batches so it can be > + * interrupted prematurely > + */ > + while (offset < count) { > + uint64_t tmp_step = min(step, count - offset); > + > + /* > + * We intentionally ignore errors from the discard ioctl. It is > + * not necessary for the mkfs functionality but just an > + * optimization. However we should stop on error. > + */ > + if (platform_discard_blocks(fd, offset, tmp_step)) > + return; > + > + offset += tmp_step; > + } > + if (!quiet) > + printf("Done.\n"); > } > > static __attribute__((noreturn)) void > @@ -2507,18 +2530,19 @@ open_devices( > > static void > discard_devices( > - struct libxfs_xinit *xi) > + struct libxfs_xinit *xi, > + int quiet) > { > /* > * This function has to be called after libxfs has been initialized. > */ > > if (!xi->disfile) > - discard_blocks(xi->ddev, xi->dsize); > + discard_blocks(xi->ddev, xi->dsize, quiet); > if (xi->rtdev && !xi->risfile) > - discard_blocks(xi->rtdev, xi->rtsize); > + discard_blocks(xi->rtdev, xi->rtsize, quiet); > if (xi->logdev && xi->logdev != xi->ddev && !xi->lisfile) > - discard_blocks(xi->logdev, xi->logBBsize); > + discard_blocks(xi->logdev, xi->logBBsize, quiet); > } > > static void > @@ -3749,7 +3773,7 @@ main( > * All values have been validated, discard the old device layout. > */ > if (discard && !dry_run) > - discard_devices(&xi); > + discard_devices(&xi, quiet); > > /* > * we need the libxfs buffer cache from here on in. >
next prev parent reply index Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-11-28 6:21 Pavel Reichl 2019-11-30 22:01 ` Chaitanya Kulkarni 2019-12-02 16:40 ` Darrick J. Wong 2019-12-04 16:24 ` Eric Sandeen 2019-12-04 17:26 ` Christoph Hellwig 2019-12-04 17:32 ` Eric Sandeen 2019-12-04 17:42 ` Darrick J. Wong 2019-12-10 7:33 ` Chaitanya Kulkarni 2019-12-10 14:20 ` Eric Sandeen 2019-12-09 22:00 ` Eric Sandeen [this message] 2019-12-09 23:34 ` Darrick J. Wong 2019-12-10 0:49 ` Eric Sandeen
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=6ad2e204-aa5b-c487-e03e-4a75b01018fa@sandeen.net \ --to=sandeen@sandeen.net \ --cc=linux-xfs@vger.kernel.org \ --cc=preichl@redhat.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
Linux-XFS Archive on lore.kernel.org Archives are clonable: git clone --mirror https://lore.kernel.org/linux-xfs/0 linux-xfs/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 linux-xfs linux-xfs/ https://lore.kernel.org/linux-xfs \ linux-xfs@vger.kernel.org public-inbox-index linux-xfs Example config snippet for mirrors Newsgroup available over NNTP: nntp://nntp.lore.kernel.org/org.kernel.vger.linux-xfs AGPL code for this site: git clone https://public-inbox.org/public-inbox.git