[PATCH] AOP_TRUNCATED_PAGE victims in read_pages() belong in the LRU

AOP_TRUNCATED_PAGE victims in read_pages() belong in the LRU Nick Piggin rightly pointed out that the introduction of AOP_TRUNCATED_PAGE to read_pages() was wrong to leave A_T_P victim pages in the page cache but not put them in the LRU. Failing to do so hid them from the VM. A_T_P just means that the aop method unlocked the page rather than performing IO. It would be very rare that the page was truncated between the unlock and testing A_T_P. So we leave the pages in the LRU for likely reuse soon rather than backing them back out of the page cache. We do this by matching the behaviour before the A_T_P introduction which added pages to the LRU regardless of what ->readpage() did. This doesn't include the unrelated cleanup in Nick's initial fix which changed read_pages() to return void to match its only caller's behaviour of ignoring errors. Signed-off-by: Nick Piggin <nickpiggin@yahoo.com.au> Signed-off-by: Zach Brown <zach.brown@oracle.com> Signed-off-by: Andrew Morton <akpm@osdl.org> Signed-off-by: Linus Torvalds <torvalds@osdl.org>

[PATCH] AOP_TRUNCATED_PAGE victims in read_pages() belong in the LRU
AOP_TRUNCATED_PAGE victims in read_pages() belong in the LRU Nick Piggin rightly pointed out that the introduction of AOP_TRUNCATED_PAGE to read_pages() was wrong to leave A_T_P victim pages in the page cache but not put them in the LRU. Failing to do so hid them from the VM. A_T_P just means that the aop method unlocked the page rather than performing IO. It would be very rare that the page was truncated between the unlock and testing A_T_P. So we leave the pages in the LRU for likely reuse soon rather than backing them back out of the page cache. We do this by matching the behaviour before the A_T_P introduction which added pages to the LRU regardless of what ->readpage() did. This doesn't include the unrelated cleanup in Nick's initial fix which changed read_pages() to return void to match its only caller's behaviour of ignoring errors. Signed-off-by: Nick Piggin <nickpiggin@yahoo.com.au> Signed-off-by: Zach Brown <zach.brown@oracle.com> Signed-off-by: Andrew Morton <akpm@osdl.org> Signed-off-by: Linus Torvalds <torvalds@osdl.org>
Zach Brown · Linus Torvalds
1 parent 09a9a45dc6
Showing 1 changed file with 5 additions and 8 deletions Side-by-side Diff
mm/readahead.c
@@ -182,14 +182,11 @@
 		list_del(&page->lru);
 		if (!add_to_page_cache(page, mapping,
 					page->index, GFP_KERNEL)) {
-			ret = mapping->a_ops->readpage(filp, page);
-			if (ret != AOP_TRUNCATED_PAGE) {
-				if (!pagevec_add(&lru_pvec, page))
-					__pagevec_lru_add(&lru_pvec);
-				continue;
-			} /* else fall through to release */
-		}
-		page_cache_release(page);
+			mapping->a_ops->readpage(filp, page);
+			if (!pagevec_add(&lru_pvec, page))
+				__pagevec_lru_add(&lru_pvec);
+		} else
+			page_cache_release(page);
 	}
 	pagevec_lru_add(&lru_pvec);
 	ret = 0;
...	...	@@ -182,14 +182,11 @@
182	182	list_del(&page->lru);
183	183	if (!add_to_page_cache(page, mapping,
184	184	page->index, GFP_KERNEL)) {
185		- ret = mapping->a_ops->readpage(filp, page);
186		- if (ret != AOP_TRUNCATED_PAGE) {
187		- if (!pagevec_add(&lru_pvec, page))
188		- __pagevec_lru_add(&lru_pvec);
189		- continue;
190		- } /* else fall through to release */
191		- }
192		- page_cache_release(page);
	185	+ mapping->a_ops->readpage(filp, page);
	186	+ if (!pagevec_add(&lru_pvec, page))
	187	+ __pagevec_lru_add(&lru_pvec);
	188	+ } else
	189	+ page_cache_release(page);
193	190	}
194	191	pagevec_lru_add(&lru_pvec);
195	192	ret = 0;