linux-kernel - Re: [PATCH] mm: vmscan: recompute page status when putting back

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 10 Jan 2012 12:44:42 +0900
From:	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>
To:	Hillf Danton <dhillf@...il.com>
Cc:	linux-mm@...ck.org, David Rientjes <rientjes@...gle.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] mm: vmscan: recompute page status when putting back

On Fri, 6 Jan 2012 22:07:29 +0800
Hillf Danton <dhillf@...il.com> 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@...fujitsu.com>
> Cc: David Rientjes <rientjes@...gle.com>
> Cc: Andrew Morton <akpm@...ux-foundation.org>
> Signed-off-by: Hillf Danton <dhillf@...il.com>
> ---
> 
> --- 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 from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/