Re: [PATCH 1/2] mkfs: Break block discard into chunks of 2 GB

Eric Sandeen
2019-11-26T20:53:27Z

On 11/21/19 3:44 PM, Pavel Reichl wrote:
> Signed-off-by: Pavel Reichl <preichl@redhat.com>
> ---
> mkfs/xfs_mkfs.c | 32 +++++++++++++++++++++++++-------
> 1 file changed, 25 insertions(+), 7 deletions(-)
>
> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> index 18338a61..a02d6f66 100644
> --- a/mkfs/xfs_mkfs.c
> +++ b/mkfs/xfs_mkfs.c
> @@ -1242,15 +1242,33 @@ done:
> static void
> discard_blocks(dev_t dev, uint64_t nsectors)
> {
> - int fd;
> + int fd;
> + uint64_t offset = 0;
> + /* Maximal chunk of bytes to discard is 2GB */
> + const uint64_t step = (uint64_t)2<<30;

Regarding the discard step size, I would like to just see us keep 2G -
I see problems with the alternate suggestions proposed in the
threads on this patch review:

1) query block device for maximal discard size
-> block device folks I've talked to (Jeff Moyer in particular) stated
that many devices are known for putting a huge value in here, and then
taking far, far too long to process that size request. In short,
maximum size != fast.

2) discard one AG size at a time
-> this can be up to 1T, which puts us right back at our problem of large,
slow discards. And in particular, AG size has no relation at all to a
device's discard behavior. (further complicating this, we don't have
this geometry available anywhere in the current chain of calls to the
discard ioctl.)

Lukas did an investigation of discard behaviors (though it was some time
ago https://sourceforge.net/projects/test-discard/) and arrived at 2G as
a reasonable size after testing many different devices - I've not seen any
complaints from mke2fs users about problems doing discards in 2G chunks.

So I think just picking a fixed 2G size is the best plan for now.

(one nitpick, I'd fix the comment above to not say "Maximal" because that
sounds like some hard limit imposed by something other than the code; I'd
just say "Discard the device 2G at a time" or something like that.)

A comment above the loop explaining in more detail that we iterate in
step sizes so that the utility can be interrupted would probably be
helpful.

Thanks,
-Eric