[PATCH] iomap: Make sure iomap_end is called after iomap_begin

[PATCH] iomap: Make sure iomap_end is called after iomap_begin

[Andreas Gruenbacher]

Make sure iomap_end is always called when iomap_begin succeeds: the
filesystem may take locks in iomap_begin and release them in iomap_end,
for example.

Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
 fs/iomap/apply.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/fs/iomap/apply.c b/fs/iomap/apply.c
index 76925b40b5fd..c00a14d825db 100644
--- a/fs/iomap/apply.c
+++ b/fs/iomap/apply.c
@@ -46,10 +46,10 @@ iomap_apply(struct inode *inode, loff_t pos, loff_t length, unsigned flags,
 	ret = ops->iomap_begin(inode, pos, length, flags, &iomap, &srcmap);
 	if (ret)
 		return ret;
-	if (WARN_ON(iomap.offset > pos))
-		return -EIO;
-	if (WARN_ON(iomap.length == 0))
-		return -EIO;
+	if (WARN_ON(iomap.offset > pos) || WARN_ON(iomap.length == 0)) {
+		written = -EIO;
+		goto out;
+	}
 
 	trace_iomap_apply_dstmap(inode, &iomap);
 	if (srcmap.type != IOMAP_HOLE)
@@ -80,6 +80,7 @@ iomap_apply(struct inode *inode, loff_t pos, loff_t length, unsigned flags,
 	written = actor(inode, pos, length, data, &iomap,
 			srcmap.type != IOMAP_HOLE ? &srcmap : &iomap);
 
+out:
 	/*
 	 * Now the data has been copied, commit the range we've copied.  This
 	 * should not fail unless the filesystem has had a fatal error.

base-commit: 97e0204907ac4c42c6e94ef466a047523f34b853
-- 
2.26.2

Re: [PATCH] iomap: Make sure iomap_end is called after iomap_begin

[Dave Chinner]

On Mon, Jun 15, 2020 at 06:02:44PM +0200, Andreas Gruenbacher wrote:
> Make sure iomap_end is always called when iomap_begin succeeds: the
> filesystem may take locks in iomap_begin and release them in iomap_end,
> for example.

Ok, i get that from the patch, but I don't know anything else about
this problem, and nor will anyone else trying to determine if this
is a fix they need to backport to other kernels. Can you add some
more information to the commit message, such as how was this found
and what filesystems it affects? It would also be good to know what
commit introduced this issue and whether it need stable back ports
(i.e. a Fixes tag).

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH] iomap: Make sure iomap_end is called after iomap_begin

[Matthew Wilcox]

On Tue, Jun 16, 2020 at 09:32:39AM +1000, Dave Chinner wrote:
> On Mon, Jun 15, 2020 at 06:02:44PM +0200, Andreas Gruenbacher wrote:
> > Make sure iomap_end is always called when iomap_begin succeeds: the
> > filesystem may take locks in iomap_begin and release them in iomap_end,
> > for example.
> 
> Ok, i get that from the patch, but I don't know anything else about
> this problem, and nor will anyone else trying to determine if this
> is a fix they need to backport to other kernels. Can you add some
> more information to the commit message, such as how was this found
> and what filesystems it affects? It would also be good to know what
> commit introduced this issue and whether it need stable back ports
> (i.e. a Fixes tag).

I'd assume Andreas is looking at converting a filesystem to use iomap,
since this problem only occurs for filesystems which have returned an
invalid extent.

I almost wonder if this should return -EFSCORRUPTED rather than -EIO.
Um, except that's currently a per-fs define.  Is it time to move that
up to errno.h?

Re: [PATCH] iomap: Make sure iomap_end is called after iomap_begin

[Dave Chinner]

On Mon, Jun 15, 2020 at 04:44:37PM -0700, Matthew Wilcox wrote:
> On Tue, Jun 16, 2020 at 09:32:39AM +1000, Dave Chinner wrote:
> > On Mon, Jun 15, 2020 at 06:02:44PM +0200, Andreas Gruenbacher wrote:
> > > Make sure iomap_end is always called when iomap_begin succeeds: the
> > > filesystem may take locks in iomap_begin and release them in iomap_end,
> > > for example.
> > 
> > Ok, i get that from the patch, but I don't know anything else about
> > this problem, and nor will anyone else trying to determine if this
> > is a fix they need to backport to other kernels. Can you add some
> > more information to the commit message, such as how was this found
> > and what filesystems it affects? It would also be good to know what
> > commit introduced this issue and whether it need stable back ports
> > (i.e. a Fixes tag).
> 
> I'd assume Andreas is looking at converting a filesystem to use iomap,
> since this problem only occurs for filesystems which have returned an
> invalid extent.

Well, I can assume it's gfs2, but you know what happens when you
assume something....

> I almost wonder if this should return -EFSCORRUPTED rather than -EIO.

We've typically used -EFSCORRUPTED for metadata corruptions and -EIO
for data IO errors that result from metadata corruption. -EIO
implies that the IO failed and the state of the data is
indeterminate (i.e. may be corrupted), but the filesystem is still
ok, whereas EFSCORRUPTED implies the filesystem needs to have fsck
run on it to diagnose and fix the metadata corruption.

This code sort of spans the grey area between them. The error could
come from in in-memory bug and not actually a filesystem corruption,
so it's not clear that corruption is the right thing to report
here...

> Um, except that's currently a per-fs define.  Is it time to move that
> up to errno.h?

Has the "EFSCORRUPTED = EUCLEAN is stupid - let's break a
longstanding user API by defining it to something completely new"
yelling died down enough in the 6 months since this was last
proposed?

https://lore.kernel.org/linux-xfs/20191031010736.113783-1-Valdis.Kletnieks@vt.edu/
https://lore.kernel.org/linux-xfs/20191104014510.102356-11-Valdis.Kletnieks@vt.edu/

We've only been trying to get a generic -EFSCORRUPTED definition
into the kernel errno headers for ~15 years... :(

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH] iomap: Make sure iomap_end is called after iomap_begin

[Bob Peterson]

----- Original Message -----
> > I'd assume Andreas is looking at converting a filesystem to use iomap,
> > since this problem only occurs for filesystems which have returned an
> > invalid extent.
> 
> Well, I can assume it's gfs2, but you know what happens when you
> assume something....

Yes, it's gfs2, which already has iomap. I found the bug while just browsing
the code: gfs2 takes a lock in the begin code. If there's an error,
however unlikely, the end code is never called, so we would never unlock.
It doesn't matter to me whether the error is -EIO because it's very unlikely
in the first place. I haven't looked back to see where the problem was
introduced, but I suspect it should be ported back to stable releases.

Bob Peterson

Re: [PATCH] iomap: Make sure iomap_end is called after iomap_begin

[Matthew Wilcox]

On Tue, Jun 16, 2020 at 08:17:28AM -0400, Bob Peterson wrote:
> ----- Original Message -----
> > > I'd assume Andreas is looking at converting a filesystem to use iomap,
> > > since this problem only occurs for filesystems which have returned an
> > > invalid extent.
> > 
> > Well, I can assume it's gfs2, but you know what happens when you
> > assume something....
> 
> Yes, it's gfs2, which already has iomap. I found the bug while just browsing
> the code: gfs2 takes a lock in the begin code. If there's an error,
> however unlikely, the end code is never called, so we would never unlock.
> It doesn't matter to me whether the error is -EIO because it's very unlikely
> in the first place. I haven't looked back to see where the problem was
> introduced, but I suspect it should be ported back to stable releases.

It shouldn't just be "unlikely", it should be impossible.  This is the
iomap code checking whether you've returned an extent which doesn't cover
the range asked for.  I don't think it needs to be backported, and I'm
pretty neutral on whether it needs to be applied.

Re: [PATCH] iomap: Make sure iomap_end is called after iomap_begin

[Andreas Gruenbacher]

On Tue, Jun 16, 2020 at 3:23 PM Matthew Wilcox <willy@infradead.org> wrote:
> On Tue, Jun 16, 2020 at 08:17:28AM -0400, Bob Peterson wrote:
> > ----- Original Message -----
> > > > I'd assume Andreas is looking at converting a filesystem to use iomap,
> > > > since this problem only occurs for filesystems which have returned an
> > > > invalid extent.
> > >
> > > Well, I can assume it's gfs2, but you know what happens when you
> > > assume something....
> >
> > Yes, it's gfs2, which already has iomap. I found the bug while just browsing
> > the code: gfs2 takes a lock in the begin code. If there's an error,
> > however unlikely, the end code is never called, so we would never unlock.
> > It doesn't matter to me whether the error is -EIO because it's very unlikely
> > in the first place. I haven't looked back to see where the problem was
> > introduced, but I suspect it should be ported back to stable releases.
>
> It shouldn't just be "unlikely", it should be impossible.  This is the
> iomap code checking whether you've returned an extent which doesn't cover
> the range asked for.  I don't think it needs to be backported, and I'm
> pretty neutral on whether it needs to be applied.

Right, when these warnings trigger, the filesystem has already screwed
up; this fix only makes things less bad. Those kinds of issues are
very likely to be fixed long before the code hits users, so it
shouldn't be backported.

This bug was in iomap_apply right from the start, so:

Fixes: ae259a9c8593 ("fs: introduce iomap infrastructure")

Thanks,
Andreas

Re: [PATCH] iomap: Make sure iomap_end is called after iomap_begin

[Darrick J. Wong]

On Tue, Jun 16, 2020 at 03:57:08PM +0200, Andreas Gruenbacher wrote:
> On Tue, Jun 16, 2020 at 3:23 PM Matthew Wilcox <willy@infradead.org> wrote:
> > On Tue, Jun 16, 2020 at 08:17:28AM -0400, Bob Peterson wrote:
> > > ----- Original Message -----
> > > > > I'd assume Andreas is looking at converting a filesystem to use iomap,
> > > > > since this problem only occurs for filesystems which have returned an
> > > > > invalid extent.
> > > >
> > > > Well, I can assume it's gfs2, but you know what happens when you
> > > > assume something....
> > >
> > > Yes, it's gfs2, which already has iomap. I found the bug while just browsing
> > > the code: gfs2 takes a lock in the begin code. If there's an error,
> > > however unlikely, the end code is never called, so we would never unlock.
> > > It doesn't matter to me whether the error is -EIO because it's very unlikely
> > > in the first place. I haven't looked back to see where the problem was
> > > introduced, but I suspect it should be ported back to stable releases.
> >
> > It shouldn't just be "unlikely", it should be impossible.  This is the
> > iomap code checking whether you've returned an extent which doesn't cover
> > the range asked for.  I don't think it needs to be backported, and I'm
> > pretty neutral on whether it needs to be applied.
> 
> Right, when these warnings trigger, the filesystem has already screwed
> up; this fix only makes things less bad. Those kinds of issues are
> very likely to be fixed long before the code hits users, so it
> shouldn't be backported.
> 
> This bug was in iomap_apply right from the start, so:
> 
> Fixes: ae259a9c8593 ("fs: introduce iomap infrastructure")

So... you found this through code inspection, and not because you
actually hit this on gfs2, or any of the other iomap users?

I generally think this looks ok, but I want to know if I should be
looking deeper. :)

--D

> Thanks,
> Andreas
> 

Re: [PATCH] iomap: Make sure iomap_end is called after iomap_begin

[Andreas Grünbacher]

Am Di., 16. Juni 2020 um 18:26 Uhr schrieb Darrick J. Wong
<darrick.wong@oracle.com>:
> On Tue, Jun 16, 2020 at 03:57:08PM +0200, Andreas Gruenbacher wrote:
> > On Tue, Jun 16, 2020 at 3:23 PM Matthew Wilcox <willy@infradead.org> wrote:
> > > On Tue, Jun 16, 2020 at 08:17:28AM -0400, Bob Peterson wrote:
> > > > ----- Original Message -----
> > > > > > I'd assume Andreas is looking at converting a filesystem to use iomap,
> > > > > > since this problem only occurs for filesystems which have returned an
> > > > > > invalid extent.
> > > > >
> > > > > Well, I can assume it's gfs2, but you know what happens when you
> > > > > assume something....
> > > >
> > > > Yes, it's gfs2, which already has iomap. I found the bug while just browsing
> > > > the code: gfs2 takes a lock in the begin code. If there's an error,
> > > > however unlikely, the end code is never called, so we would never unlock.
> > > > It doesn't matter to me whether the error is -EIO because it's very unlikely
> > > > in the first place. I haven't looked back to see where the problem was
> > > > introduced, but I suspect it should be ported back to stable releases.
> > >
> > > It shouldn't just be "unlikely", it should be impossible.  This is the
> > > iomap code checking whether you've returned an extent which doesn't cover
> > > the range asked for.  I don't think it needs to be backported, and I'm
> > > pretty neutral on whether it needs to be applied.
> >
> > Right, when these warnings trigger, the filesystem has already screwed
> > up; this fix only makes things less bad. Those kinds of issues are
> > very likely to be fixed long before the code hits users, so it
> > shouldn't be backported.
> >
> > This bug was in iomap_apply right from the start, so:
> >
> > Fixes: ae259a9c8593 ("fs: introduce iomap infrastructure")
>
> So... you found this through code inspection, and not because you
> actually hit this on gfs2, or any of the other iomap users?

Bob did, yes. I've only hit those warnings in the very early stages of
gfs2 iomap development, long before that code was even posted for
review.

> I generally think this looks ok, but I want to know if I should be
> looking deeper. :)

It's really supposed to be a simple, straight forward fix only.

Thanks,
Andreas

Re: [PATCH] iomap: Make sure iomap_end is called after iomap_begin

[Bob Peterson]

----- Original Message -----
> So... you found this through code inspection, and not because you
> actually hit this on gfs2, or any of the other iomap users?
> 
> I generally think this looks ok, but I want to know if I should be
> looking deeper. :)
> 
> --D

Correct. Code-inspection only. I never actually hit the problem.
If those errors are really so unusual and catastrophic, perhaps
we should just change them to BUG_ONs or something instead.
Why bother unlocking if we're already 1.9 meters underground?

Bob Peterson

Re: [PATCH] iomap: Make sure iomap_end is called after iomap_begin

[Dave Chinner]

On Tue, Jun 16, 2020 at 12:38:38PM -0400, Bob Peterson wrote:
> ----- Original Message -----
> > So... you found this through code inspection, and not because you
> > actually hit this on gfs2, or any of the other iomap users?
> > 
> > I generally think this looks ok, but I want to know if I should be
> > looking deeper. :)
> > 
> > --D
> 
> Correct. Code-inspection only. I never actually hit the problem.
> If those errors are really so unusual and catastrophic, perhaps
> we should just change them to BUG_ONs or something instead.

We do not panic a machine because a detectable data or filesystem
corruption event has occurred. We have a viable error path to tell
userspace a fatal IO error occurred so that is all the generic
infrastructure should be doing.

If a loud warning needs to be issued, then WARN_ON_ONCE() may be
appropriate, though I suspect even that is overkill for this
situation....

> Why bother unlocking if we're already 1.9 meters underground?

Because then a filesystem that has shutdown because it has
recognised that it is walking dead can be unmounted and the user can
then run an autopsy to find and fix the problem without having to
reboot the machine....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH] iomap: Make sure iomap_end is called after iomap_begin

[Darrick J. Wong]

On Mon, Jun 15, 2020 at 06:02:44PM +0200, Andreas Gruenbacher wrote:
> Make sure iomap_end is always called when iomap_begin succeeds: the
> filesystem may take locks in iomap_begin and release them in iomap_end,
> for example.
> 
> Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
> ---
>  fs/iomap/apply.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/iomap/apply.c b/fs/iomap/apply.c
> index 76925b40b5fd..c00a14d825db 100644
> --- a/fs/iomap/apply.c
> +++ b/fs/iomap/apply.c
> @@ -46,10 +46,10 @@ iomap_apply(struct inode *inode, loff_t pos, loff_t length, unsigned flags,
>  	ret = ops->iomap_begin(inode, pos, length, flags, &iomap, &srcmap);
>  	if (ret)
>  		return ret;
> -	if (WARN_ON(iomap.offset > pos))
> -		return -EIO;
> -	if (WARN_ON(iomap.length == 0))
> -		return -EIO;
> +	if (WARN_ON(iomap.offset > pos) || WARN_ON(iomap.length == 0)) {

<urk> Forgot to actually review the original patch. :P

Why combine these WARN_ON?  Before, you could distinguish between your
iomap_begin method returning zero length vs. bad offset.

--D

> +		written = -EIO;
> +		goto out;
> +	}
>  
>  	trace_iomap_apply_dstmap(inode, &iomap);
>  	if (srcmap.type != IOMAP_HOLE)
> @@ -80,6 +80,7 @@ iomap_apply(struct inode *inode, loff_t pos, loff_t length, unsigned flags,
>  	written = actor(inode, pos, length, data, &iomap,
>  			srcmap.type != IOMAP_HOLE ? &srcmap : &iomap);
>  
> +out:
>  	/*
>  	 * Now the data has been copied, commit the range we've copied.  This
>  	 * should not fail unless the filesystem has had a fatal error.
> 
> base-commit: 97e0204907ac4c42c6e94ef466a047523f34b853
> -- 
> 2.26.2
> 

Re: [PATCH] iomap: Make sure iomap_end is called after iomap_begin

[Andreas Gruenbacher]

On Thu, Jun 18, 2020 at 3:39 AM Darrick J. Wong <darrick.wong@oracle.com> wrote:
> On Mon, Jun 15, 2020 at 06:02:44PM +0200, Andreas Gruenbacher wrote:
> > Make sure iomap_end is always called when iomap_begin succeeds: the
> > filesystem may take locks in iomap_begin and release them in iomap_end,
> > for example.
> >
> > Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
> > ---
> >  fs/iomap/apply.c | 9 +++++----
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/fs/iomap/apply.c b/fs/iomap/apply.c
> > index 76925b40b5fd..c00a14d825db 100644
> > --- a/fs/iomap/apply.c
> > +++ b/fs/iomap/apply.c
> > @@ -46,10 +46,10 @@ iomap_apply(struct inode *inode, loff_t pos, loff_t length, unsigned flags,
> >       ret = ops->iomap_begin(inode, pos, length, flags, &iomap, &srcmap);
> >       if (ret)
> >               return ret;
> > -     if (WARN_ON(iomap.offset > pos))
> > -             return -EIO;
> > -     if (WARN_ON(iomap.length == 0))
> > -             return -EIO;
> > +     if (WARN_ON(iomap.offset > pos) || WARN_ON(iomap.length == 0)) {
>
> <urk> Forgot to actually review the original patch. :P
>
> Why combine these WARN_ON?  Before, you could distinguish between your
> iomap_begin method returning zero length vs. bad offset.

Right, the WARN_ONs shouldn't both report the same line number. I'll
send an update.

Thanks,
Andreas

Re: [PATCH] iomap: Make sure iomap_end is called after iomap_begin

[Matthew Wilcox]

On Wed, Jun 17, 2020 at 06:39:01PM -0700, Darrick J. Wong wrote:
> > -	if (WARN_ON(iomap.offset > pos))
> > -		return -EIO;
> > -	if (WARN_ON(iomap.length == 0))
> > -		return -EIO;
> > +	if (WARN_ON(iomap.offset > pos) || WARN_ON(iomap.length == 0)) {
> 
> Why combine these WARN_ON?  Before, you could distinguish between your
> iomap_begin method returning zero length vs. bad offset.

Does it matter?  They're both the same problem -- the filesystem has
returned an invalid iomap.  I'd go further and combine the two:

	if (WARN_ON(iomap.offset > pos || iomap.length == 0)) {

that'll save a few bytes of .text

Re: [PATCH] iomap: Make sure iomap_end is called after iomap_begin

[Andreas Gruenbacher]

On Thu, Jun 18, 2020 at 2:32 PM Matthew Wilcox <willy@infradead.org> wrote:
> On Wed, Jun 17, 2020 at 06:39:01PM -0700, Darrick J. Wong wrote:
> > > -   if (WARN_ON(iomap.offset > pos))
> > > -           return -EIO;
> > > -   if (WARN_ON(iomap.length == 0))
> > > -           return -EIO;
> > > +   if (WARN_ON(iomap.offset > pos) || WARN_ON(iomap.length == 0)) {
> >
> > Why combine these WARN_ON?  Before, you could distinguish between your
> > iomap_begin method returning zero length vs. bad offset.
>
> Does it matter?  They're both the same problem -- the filesystem has
> returned an invalid iomap.  I'd go further and combine the two:
>
>         if (WARN_ON(iomap.offset > pos || iomap.length == 0)) {
>
> that'll save a few bytes of .text

That would be fine by me as well. Christoph may have wanted separate
warnings for a particular reason though.

Andreas

Re: [PATCH] iomap: Make sure iomap_end is called after iomap_begin

[Christoph Hellwig]

On Thu, Jun 18, 2020 at 02:37:37PM +0200, Andreas Gruenbacher wrote:
> On Thu, Jun 18, 2020 at 2:32 PM Matthew Wilcox <willy@infradead.org> wrote:
> > On Wed, Jun 17, 2020 at 06:39:01PM -0700, Darrick J. Wong wrote:
> > > > -   if (WARN_ON(iomap.offset > pos))
> > > > -           return -EIO;
> > > > -   if (WARN_ON(iomap.length == 0))
> > > > -           return -EIO;
> > > > +   if (WARN_ON(iomap.offset > pos) || WARN_ON(iomap.length == 0)) {
> > >
> > > Why combine these WARN_ON?  Before, you could distinguish between your
> > > iomap_begin method returning zero length vs. bad offset.
> >
> > Does it matter?  They're both the same problem -- the filesystem has
> > returned an invalid iomap.  I'd go further and combine the two:
> >
> >         if (WARN_ON(iomap.offset > pos || iomap.length == 0)) {
> >
> > that'll save a few bytes of .text
> 
> That would be fine by me as well. Christoph may have wanted separate
> warnings for a particular reason though.

Yes.  The line number in the WARN_ON will tell you which condition
you if they are separate, which is really useful to diagnose what is
going on.

Re: [PATCH] iomap: Make sure iomap_end is called after iomap_begin

[Matthew Wilcox]

On Thu, Jun 18, 2020 at 06:56:39AM -0700, Christoph Hellwig wrote:
> On Thu, Jun 18, 2020 at 02:37:37PM +0200, Andreas Gruenbacher wrote:
> > On Thu, Jun 18, 2020 at 2:32 PM Matthew Wilcox <willy@infradead.org> wrote:
> > > On Wed, Jun 17, 2020 at 06:39:01PM -0700, Darrick J. Wong wrote:
> > > > > -   if (WARN_ON(iomap.offset > pos))
> > > > > -           return -EIO;
> > > > > -   if (WARN_ON(iomap.length == 0))
> > > > > -           return -EIO;
> > > > > +   if (WARN_ON(iomap.offset > pos) || WARN_ON(iomap.length == 0)) {
> > > >
> > > > Why combine these WARN_ON?  Before, you could distinguish between your
> > > > iomap_begin method returning zero length vs. bad offset.
> > >
> > > Does it matter?  They're both the same problem -- the filesystem has
> > > returned an invalid iomap.  I'd go further and combine the two:
> > >
> > >         if (WARN_ON(iomap.offset > pos || iomap.length == 0)) {
> > >
> > > that'll save a few bytes of .text
> > 
> > That would be fine by me as well. Christoph may have wanted separate
> > warnings for a particular reason though.
> 
> Yes.  The line number in the WARN_ON will tell you which condition
> you if they are separate, which is really useful to diagnose what is
> going on.

Thinking about it, wouldn't the second test be better replaced with:

	if (WARN_ON(iomap.offset + iomap.length <= pos))

in case the filesystem returns an extent which finishes before pos?
This would be a superset of the test for length being 0.

Re: [PATCH] iomap: Make sure iomap_end is called after iomap_begin

[Christoph Hellwig]

On Thu, Jun 18, 2020 at 08:15:23AM -0700, Matthew Wilcox wrote:
> Thinking about it, wouldn't the second test be better replaced with:
> 
> 	if (WARN_ON(iomap.offset + iomap.length <= pos))
> 
> in case the filesystem returns an extent which finishes before pos?
> This would be a superset of the test for length being 0.

The idea was to tell what is wrong.  Both with the initial iomap work
and later the COW support I had all kinds of weird scenarious during
bringup where an obvious error has been very helpful.