Re: Introduce SEEK_DATA/SEEK_HOLE to XFS V5

Mark Tinguely
2012-01-11T21:12:00Z

xfs_has_unwritten_buffer() always returns the offset of the first
dirty unwritten page. This can cause xfs_seek_data() and xfs_seek_hole()
to give the wrong results in certain circumstances.

In xfs_seek_data(), every page past first dirty/unwritten page in the
unwritten extent will be reported as data.

in xfs_seek_data():
+ /*
+ * Landed in an unwritten extent, try to find out the data
+ * buffer offset from page cache firstly. If nothing was
+ * found, treat it as a hole, and skip to check the next
+ * extent, something just like above.
+ */
+ if (map[0].br_state == XFS_EXT_UNWRITTEN) {
+ if (xfs_has_unwritten_buffer(inode, &map[0],
+ PAGECACHE_TAG_DIRTY,
+ &offset) ||
+ xfs_has_unwritten_buffer(inode, &map[0],
+ PAGECACHE_TAG_WRITEBACK,
+ &offset)) {
+ offset = max_t(loff_t, seekoff, offset);
+ break;
+ }

Since the xfs_has_unwritten_buffer() returns the offset of the first
dirty/unwritten page (the first page in this example), the max_t()
comparison will say that every page after the first dirty page has
data.

-----

xfs_seek_hole() can only find a hole if it precedes the first dirty
page.

in xfs_seek_hole():
+ /*
+ * Landed in an unwritten extent, try to lookup the page
+ * cache to find out if there is dirty data or not. If
+ * nothing was found, treate it as a hole. If there has
+ * dirty data and its offset starts past both the start
+ * block of the map and the current seek offset, it should
+ * be treated as hole too. Otherwise, go through the next
+ * extent to fetch holes.
+ */
+ if (map[0].br_state == XFS_EXT_UNWRITTEN) {
+ if (xfs_has_unwritten_buffer(inode, &map[0],
+ PAGECACHE_TAG_DIRTY,
+ &offset) ||
+ xfs_has_unwritten_buffer(inode, &map[0],
+ PAGECACHE_TAG_WRITEBACK,
+ &offset)) {
+ if (offset > max_t(loff_t, seekoff,
+ XFS_FSB_TO_B(mp,
+ map[0].br_startoff))) {
+ offset = max_t(loff_t, seekoff,
+ XFS_FSB_TO_B(mp,
+ map[0].br_startoff));
+ break;
+ }
+ } else {
+ offset = max_t(loff_t, seekoff,
+ XFS_FSB_TO_B(mp, map[0].br_startoff));
+ break;
+ }

--Mark Tinguely.