On 08/03/12 01:10, Jeff Liu wrote:
Introduce helpers to probe data or hole offset from page cache for unwritten extents.
---
fs/xfs/xfs_file.c | 213 +++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 213 insertions(+), 0 deletions(-)
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
index 98642cf..aff0c30 100644
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -36,6 +36,7 @@
+ * keep the offset argument unchanged.
+ */
+STATIC bool
+xfs_lookup_buffer_offset(
I am fine with this helper routine.
+
+/*
+ * This routine is called to find out and return a data or hole offset
+ * from the page cache for unwritten extents according to the desired
+ * type for xfs_seek_data() or xfs_seek_hole().
+ *
+ * The argument offset is used to tell where we start to search from the
+ * page cache. Map is used to figure out the end points of the range to
+ * lookup pages.
+ *
+ * Return true if the desired type of offset was found, and the argument
+ * offset is filled with that address. Otherwise, return false and keep
+ * offset unchanged.
+ */
+STATIC bool
+xfs_find_get_desired_pgoff(
+ struct inode *inode,
+ struct xfs_bmbt_irec *map,
+ unsigned int type,
+ 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;
+ loff_t endoff;
+ loff_t startoff = *offset;
+ loff_t lastoff = startoff;
+ bool found = false;
+
+ pagevec_init(&pvec, 0);
+
+ index = startoff>> PAGE_CACHE_SHIFT;
+ endoff = XFS_FSB_TO_B(mp, map->br_startoff + map->br_blockcount);
+ end = endoff>> PAGE_CACHE_SHIFT;
+ do {
+ int want;
+ unsigned nr_pages;
+ unsigned int i;
+
+ want = min_t(pgoff_t, end - index, (pgoff_t)PAGEVEC_SIZE);
+ nr_pages = pagevec_lookup(&pvec, inode->i_mapping, index,
+ want);
+ /*
+ * No page mapped into given range. If we are searching holes
+ * and if this is the first time we got into the loop, it means
+ * that the given offset is landed in a hole, return it.
+ *
+ * If we have already stepped through some block buffers to find
+ * holes but they all contains data. In this case, the last
+ * offset is already updated and pointed to the end of the last
+ * mapped page, if it does not reach the endpoint to search,
+ * that means there should be a hole between them.
+ */
+ if (nr_pages == 0) {
+ /* Data search found nothing */
+ if (type == DATA_OFF)
+ break;
+
+ ASSERT(type == HOLE_OFF);
+ if (lastoff == startoff || lastoff< endoff) {
+ found = true;
+ *offset = lastoff;
+ }
+ break;
+ }
+
+ /*
+ * At lease we found one page. If this is the first time we
+ * step into the loop, and if the first page index offset is
+ * greater than the given search offset, a hole was found.
+ */
+ if (type == HOLE_OFF&& lastoff == startoff&&
+ lastoff< page_offset(pvec.pages[0])) {
+ found = true;
+ break;
+ }
I played with the code and appreciate the trickiness of the startoff.
I this can be refined a bit more. see below.
+
+ for (i = 0; i< nr_pages; i++) {
+ struct page *page = pvec.pages[i];
+ loff_t b_offset;
+
+ /*
+ * Page index is out of range, searching done.
+ * If the current offset is not reaches the end
+ * of the specified search range, there should
+ * be a hole between them.
+ */
+ if (page->index> end) {
+ if (type == HOLE_OFF&& lastoff< endoff) {
+ *offset = lastoff;
+ found = true;
+ }
+ goto out;
+ }
Before we take the lock, we can check to see if the page starts later
than expected (a hole). The pre-loop start check can be removed and
something like the follow added:
/* Skip over this page if it precedes the start offset */
if (page_offset(page) + PAGE_SIZE < startoff)
continue;
if (type == HOLE_OFF && page_offset(page) >
lastoff) {
*offset = lastoff;
found = true;
goto out;
}
My experiment passes your newest, more extensive tests. I can give you the
complete experiment patch if you want.
+
+ lock_page(page);
+ /*
+ * Page truncated or invalidated(page->mapping == NULL).
+ * We can freely skip it and proceed to check the next
+ * page.
+ */
+ if (unlikely(page->mapping != inode->i_mapping)) {
+ unlock_page(page);
+ continue;
+ }
+
+ if (!page_has_buffers(page)) {
+ unlock_page(page);
+ continue;
+ }
Would this be a hole condition? I suppose the next iteration of the loop
will figure this out.
+
+ found = xfs_lookup_buffer_offset(page,&b_offset, type);
+ if (found) {
+ /*
+ * The found offset may be less than the start
+ * point to search if this is the first time to
+ * come here.
+ */
+ *offset = max_t(loff_t, startoff, b_offset);
+ goto out;
+ }
+
+ /*
+ * We either searching data but nothing was found, or
+ * searching hole but found a data buffer. In either
+ * case, probably the next page contains the desired
+ * things, update the last offset to it so.
+ */
+ lastoff = page_offset(page) + PAGE_SIZE;
+ unlock_page(page);
+ }
+
+ /*
+ * The number of returned pages less than our desired, search
+ * done. In this case, nothing was found for searching data,
+ * but we found a hole behind the last offset.
+ */
+ if (nr_pages< want) {
+ if (type == HOLE_OFF) {
+ *offset = lastoff;
+ found = true;
+ }
+ break;
+ }
+
+ index = pvec.pages[i - 1]->index + 1;
+ pagevec_release(&pvec);
+ } while (index< end);
+
+out:
+ pagevec_release(&pvec);
+ return found;
+}
+
STATIC loff_t
xfs_seek_data(
struct file *file,
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
