Re: possible dev branch regression - xfstest 285/1k

[Eric Whitney <enwlinux@gmail.com>]

* Zheng Liu <gnehzuil.liu@gmail.com>:
> On Fri, Mar 15, 2013 at 06:28:18PM -0400, Eric Whitney wrote:
> > I'm seeing Xfstest 285 consistently fail for the 1k test case using the
> > latest dev branch while running on both x86 and ARM.  Subtest 08 is
> > the problem. From the test output:
> > 
> > 08. Test file with unwritten extents, only have unwritten pages
> > 08.01 SEEK_HOLE expected 0 or 4194304, got 11264.                 FAIL
> > 08.02 SEEK_HOLE expected 1 or 4194304, got 11264.                 FAIL
> > 08.03 SEEK_DATA expected 10240 or 10240, got 0.                   FAIL
> > 08.04 SEEK_DATA expected 10240 or 10240, got 1.                   FAIL
> > 
> > From previous discussions, we expect 285 to fail in the ext3 (nodelalloc,
> > no flex_bg, and no extents) test case, but in subtest 07.  It still does
> > that.
> > 
> > In the dev branch, reverting 4f42f80a8f - "ext4: use s_extent_max_zeroout_kb
> > value as number of kb" - results in success for 285 in the 1k test case.
> 
> Hi Eric,
> 
> I see what's going on.  First of all it isn't a bug. :-)  Please let me
> describe why it happens.
> 
> In this commit (4f42f80a8f), it tries to fix a bug that we never zero
> out an unwritten extent.  So after applied it, when an unwritten extent
> is converted, it could be zeroed out.  In xfstests #285 subtest 08 it
> preallocates an unwritten extent which is 4MB.  Then it writes some data
> at offset 10 * blocksize, which the length is one blocksize, and calles
> sync_file_range(2) to flush it.  So the call trace looks like:
> 
> ext4_fallocate()
>   ->ext4_map_blocks()
>     [one unwritten extent is allocated]
> ext4_file_write()
> ext4_da_writepages()
>   ->ext4_map_blocks() with EXT4_GET_BLOCKS_CREATE flag
>     ->ext4_ext_handle_uninitialized_extents()
>       ->ext4_ext_convert_to_initialized()
> 
> In ext4_ext_convert_to_initialized() it tries to zero out unwritten
> extent if condition is matched.  Let's see what happens.
> 
> case a) 1k block size
>   max_zeroout: 32
>   ee_len: 4096
>   allocated: 4086
>   m_len: 1
> 
> In this case, the following condition is matched.
> 
> fs/ext4/extents.c:3310
> 
>         else if (map->m_lblk - ee_block + map-m_len < max_zeroout)
>                  10          - 0        + 1         < 32
> 
> So unwritten extent [0,11] will be converted to written.  That is why
> 11264 (11 * 1k) is returned when we seek a hole from offset 0 and 1,
> and 0 and 1 are returned when we seek a data from offset 0 and 1.
> 
> case b) 4k block size
>   max_zeroout: 8
>   ee_len: 1024
>   allocated: 1014
>   m_len: 1
> 
> In this case, the above condition won't be matched.
> 
>         else if (map->m_lblk - ee_block + map-m_len < max_zeroout)
>                  10          - 0        + 1         < 8
> 
> So only one unwritten extent [10, 1] is converted, and the test can
> pass.
> 

Hi Zheng:

Thanks very much for taking the time to look at this and for your clear
explanation - much appreciated.  I'm happy to hear there's no reason to be
concerned about a regression, and that 4f42f80a8f simply exposed another
problem in xfstest 285 when applied to ext4.

Thanks,
Eric