On Wed, Nov 28, 2007 at 02:56:54PM -0800, Nicholas Miell wrote: > > On Wed, 2007-11-28 at 15:02 -0500, Josef Bacik wrote: > > Hello, > > > > This is my first pass at implementing SEEK_HOLE/SEEK_DATA. This has been in > > solaris for about a year now, and is described here > > > > http://docs.sun.com/app/docs/doc/819-2241/lseek-2?l=en&a=view&q=SEEK_HOLE > > http://blogs.sun.com/roller/page/bonwick?entry=seek_hole_and_seek_data > > > > I've added a file operation to allow filesystems to override the default > > seek_hole_data function, which just loops through bmap looking for either a hole > > or data. I've tested this and it seems to work well. I ran my testcase on a > > solaris box to make sure I got consistent results (I just ran my test script on > > the solaris box, I haven't looked at any of their code in case thats a concern). > > All comments welcome. Thank you, > > > > Josef > > I stand by my belief that SEEK_HOLE/SEEK_DATA is a lousy interface. > > It abuses the seek operation to become a query operation, it requires a > total number of system calls proportional to the number holes+data and > it isn't general enough for other similar uses (e.g. total number of > contiguous extents, compressed extents, offline extents, extents > currently shared with other inodes, extents embedded in the inode > (tails), etc.) > > Something like the following would be much better: > > int getfilextents(int fd, off_t offset, int type, size_t *length, struct > extent *extents) > > with > > int fd: open file > > offset: offset in file to start reporting extents > > type: one of EXTENT_TYPE_HOLE, EXTENT_TYPE_DATA, EXTENT_TYPE_EXTENTS, > EXTENT_TYPE_COMPRESSED, EXTENT_TYPE_UNCOMPRESSED etc. > > length: in/out parameter, on entry contains length of extents array, on > exit contains number of valid entries in the extents array or total > number of extents remaining in the file, whichever is larger > > extents: array of struct extent { off_t offset; off_t length }, only > updated if non-NULL > > Making the type parameter a bitmask and adding a type member to struct > extent could be useful so that multiple types of extents could be > reported at once could be useful, too. (But you end up with weird cases > like data extents overlapping with compressed extents.) > > Actually, now that I've searched my mailbox, Andreas Dilger's FIEMAP > proposal is pretty much what I suggest here and is certainly superior to > Sun's SEEK_HOLE/SEEK_DATA. > Agreed, however in speaking hch and others the consensus was FIEMAP was good, however there was no reason why SEEK_HOLE/SEEK_DATA shouldn't also be implemented, and then at some point down the road when a generic FIEMAP is in place either change the SEEK_HOLE/SEEK_DATA implementation to try to use FIEMAP by default and then fall back on bmap if it has to, or some other such operation. I'm cool with passing on this implementation in preference for FIEMAP, but given the discussion I had earlier this week with some of the other fs people the general thought was go ahead and do this for now. Josef - 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
