[PATCH v3] mkfs: Break block discard into chunks of 2 GB

[PATCH v3] mkfs: Break block discard into chunks of 2 GB

[Pavel Reichl]

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.

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");
+	}
+
+	/* 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.
-- 
2.23.0

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

[Chaitanya Kulkarni]

Not an XFS expert, but patch to handle ^C is been discussed on the
block layer mailing list which includes discard operations. [1]

This solution seems specific to one file system, which will lead to
code repetition for all the file systems which are in question.

How about we come up with the generic solution in the block-layer so
it can be reused for all the file systems ?

(fyi, I'm not aware of any drawbacks of handling ^C it in the block
layer and would like to learn if any).

[1] https://patchwork.kernel.org/patch/11234607/

-Chaitanya

On 11/27/2019 10:21 PM, 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.
>
> Signed-off-by: Pavel Reichl<preichl@redhat.com>
> ---

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

[Darrick J. Wong]

On Sat, Nov 30, 2019 at 10:01:17PM +0000, Chaitanya Kulkarni wrote:
> Not an XFS expert, but patch to handle ^C is been discussed on the
> block layer mailing list which includes discard operations. [1]

Heh, I wasn't aware of that. :)

> This solution seems specific to one file system, which will lead to
> code repetition for all the file systems which are in question.
> 
> How about we come up with the generic solution in the block-layer so
> it can be reused for all the file systems ?
> 
> (fyi, I'm not aware of any drawbacks of handling ^C it in the block
> layer and would like to learn if any).

The only one that I can think of is how to signal a partial completion,
but if you're only aborting on *fatal* signals then that doesn't matter.

Fixing the block layer seems like a better answer anyway.

> [1] https://patchwork.kernel.org/patch/11234607/

Though looking through that patch raises the question of whether xfs'
control loops also need to check for fatal signals, similar to what the
online scrub loops do?

--D

> -Chaitanya
> 
> On 11/27/2019 10:21 PM, 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.
> >
> > Signed-off-by: Pavel Reichl<preichl@redhat.com>
> > ---
> 

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

[Eric Sandeen]

On 11/30/19 4:01 PM, Chaitanya Kulkarni wrote:
> Not an XFS expert, but patch to handle ^C is been discussed on the
> block layer mailing list which includes discard operations. [1]
> 
> This solution seems specific to one file system, which will lead to
> code repetition for all the file systems which are in question.
> 
> How about we come up with the generic solution in the block-layer so
> it can be reused for all the file systems ?
> 
> (fyi, I'm not aware of any drawbacks of handling ^C it in the block
> layer and would like to learn if any).
> 
> [1] https://patchwork.kernel.org/patch/11234607/
> 
> -Chaitanya

It'd be great to fix this universally in the kernel but it seems like
that patch is in discussion for now, and TBH I don't see any real
drawbacks to looping in mkfs - it would also solve the problem on any
old kernel w/o the block layer change.

I'd propose that we go ahead w/ the mkfs change, and if/when the kernel
handles this better, and it's reasonable to expect that we're running
on a kernel where it can be interrupted, we could remove the mkfs loop
at a later date if we wanted to.

-Eric

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

[Christoph Hellwig]

On Wed, Dec 04, 2019 at 10:24:32AM -0600, Eric Sandeen wrote:
> It'd be great to fix this universally in the kernel but it seems like
> that patch is in discussion for now, and TBH I don't see any real
> drawbacks to looping in mkfs - it would also solve the problem on any
> old kernel w/o the block layer change.

The problem is that we throw out efficiency for no good reason.

> I'd propose that we go ahead w/ the mkfs change, and if/when the kernel
> handles this better, and it's reasonable to expect that we're running
> on a kernel where it can be interrupted, we could remove the mkfs loop
> at a later date if we wanted to.

I'd rather not touch mkfs if a trivial kernel patch handles the issue.

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

[Eric Sandeen]

On 12/4/19 11:26 AM, Christoph Hellwig wrote:
> On Wed, Dec 04, 2019 at 10:24:32AM -0600, Eric Sandeen wrote:
>> It'd be great to fix this universally in the kernel but it seems like
>> that patch is in discussion for now, and TBH I don't see any real
>> drawbacks to looping in mkfs - it would also solve the problem on any
>> old kernel w/o the block layer change.
> 
> The problem is that we throw out efficiency for no good reason.

The reason, as I stated earlier, is that up to this day, no kernel properly
handles this, and people are hitting this problem today.

And nobody has shown a significant efficiency loss.  (To be fair, nobody
has shown that there isn't a loss either, but in my admittedly small
test set I didn't see any meaningful overhead from the loop.)

>> I'd propose that we go ahead w/ the mkfs change, and if/when the kernel
>> handles this better, and it's reasonable to expect that we're running
>> on a kernel where it can be interrupted, we could remove the mkfs loop
>> at a later date if we wanted to.
> 
> I'd rather not touch mkfs if a trivial kernel patch handles the issue.

I don't think a trivial kernel patch to handle the issue even exists yet...

-Eric

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

[Darrick J. Wong]

On Wed, Dec 04, 2019 at 09:26:52AM -0800, Christoph Hellwig wrote:
> On Wed, Dec 04, 2019 at 10:24:32AM -0600, Eric Sandeen wrote:
> > It'd be great to fix this universally in the kernel but it seems like
> > that patch is in discussion for now, and TBH I don't see any real
> > drawbacks to looping in mkfs - it would also solve the problem on any
> > old kernel w/o the block layer change.
> 
> The problem is that we throw out efficiency for no good reason.

True...

> > I'd propose that we go ahead w/ the mkfs change, and if/when the kernel
> > handles this better, and it's reasonable to expect that we're running

How do we detect that the kernel will handle it better?

> > on a kernel where it can be interrupted, we could remove the mkfs loop
> > at a later date if we wanted to.
> 
> I'd rather not touch mkfs if a trivial kernel patch handles the issue.

Did some version of Tetsuo's patch even make it for 5.5?  It seemed to
call submit_bio_wait from within a blk_plug region, which seems way
worse.

--D

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

[Chaitanya Kulkarni]

On 12/04/2019 09:42 AM, Darrick J. Wong wrote:
> On Wed, Dec 04, 2019 at 09:26:52AM -0800, Christoph Hellwig wrote:
>> On Wed, Dec 04, 2019 at 10:24:32AM -0600, Eric Sandeen wrote:
>>> It'd be great to fix this universally in the kernel but it seems like
>>> that patch is in discussion for now, and TBH I don't see any real
>>> drawbacks to looping in mkfs - it would also solve the problem on any
>>> old kernel w/o the block layer change.
>>
>> The problem is that we throw out efficiency for no good reason.
>
> True...
>
>>> I'd propose that we go ahead w/ the mkfs change, and if/when the kernel
>>> handles this better, and it's reasonable to expect that we're running
>
> How do we detect that the kernel will handle it better?

>
>>> on a kernel where it can be interrupted, we could remove the mkfs loop
>>> at a later date if we wanted to.
>>
>> I'd rather not touch mkfs if a trivial kernel patch handles the issue.
>
> Did some version of Tetsuo's patch even make it for 5.5?  It seemed to
> call submit_bio_wait from within a blk_plug region, which seems way
> worse.
>

It did not yet, I can ping on the series with reference to this discussion.

> --D
>

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

[Eric Sandeen]

On 12/10/19 1:33 AM, Chaitanya Kulkarni wrote:
> On 12/04/2019 09:42 AM, Darrick J. Wong wrote:
>> On Wed, Dec 04, 2019 at 09:26:52AM -0800, Christoph Hellwig wrote:
>>> On Wed, Dec 04, 2019 at 10:24:32AM -0600, Eric Sandeen wrote:
>>>> It'd be great to fix this universally in the kernel but it seems like
>>>> that patch is in discussion for now, and TBH I don't see any real
>>>> drawbacks to looping in mkfs - it would also solve the problem on any
>>>> old kernel w/o the block layer change.
>>>
>>> The problem is that we throw out efficiency for no good reason.
>>
>> True...
>>
>>>> I'd propose that we go ahead w/ the mkfs change, and if/when the kernel
>>>> handles this better, and it's reasonable to expect that we're running
>>
>> How do we detect that the kernel will handle it better?
> 
>>
>>>> on a kernel where it can be interrupted, we could remove the mkfs loop
>>>> at a later date if we wanted to.
>>>
>>> I'd rather not touch mkfs if a trivial kernel patch handles the issue.
>>
>> Did some version of Tetsuo's patch even make it for 5.5?  It seemed to
>> call submit_bio_wait from within a blk_plug region, which seems way
>> worse.
>>
> 
> It did not yet, I can ping on the series with reference to this discussion.

That's fine, though I'm going to merge this patch in any case; we need a solution
for kernels that are not bleeding-edge new as well.

-eric

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

[Eric Sandeen]

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.
> 

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

[Darrick J. Wong]

On Mon, Dec 09, 2019 at 04:00:17PM -0600, Eric Sandeen wrote:
> 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... ");

This needs fflush(stdout); here to force the message out of stdio output
buffering.

--D

> 
> ...
> 
> +	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.
> > 

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

[Eric Sandeen]

On 12/9/19 5:34 PM, Darrick J. Wong wrote:
> On Mon, Dec 09, 2019 at 04:00:17PM -0600, Eric Sandeen wrote:
>> 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... ");
> This needs fflush(stdout); here to force the message out of stdio output
> buffering.
> 
> --D

I thought of that, then forgot.  ;)  Maybe we do need a formal v4 on the list.