Hi, On 12/24 2013 08:34 AM, Josef Bacik wrote: > 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, Maybe we could consider this semantics from user space tools point of view, i.e, Coreutils cp, sha1susm, GUN tar, AST(cp, mv, pax), etc... I once tried to introduce SEEK_HOLE/DATA to Coreutils to improve cp(1) about two years ago, that is intended to replace my old implementation via fiemap ioctl(2), http://lists.gnu.org/archive/html/bug-coreutils/2011-08/msg00102.html Currently cp(1) handle sparse files via fiemap ioctl(2) although it have some issues that have been discussed previously, but maybe we still have to support it as a back up given that it performs efficiently for dealing with sparse files on file systems without SEEK_HOLE/DATA support, therefore we can fall back to the old fiemap calls. Hence, I personally would like to see if we can return EOPNOTSUPP in this case. BTW, star do pre-checkups if the file system support SEEK_HOLE/DATA by checking EOPNOTSUPP, maybe this is in terms of the Solaris interface I'm not yet verified. Thanks, -Jeff -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
