[Top] [All Lists]

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

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH] xfstests 255: add a seek_data/seek_hole tester
From: Sunil Mushran <sunil.mushran@xxxxxxxxxx>
Date: Wed, 29 Jun 2011 10:10:07 -0700
Cc: Dave Chinner <david@xxxxxxxxxxxxx>, Josef Bacik <josef@xxxxxxxxxx>, linux-fsdevel@xxxxxxxxxxxxxxx, viro@xxxxxxxxxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx, linux-btrfs@xxxxxxxxxxxxxxx, xfs@xxxxxxxxxxx
In-reply-to: <20110629074021.GA26086@xxxxxxxxxxxxx>
References: <1309275199-10801-1-git-send-email-josef@xxxxxxxxxx> <1309275199-10801-5-git-send-email-josef@xxxxxxxxxx> <20110629065306.GC1026@dastard> <20110629074021.GA26086@xxxxxxxxxxxxx>
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv: Gecko/20110516 Lightning/1.0b2 Thunderbird/3.1.10
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

        - 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.

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