On Fri, 6 Jan 2012 22:07:29 +0800
Hillf Danton <dhillf@xxxxxxxxx> wrote:
> If unlikely the given page is isolated from lru list again, its status is
> recomputed before putting back to lru list, since the comment says page's
> status can change while we move it among lru.
>
>
> Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx>
> Cc: David Rientjes <rientjes@xxxxxxxxxx>
> Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Hillf Danton <dhillf@xxxxxxxxx>
> ---
>
> --- a/mm/vmscan.c Thu Dec 29 20:20:16 2011
> +++ b/mm/vmscan.c Fri Jan 6 21:31:56 2012
> @@ -633,12 +633,14 @@ int remove_mapping(struct address_space
> void putback_lru_page(struct page *page)
> {
> int lru;
> - int active = !!TestClearPageActive(page);
> - int was_unevictable = PageUnevictable(page);
> + int active;
> + int was_unevictable;
>
> VM_BUG_ON(PageLRU(page));
>
> redo:
> + active = !!TestClearPageActive(page);
> + was_unevictable = PageUnevictable(page);
> ClearPageUnevictable(page);
>
> if (page_evictable(page, NULL)) {
Hm. Do you handle this case ?
==
/*
* page's status can change while we move it among lru. If an evictable
* page is on unevictable list, it never be freed. To avoid that,
* check after we added it to the list, again.
*/
if (lru == LRU_UNEVICTABLE && page_evictable(page, NULL)) {
if (!isolate_lru_page(page)) {
put_page(page);
goto redo;
}
==
Ok, let's start from "was_unevictable"
"was_unevicatable" is used for this
==
if (was_unevictable && lru != LRU_UNEVICTABLE)
count_vm_event(UNEVICTABLE_PGRESCUED);
==
This is for checking that the page turned out to be evictable while we put it
into LRU. Assume the 'redo' case, the page's state chages from UNEVICTABLE to
ACTIVE_ANON (for example)
1. at start of function: Page was Unevictable, was_unevictable=true
2. lru = LRU_UNEVICTABLE
3, add the page to LRU.
4. check page_evictable(),..... it returns 'true'.
5. isoalte the page again and goto redo.
6. lru = LRU_ACTIVE_ANON
7. add the page to LRU.
8. was_unevictable==true, then, count_vm_event(UNEVICTABLE_PGRESCUED);
Your patch overwrites was_unevictable between 5. and 6., then,
corrupts this event counting.
about "active" flag.
PageActive() flag will be set in lru_cache_add_lru() and
there will be no inconsistency between page->flags and LRU.
And, in what case the changes in 'active' will be problematic ?
-Kame
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>