[PATCH] 2.3.14: filemap_nopage hangs when offset >= inode->i_size

Steve Dodd (dirk@loth.demon.co.uk)
Fri, 20 Aug 1999 05:35:34 +0100

Ok, you know all the stuff that got ripped out of filemap_nopage for 2.3.14?
It seems we need it :- try_to_read_ahead won't operate beyond of the end of
the file (obviously), but that means when a non-shared mapping does extend
past the end, filemap_nopage just loops, calling try_to_read_ahead forever
to get the page it wants.

The patch below includes the last one, and more or less reverts the 2.3.14
changes (without reintroducing the bug it was meant to fix, I hope). Would
it actually make sense to look at the whole function again? AFAICS the comments
about overlapping allocation and I/O are wrong - surely we have to start the
I/O, *then* allocate the page, if we want to overlap them? Would that be worth
doing?

Does lmbench time this sort of stuff?

(Oh, I fixed a rather misleading comment in memory.c as well):

--- mm/filemap.c.orig Fri Aug 20 05:33:00 1999
+++ mm/filemap.c Fri Aug 20 05:43:59 1999
@@ -1315,8 +1315,10 @@
goto failure;
}

- if (!Page_Uptodate(page))
+ if (!Page_Uptodate(page)) {
+ lock_page(page);
goto page_not_uptodate;
+ }

success:
/*
@@ -1337,8 +1339,13 @@
}

/*
- * No sharing ... copy to the new page.
+ * No sharing ... get a new page if necessary and copy to it.
*/
+ if (!new_page)
+ new_page = page_cache_alloc();
+ if (!new_page)
+ goto no_page;
+
copy_page(new_page, old_page);
flush_page_to_ram(new_page);
page_cache_release(page);
@@ -1346,6 +1353,13 @@

no_cached_page:
/*
+ * read ahead won't read past end of file, so we will have
+ * to add the page ourselves.
+ */
+ if (offset >= inode->i_size)
+ goto add_new_page;
+
+ /*
* Try to read in an entire cluster at once.
*/
reada = offset;
@@ -1362,8 +1376,22 @@
*/
goto retry_find;

+add_new_page:
+ if (!new_page)
+ new_page = page_cache_alloc();
+ if (!new_page)
+ goto no_page;
+
+ /*
+ * Add the new page to the page cache. If someone beat
+ * us to it, go and look for the page again.
+ */
+ page = page_cache_entry(new_page);
+ if (add_to_page_cache_unique(page, inode, offset, hash))
+ goto retry_find;
+ new_page = 0;
+
page_not_uptodate:
- lock_page(page);
if (Page_Uptodate(page)) {
UnlockPage(page);
goto success;
@@ -1375,6 +1403,7 @@
wait_on_page(page);
if (Page_Uptodate(page))
goto success;
+ lock_page(page);
}

/*
@@ -1383,8 +1412,11 @@
* because there really aren't any performance issues here
* and we need to check for errors.
*/
- if (!PageLocked(page))
- PAGE_BUG(page);
+ if (Page_Uptodate(page)) {
+ UnlockPage(page);
+ goto success;
+ }
+
ClearPageError(page);
error = inode->i_op->readpage(file, page);
if (error)
--- mm/memory.c.orig Fri Aug 20 04:51:16 1999
+++ mm/memory.c Fri Aug 20 04:51:29 1999
@@ -1034,8 +1034,7 @@
* As this is called only for pages that do not currently exist, we
* do not need to flush old virtual caches or the TLB.
*
- * This is called with the MM semaphore and the kernel lock held.
- * We need to release the kernel lock as soon as possible..
+ * This is called with the MM semaphore held.
*/
static int do_no_page(struct task_struct * tsk, struct vm_area_struct * vma,
unsigned long address, int write_access, pte_t *page_table)

-- 
When in doubt, use brute force.
                -- Ken Thompson


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/