On Mon 23-12-13 19:34:18, 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. Correct. My point is that I actually don't see anything 'jackass' in returning -EOPNOTSUPP. I agree the way you've implemented SEEK_HOLE/SEEK_DATA is a correct implementation of the spec but is it a useful one? I mean e.g. cp(1) will rather want to fall back to its old heuristic if SEEK_DATA just returns the current file offset. And if the most of reasonable usecases of SEEK_DATA/SEEK_HOLE will need to detect the case of dumb SEEK_DATA/SEEK_HOLE implementation, then what's the point? > 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, Honza -- Jan Kara <jack@xxxxxxx> SUSE Labs, CR -- 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
