Re: Usefulness of SEEK_HOLE / SEEK_DATA in generic_file_llseek()

[Josef Bacik <jbacik@fb.com>]

On 12/23/2013 06:12 PM, Jan Kara wrote:
>    Hello,
>
>    so I've now hit a xfstests failure for UDF which is caused by the
> implementation of SEEK_HOLE / SEEK_DATA in generic_file_llseek(). UDF uses
> that function as its .llseek method but it supports holes as any other unix
> filesystem (e.g. ext2). The test in xfstests assumes that when it creates a
> file by pwrite(fd, buf, bufsz, off), then SEEK_DATA on offset 0 should
> return 'off' (off is reasonably rounded) but that's not true for the
> implementation in generic_file_llseek().
>
> Now I'm not so much interested in that test itself - that can be tweaked to
> detect that case. But I rather wanted to ask - how useful is it to
> implement SEEK_HOLE / SEEK_DATA the way it is in generic_file_llseek()?
> Because it seems to me that any serious user will have to detect whether
> SEEK_HOLE / SEEK_DATA works reasonably and if not, fall back to some
> heuristic anyway. So why bother inventing bogus values in
> generic_file_llseek and thus making detection of working implementation
> harder?
I'm writing this from my in-laws so I'm going to make some assumptions 
about how the code works based on my memory, so sorry in advanced if 
this is completely wrong ;).

IIRC with the generic implementation we treat everything <= i_size as 
data and i_size as the first hole.  The way the spec works is that if we 
are currently at data and do seek_data then we just return our current 
offset, same for a hole.  In order to not be a jackass and have 
-EOPNOTSUPP for anybody who didn't implement seek_hole/seek_data I just 
did it this way where the only hole is the one that starts at i_size, so 
seek_data before that is going to return the value.

As far as detecting an optimized handling of seek_hole/seek_data I'm not 
sure what the best answer for that is.  I suppose seek_hole/seek_data is 
new enough that people will have checks for -EOPNOTSUPP anyway so we 
could just switch it back to that, but that seems like a regression of 
sorts to me.  I'm not married to the implementation as it is so I'm open 
to suggestions.  Thanks,

Josef