Re: [PATCH] ext4:Fix a bug in ext4_ext_fiemap_cb().

[Dave Chinner <david@fromorbit.com>]

On Wed, Feb 23, 2011 at 10:41:42AM -0600, Eric Sandeen wrote:
> On 2/23/11 9:59 AM, Yongqiang Yang wrote:
> > 1] Delayed extents after a hole are neglected.
> > 
> >    By using find_get_pages() instead of find_get_page() to
> >    lookup pagecache, delayed extents can be found, because
> >    find_get_pages() with nr_pages=1 will return the next page
> >    in pagecache.
> > 
> > 2] Extents after a delayed extent or a hole are neglected as well.
> > 
> >    Fix it by accurating the request range by the result of
> >    ext4_ext_next_allocated_block().
> > 
> > Reported by Chris Mason <chris.mason@oracle.com>:
> > We've had reports on btrfs that cp is giving us files full of zeros
> > instead of actually copying them.  It was tracked down to a bug with
> > the btrfs fiemap implementation where it was returning holes for
> > delalloc ranges.
> > 
> > Newer versions of cp are trusting fiemap to tell it where the holes
> > are, which does seem like a pretty neat trick.
> > 
> > I decided to give xfs and ext4 a shot with a few tests cases too, xfs
> > passed with all the ones btrfs was getting wrong, and ext4 got the basic
> > delalloc case right.
> > $ mkfs.ext4 /dev/xxx
> > $ mount /dev/xxx /mnt
> > $ dd if=/dev/zero of=/mnt/foo bs=1M count=1
> > $ fiemap-test foo
> > ext:   0 logical: [       0..     255] phys:        0..     255 
> > flags: 0x007 tot: 256
> > 
> > Horray!  But once we throw a hole in, things go bad:
> > $ mkfs.ext4 /dev/xxx
> > $ mount /dev/xxx /mnt
> > $ dd if=/dev/zero of=/mnt/foo bs=1M count=1 seek=1
> > $ fiemap-test foo
> > < no output >
> > 
> > We've got a delalloc extent after the hole and ext4 fiemap didn't find
> > it.  If I run sync to kick the delalloc out:
> > $sync
> > $ fiemap-test foo
> > ext:   0 logical: [     256..     511] phys:    34048..   34303 
> > flags: 0x001 tot: 256
> > 
> > fiemap-test is sitting in my /usr/local/bin, and I have no idea how it
> > got there.  It's full of pretty comments so I know it isn't mine, but
> > you can grab it here:
> > 
> > http://oss.oracle.com/~mason/fiemap-test.c
> > 
> > xfsqa has a fiemap program too.
.....
> I think this will fall down if you have:
> 
> [ HOLE ][ DELALLOC ][ HOLE ][ ALLOCATED ] won't it?
> 
> i.e. your "end" will be the first block of "allocated" right?

This sound sort of problem indicateѕ to me that we need a generic
fiemap test in xfstests that exercises all these corner cases.
Perhaps something similar to the way I tested all the
XFS_IOC_ZERO_RANGE corner cases in test 242 by setting up all the
different hole/delalloc/unwritten/allocated combinations using
xfs_io and used the fiemap output as the golden output?

That would catch all of these problems in the different filesystems
that implement fiemap and make sure we notice regressions pretty
quickly.....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html