lkml.org 
[lkml]   [2015]   [Oct]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 3/5] mm: clear PG_dirty to mark page freeable
On Mon, 19 Oct 2015, Minchan Kim wrote:

> Basically, MADV_FREE relies on dirty bit in page table entry
> to decide whether VM allows to discard the page or not.
> IOW, if page table entry includes marked dirty bit, VM shouldn't
> discard the page.
>
> However, as a example, if swap-in by read fault happens,
> page table entry doesn't have dirty bit so MADV_FREE could discard
> the page wrongly.
>
> For avoiding the problem, MADV_FREE did more checks with PageDirty
> and PageSwapCache. It worked out because swapped-in page lives on
> swap cache and since it is evicted from the swap cache, the page has
> PG_dirty flag. So both page flags check effectively prevent
> wrong discarding by MADV_FREE.
>
> However, a problem in above logic is that swapped-in page has
> PG_dirty still after they are removed from swap cache so VM cannot
> consider the page as freeable any more even if madvise_free is
> called in future.
>
> Look at below example for detail.
>
> ptr = malloc();
> memset(ptr);
> ..
> ..
> .. heavy memory pressure so all of pages are swapped out
> ..
> ..
> var = *ptr; -> a page swapped-in and could be removed from
> swapcache. Then, page table doesn't mark
> dirty bit and page descriptor includes PG_dirty
> ..
> ..
> madvise_free(ptr); -> It doesn't clear PG_dirty of the page.
> ..
> ..
> ..
> .. heavy memory pressure again.
> .. In this time, VM cannot discard the page because the page
> .. has *PG_dirty*
>
> To solve the problem, this patch clears PG_dirty if only the page
> is owned exclusively by current process when madvise is called
> because PG_dirty represents ptes's dirtiness in several processes
> so we could clear it only if we own it exclusively.
>
> Cc: Hugh Dickins <hughd@google.com>
> Signed-off-by: Minchan Kim <minchan@kernel.org>

Acked-by: Hugh Dickins <hughd@google.com>

(and patches 1/5 and 2/5 too if you like)

> ---
> mm/madvise.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/mm/madvise.c b/mm/madvise.c
> index fdfb14a78c60..5db546431285 100644
> --- a/mm/madvise.c
> +++ b/mm/madvise.c
> @@ -312,11 +312,19 @@ static int madvise_free_pte_range(pmd_t *pmd, unsigned long addr,
> if (!page)
> continue;
>
> - if (PageSwapCache(page)) {
> + if (PageSwapCache(page) || PageDirty(page)) {
> if (!trylock_page(page))
> continue;
> + /*
> + * If page is shared with others, we couldn't clear
> + * PG_dirty of the page.
> + */
> + if (page_count(page) != 1 + !!PageSwapCache(page)) {
> + unlock_page(page);
> + continue;
> + }
>
> - if (!try_to_free_swap(page)) {
> + if (PageSwapCache(page) && !try_to_free_swap(page)) {
> unlock_page(page);
> continue;
> }
> --
> 1.9.1
>
>


\
 
 \ /
  Last update: 2015-10-27 02:41    [W:0.121 / U:1.304 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site