xfs
[Top] [All Lists]

[PATCH v3] xfs: probe data buffer from page cache for unwritten extents

To: xfs@xxxxxxxxxxx
Subject: [PATCH v3] xfs: probe data buffer from page cache for unwritten extents
From: Jeff Liu <jeff.liu@xxxxxxxxxx>
Date: Tue, 03 Jul 2012 15:15:56 +0800
Organization: Oracle
Reply-to: jeff.liu@xxxxxxxxxx
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.28) Gecko/20120313 Thunderbird/3.1.20
Hello,

This is v3 of the patch.

We can trigger BUG() in xfs_seek_data() if met two unwritten without data or 
hole extents at last version.
So making the extents map reading in loop could solve it.

Sorry, Am not yet try the repeated holes scenario according to Dave's comments 
as lack of X64 test env, still
waiting for it ready.  But this patch is already too long delayed, I have 
worked it out one weeks ago.
So I'd like to post it because of it could handle repeated hole/unwritten 
extents well in a loop, and I also improved
xfstests:286 with those cases for the verification, will post it soon.

v2->v3:
Tested by Mark, hit BUG() for continuous unwritten extents without data wrote.
* xfs_seek_data(), remove BUG() and having extents map search in loop.

v1->v2:
suggested by Mark.
* xfs_has_unwritten_buffer(), use the input offset instead of bmap->br_startoff 
to
 calculate page index for data buffer probing.

Thanks,
-Jeff


Signed-off-by: Jie Liu <jeff.liu@xxxxxxxxxx>

---
 fs/xfs/xfs_file.c |  197 +++++++++++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 178 insertions(+), 19 deletions(-)

diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 9f7ec15..a8821c1 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -36,6 +36,7 @@
 
 #include <linux/dcache.h>
 #include <linux/falloc.h>
+#include <linux/pagevec.h>
 
 static const struct vm_operations_struct xfs_file_vm_ops;
 
@@ -966,6 +967,108 @@ xfs_vm_page_mkwrite(
        return block_page_mkwrite(vma, vmf, xfs_get_blocks);
 }
 
+/*
+ * Probe the data buffer offset in page cache for unwritten extents.
+ * Iterate each page to find out if a buffer head state has BH_Unwritten
+ * or BH_Uptodate set.
+ */
+STATIC bool
+xfs_has_unwritten_buffer(
+       struct inode            *inode,
+       struct xfs_bmbt_irec    *map,
+       loff_t                  *offset)
+{
+       struct xfs_inode        *ip = XFS_I(inode);
+       struct xfs_mount        *mp = ip->i_mount;
+       struct pagevec          pvec;
+       pgoff_t                 index;
+       pgoff_t                 end;
+       bool                    found = false;
+
+       pagevec_init(&pvec, 0);
+
+       index = XFS_FSB_TO_B(mp, XFS_B_TO_FSBT(mp, *offset))
+                            >> PAGE_CACHE_SHIFT;
+       end = XFS_FSB_TO_B(mp, map->br_startoff + map->br_blockcount)
+                          >> PAGE_CACHE_SHIFT;
+       do {
+               unsigned int    i;
+               unsigned        nr_pages;
+               int             want = min_t(pgoff_t, end - index,
+                                            (pgoff_t)PAGEVEC_SIZE - 1) + 1;
+               nr_pages = pagevec_lookup(&pvec, inode->i_mapping, index,
+                                         want);
+               if (nr_pages == 0)
+                       break;
+
+               for (i = 0; i < nr_pages; i++) {
+                       struct page             *page = pvec.pages[i];
+                       struct buffer_head      *bh;
+                       struct buffer_head      *head;
+                       xfs_fileoff_t           last;
+
+                       /*
+                        * There is no need to check the following pages
+                        * if the current page offset is out of range.
+                        */
+                       if (page->index > end)
+                               goto out;
+
+                       if (!trylock_page(page))
+                               goto out;
+
+                       if (!page_has_buffers(page)) {
+                               unlock_page(page);
+                               continue;
+                       }
+
+                       last = XFS_B_TO_FSBT(mp,
+                                            page->index << PAGE_CACHE_SHIFT);
+                       bh = head = page_buffers(page);
+                       do {
+                               /*
+                                * An extent in XFS_EXT_UNWRITTEN has disk
+                                * blocks already mapped to it, but no data
+                                * has been committed to them yet.  If it has
+                                * dirty data in the page cache it can be
+                                * identified by having BH_Unwritten set in
+                                * each buffers.  Also, the buffer head state
+                                * might be in BH_Uptodate too if the buffer
+                                * writeback procedure was fired, we need to
+                                * examine it as well.
+                                */
+                               if ((buffer_unwritten(bh) ||
+                                    buffer_uptodate(bh)) &&
+                                    *offset <= XFS_FSB_TO_B(mp, last)) {
+                                       found = true;
+                                       *offset = XFS_FSB_TO_B(mp, last);
+                                       unlock_page(page);
+                                       goto out;
+                               }
+                               last++;
+                       } while ((bh = bh->b_this_page) != head);
+                       unlock_page(page);
+               }
+
+               /*
+                * If the number of probed pages less than our desired,
+                * there should no more pages mapped, search done.
+                */
+               if (nr_pages < want)
+                       break;
+
+               index = pvec.pages[i - 1]->index + 1;
+               pagevec_release(&pvec);
+       } while (index < end);
+
+out:
+       pagevec_release(&pvec);
+       if (!found)
+               *offset = 0;
+
+       return found;
+}
+
 STATIC loff_t
 xfs_seek_data(
        struct file             *file,
@@ -975,8 +1078,6 @@ xfs_seek_data(
        struct inode            *inode = file->f_mapping->host;
        struct xfs_inode        *ip = XFS_I(inode);
        struct xfs_mount        *mp = ip->i_mount;
-       struct xfs_bmbt_irec    map[2];
-       int                     nmap = 2;
        loff_t                  uninitialized_var(offset);
        xfs_fsize_t             isize;
        xfs_fileoff_t           fsbno;
@@ -992,36 +1093,94 @@ xfs_seek_data(
                goto out_unlock;
        }
 
-       fsbno = XFS_B_TO_FSBT(mp, start);
-
        /*
         * Try to read extents from the first block indicated
         * by fsbno to the end block of the file.
         */
+       fsbno = XFS_B_TO_FSBT(mp, start);
        end = XFS_B_TO_FSB(mp, isize);
+       for (;;) {
+               struct xfs_bmbt_irec    map[2];
+               int                     nmap = 2;
 
-       error = xfs_bmapi_read(ip, fsbno, end - fsbno, map, &nmap,
-                              XFS_BMAPI_ENTIRE);
-       if (error)
-               goto out_unlock;
+               error = xfs_bmapi_read(ip, fsbno, end - fsbno, map, &nmap,
+                                      XFS_BMAPI_ENTIRE);
+               if (error)
+                       goto out_unlock;
 
-       /*
-        * Treat unwritten extent as data extent since it might
-        * contains dirty data in page cache.
-        */
-       if (map[0].br_startblock != HOLESTARTBLOCK) {
-               offset = max_t(loff_t, start,
-                              XFS_FSB_TO_B(mp, map[0].br_startoff));
-       } else {
-               if (nmap == 1) {
+               /* No extents at given offset, must be beyond EOF */
+               if (nmap == 0) {
                        error = ENXIO;
                        goto out_unlock;
                }
 
-               offset = max_t(loff_t, start,
-                              XFS_FSB_TO_B(mp, map[1].br_startoff));
+               offset = start;
+               if (map[0].br_state == XFS_EXT_NORM &&
+                   !isnullstartblock(map[0].br_startblock))
+                       break;
+               else {
+                       /*
+                        * Landed in an unwritten extent, try to search
+                        * data buffer from page cache firstly.  Treat
+                        * it as a hole if nothing was found, and skip
+                        * to check the next extent.
+                        */
+                       if (map[0].br_startblock == DELAYSTARTBLOCK ||
+                           map[0].br_state == XFS_EXT_UNWRITTEN) {
+                               /* Probing page cache start from offset */
+                               if (xfs_has_unwritten_buffer(inode, &map[0],
+                                                            &offset))
+                                       goto out;
+                       }
+
+                       /*
+                        * Found a hole in map[0] and nothing in map[1].
+                        * Probably means that we are reading after EOF.
+                        */
+                       if (nmap == 1) {
+                               error = ENXIO;
+                               goto out_unlock;
+                       }
+
+                       /*
+                        * We have two mappings, and need to check map[1] to
+                        * see if there is data or not.
+                        */
+                       offset = max_t(loff_t, start,
+                                      XFS_FSB_TO_B(mp, map[1].br_startoff));
+
+                       if (map[1].br_state == XFS_EXT_NORM &&
+                           !isnullstartblock(map[1].br_startblock))
+                               goto out;
+                       else {
+                               /*
+                                * So map[1] is an unwritten extent as well,
+                                * try to search for data buffer in page cache.
+                                * We might find nothing if it is an allocated
+                                * and resvered extent.
+                                */
+                               if (map[1].br_startblock == DELAYSTARTBLOCK ||
+                                   map[1].br_state == XFS_EXT_UNWRITTEN) {
+                                       if (xfs_has_unwritten_buffer(inode,
+                                                       &map[1], &offset))
+                                               goto out;
+                               }
+                       }
+               }
+
+               /*
+                * Nothing was found, proceed to the next round of search if
+                * reading offset not beyond EOF.
+                */
+               fsbno = map[1].br_startoff + map[1].br_blockcount;
+               start = XFS_FSB_TO_B(mp, fsbno);
+               if (start > isize) {
+                       error = ENXIO;
+                       goto out_unlock;
+               }
        }
 
+out:
        if (offset != file->f_pos)
                file->f_pos = offset;
 
-- 
1.7.9

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