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

[Marco Stornelli]

2011/8/26 Dave Chinner <david@xxxxxxxxxxxxx>:
> On Thu, Aug 25, 2011 at 12:51:56AM -0600, Andreas Dilger wrote:
>> On 2011-08-25, at 12:40 AM, Dave Chinner wrote:
>> > On Thu, Aug 25, 2011 at 02:06:32AM -0400, Christoph Hellwig 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.
>> >>
>> >> Can you resend this with any updates that happened in the meantime?
>> >>
>> >> Dave also still had some comments about semantics, so it might be worth
>> >> to incorporate that as well.
>> >
>> > The main questions I had when looking at this was how we should
>> > handle unwritten extents - the only answer I got was along the lines
>> > of "we'll deal with that once filesystems have implemented
>> > something". That's a bit of a chicken-and-egg situation, and doesn't
>> > help me decide what is the best thing to do. I don't want to have to
>> > re-implement this code when it's decided i did the wrong thing
>> > initially.
>>
>> Let's first clarify what you mean by an unwritten extent?  Do you mean a
>> preallocated extent that returns 0 when read,
>
> Exactly that.
>
>> or do you mean a delayed
>> allocation extent that was written by the application that is still in
>> memory but not yet written to disk?
>
> That's not an unwritten extent - that's a delayed allocation extent ;)
>
>> Unfortunately, ZFS has no concept of preallocated extents, so we can't
>> look to it for precedent, but it definitely has delayed allocation.
>> Possibly if UFS on Solaris has SEEK_HOLE and also preallocated extents
>> (I have no idea) it could be tested?
>>
>> > The most basic question I really want answered is this:
>> >
>> >     - is preallocated space a HOLE, or is it DATA?
>> >
>> > Whatever the answer, I think it should be consistently
>> > presented by all filesystems that support preallocation, and it
>> > should be encoded into the generic SEEK_HOLE/SEEK_DATA tests....
>>
>> My thought would be that a preallocated extent is still a HOLE, because
>> it doesn't contain data that an application actually cares about.  On
>> the other hand, a delalloc extent is DATA because it has something that
>> an application cares about.
>
> OK, that's the way I'd expect to treat both preallocated and
> delalloc space.
>
>> > Answering that question then helps answer the more complex questions
>> > I had, like:
>> >
>> >     - what does SEEK_DATA return when you have a file layout
>> >       like "hole-prealloc-data"?
>>
>> I would think only the "data" part, since that is what the definition
>> of "SEEK_DATA" is IMHO.
>
> Agreed, that's the way I'd interpret it, too. So perhaps we need to
> ensure that this interpretation is actually tested by this test?
>
> How about some definitions to work by:
>
> Data: a range of the file that contains valid data, regardless of
> whether it exists in memory or on disk. The valid data can be
> preceeded and/or followed by an arbitrary number of zero bytes
> dependent on the underlying implementation of hole detection.
>
> Hole: a range of the file that contains no data or is made up
> entirely of  NULL (zero) data. Holes include preallocated ranges of
> files that have not had actual data written to them.
>
> Does that make sense? It has sufficient flexibility in it for the

No for me. A hole is made up of zero data? It's a strange definition
for me. A hole is simply no data. With this definition a hole can be
start from a block with data but partially filled, and fs should check
instead of presence of a block, the data content. I don't like it very
much. For the fallocate case, we need only a simple rule: no check is
done beyond eof. At this point it's clear that we can check the file
block allocation till i_size, don't care if there are other blocks
allocated. I don't know if it's applicable even for unwritten extents
case.

Marco
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/