Re: Introduce SEEK_DATA/SEEK_HOLE to XFS V5

[Mark Tinguely <tinguely@sgi.com>]

xfs_bmapi_read() returns the br_state == XFS_EXT_NORM for a hole.
There are a couple places that a hole can trigger a data test.
BTW, I could not generate a large enough hole that xfs_bmapi_read()
would return as more than one hole entry, so I will ignore those
situations and just list the couple places that a hole may be match
a data rule:

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;
+			}
+
+			/* No data extent at the given offset */
+			if (nmap == 1) {
+				error = ENXIO;
+				break;
+			}
+
+			if (map[1].br_state == XFS_EXT_NORM ||
			^^^ could be a hole and not data^^^

I think you need to add back the br_startblock test:

+			if ((map[1].br_state == XFS_EXT_NORM &&
+			     map[1].br_startblock != HOLESTARTBLOCK) ||


in xfs_seek_hole():
+		/*
+		 * Landed in a delay allocated extent or a real data extent,
+		 * if the next extent is landed in a hole or in an unwritten
+		 * extent but without data committed in the page cache, return
+		 * its offset. If the next extent has dirty data in page cache,
+		 * but its offset starts past both the start block of the map
+		 * and the seek offset, it still be a hole.
+		 */
+		if (map[0].br_startblock == DELAYSTARTBLOCK ||
+		    map[0].br_state == XFS_EXT_NORM) {
			^^^ could be a hole ^^^

    and this only matters because this test is checked before the next test:
		
+
+		/* Landed in a hole, its fine to return */
+		if (map[0].br_startblock == HOLESTARTBLOCK) {
+			offset = max_t(loff_t, seekoff,
+				       XFS_FSB_TO_B(mp, map[0].br_startoff));
+			break;
+		}



Switching the order of these two tests would return the immediate offset
starting a hole seek at the offset of a hole.


None of these conditions will result in data corruption, only earlier
detection of a hole.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs