[Top] [All Lists]

Re: [PATCH] xfstests 255: add a seek_data/seek_hole tester

To: Sunil Mushran <sunil.mushran@xxxxxxxxxx>
Subject: Re: [PATCH] xfstests 255: add a seek_data/seek_hole tester
From: Josef Bacik <josef@xxxxxxxxxx>
Date: Wed, 29 Jun 2011 13:52:44 -0400
Cc: Christoph Hellwig <hch@xxxxxxxxxxxxx>, Dave Chinner <david@xxxxxxxxxxxxx>, linux-fsdevel@xxxxxxxxxxxxxxx, viro@xxxxxxxxxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx, linux-btrfs@xxxxxxxxxxxxxxx, xfs@xxxxxxxxxxx
In-reply-to: <4E0B5C6F.3060803@xxxxxxxxxx>
References: <1309275199-10801-1-git-send-email-josef@xxxxxxxxxx> <1309275199-10801-5-git-send-email-josef@xxxxxxxxxx> <20110629065306.GC1026@dastard> <20110629074021.GA26086@xxxxxxxxxxxxx> <4E0B5C6F.3060803@xxxxxxxxxx>
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv: Gecko/20110428 Fedora/3.1.10-1.fc15 Lightning/1.0b2 Thunderbird/3.1.10
On 06/29/2011 01:10 PM, Sunil Mushran wrote:
> On 06/29/2011 12:40 AM, Christoph Hellwig wrote:
>> On Wed, Jun 29, 2011 at 04:53:07PM +1000, Dave Chinner wrote:
>>> On Tue, Jun 28, 2011 at 11:33:19AM -0400, Josef Bacik wrote:
>>>> This is a test to make sure seek_data/seek_hole is acting like it
>>>> does on
>>>> Solaris.  It will check to see if the fs supports finding a hole or
>>>> not and will
>>>> adjust as necessary.
>>> So I just looked at this with an eye to validating an XFS
>>> implementation, and I came up with this list of stuff that the test
>>> does not cover that I'd need to test in some way:
>>>     - files with clean unwritten extents. Are they a hole or
>>>       data? What's SEEK_DATA supposed to return on layout like
>>>       hole-unwritten-data? i.e. needs to add fallocate to the
>>>       picture...
>>>     - files with dirty unwritten extents (i.e. dirty in memory,
>>>       not on disk). They are most definitely data, and most
>>>       filesystems will need a separate lookup path to detect
>>>       dirty unwritten ranges because the state is kept
>>>       separately (page cache vs extent cache).  Plenty of scope
>>>       for filesystem specific bugs here so needs a roubust test.
>> The discussion leading up to the resurrection of SEEK_HOLE/SEEK_DATA
>> was pretty much about that point.  The conclusion based on the Sun
>> documentation and common sense was that SEEK_DATA may only consider
>> unwritten extents as hole if the filesystem has a way to distinguish
>> plain unwritten extents and those that have been dirtied.  Else it
>> should be considered data.
>> Testing for making sure dirty preallocated areas aren't wrongly
>> reported sounds relatively easy, the rest falls into implementation
>> details, which imho is fine.  Not reporting preallocated extents
>> as holes just is a quality of implementation issue and not a bug.
> I agree. And if I might add my 2 cents that it would be much easier
> if we added another test that created files with all the worrisome boundary
> conditions and used SEEK_DATA/HOLE to copy the files and compared
> using md5sum. This would be far easier than one that expects a certain
> pos for each operation.

That's a great point, I think I will rig something like that up.  Thanks,


<Prev in Thread] Current Thread [Next in Thread>