xfs
[Top] [All Lists]

Re: Introduce SEEK_DATA/SEEK_HOLE to XFS V5

To: jeff.liu@xxxxxxxxxx
Subject: Re: Introduce SEEK_DATA/SEEK_HOLE to XFS V5
From: Mark Tinguely <tinguely@xxxxxxx>
Date: Wed, 11 Jan 2012 15:12:00 -0600
Cc: Ben Myers <bpm@xxxxxxx>, Christoph Hellwig <hch@xxxxxxxxxxxxx>, Chris Mason <chris.mason@xxxxxxxxxx>, xfs@xxxxxxxxxxx
In-reply-to: <4F0D21E5.7010908@xxxxxxxxxx>
References: <4F06F71A.2010301@xxxxxxxxxx> <20120110171855.GX6390@xxxxxxx> <4F0D21E5.7010908@xxxxxxxxxx>
User-agent: Mozilla/5.0 (X11; U; FreeBSD amd64; en-US; rv:1.9.2.24) Gecko/20111206 Thunderbird/3.1.16
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.

<Prev in Thread] Current Thread [Next in Thread>