Re: xfstests 252 failure

[Josef Bacik <josef@redhat.com>]

On 06/14/2011 12:06 PM, Eric Sandeen wrote:
> On 6/14/11 10:41 AM, Allison Henderson wrote:
>> Hi all,
>>
>> I just wanted to get some ideas moving on this question before too
>> much time goes by. Ext4 is currently failing xfstest 252, test number
>> 12. Currently test 12 is:
>>
>>     $XFS_IO_PROG $xfs_io_opt -f -c "truncate 20k" \
>>         -c "$alloc_cmd 0 20k" \   
>>         -c "pwrite 8k 4k" -c "fsync" \       
>>         -c "$zero_cmd 4k 12k" \
>>         -c "$map_cmd -v" $testfile | $filter_cmd
>>     [ $? -ne 0 ]&&  die_now
> 
> so the file should go through these steps:
> (H=hole, P=prealloc, D=data)
> 
> 0k                       20k
> |  H |  H |  H |  H |  H | (truncate)
> |  P |  P |  P |  P |  P | (alloc_cmd)
> |  P |  P |  D |  P |  P | (pwrite)
> <fsync>                    (fsync)
> |  P |  H |  H |  H |  P | (punch)
>  
>> and the output is:
>>
>>         12. unwritten -> data -> unwritten
>> 0: [0..7]: unwritten
>> 1: [8..31]: hole
>> 2: [32..39]: unwritten
>>
>> Ext4 gets data extents here instead of unwritten extents. 
> 
> so it's like this?
> 
> 0: [0..7]: data
> 1: [8..31]: hole
> 2: [32..39]: data
> 
>> I did some
>> investigating and it looks like the fsync command causes the extents
>> to be written out before the punch hole operation starts. It looks
>> like what happens is that when an unwritten extent gets written to,
>> it doesnt always split the extent. If the extent is small enough,
>> then it just zeros out the portions that are not written to, and the
>> whole extent becomes a written extent. Im not sure if that is
>> incorrect or if we need to change the test to not compare the extent
>> types.
> 
> Yes, it does do that IIRC.
> 
> I probably need to look closer, but any test which expects exact
> layouts from a filesystem after a series of operations is probably
> expecting too much...
> 
> From a data integrity perspective, written zeros is as good as a hole is
> as good as preallocated space, so I suppose those should all be acceptable,
> though I guess "punch" should result in holes exactly as requested.
> 
>> It looks to me that the code in ext4 that does this is supposed to be
>> an optimization to help reduce fragmentation. We could change the
>> filters to print just "extent" instead of "unwritten" or "data", but
>> I realize that probably makes the test a lot less effective for xfs.
>> If anyone can think of some more elegant fixes, please let me know.
>> Thx!
> 
> Josef, what do you think?  It's your test originally.  :)
> 

Yes, a test that was really only meant to test the block based fiemap
since they all act in a dumb and easy to verify way.  I think if we want
to keep this test we should probably have it just recognize these little
optimizations so it doesn't freak out.  Thanks,

Josef