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

[Eric Sandeen <sandeen@sandeen.net>]

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