From: Hugh Dickins <hughd@google.com>
To: Minchan Kim <minchan@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
linux-mm@kvack.org, linux-kernel@vger.kernel.org,
Hugh Dickins <hughd@google.com>, Rik van Riel <riel@redhat.com>,
Mel Gorman <mgorman@suse.de>, Michal Hocko <mhocko@suse.cz>,
Johannes Weiner <hannes@cmpxchg.org>,
"Kirill A. Shutemov" <kirill@shutemov.name>,
Vlastimil Babka <vbabka@suse.cz>
Subject: Re: [PATCH 5/5] mm: mark stable page dirty in KSM
Date: Mon, 26 Oct 2015 19:23:12 -0700 (PDT) [thread overview]
Message-ID: <alpine.LSU.2.11.1510261909250.10825@eggly.anvils> (raw)
In-Reply-To: <1445236307-895-6-git-send-email-minchan@kernel.org>
On Mon, 19 Oct 2015, Minchan Kim wrote:
> Stable page could be shared by several processes and last process
> could own the page among them after CoW or zapping for every process
> except last process happens. Then, page table entry of the page
> in last process can have no dirty bit and PG_dirty flag in page->flags.
> In this case, MADV_FREE could discard the page wrongly.
> For preventing it, we mark stable page dirty.
I agree with the change, but found that comment (repeated in the source)
rather hard to follow. And it doesn't really do justice to the changes
you have made.
This is not now a MADV_FREE thing, it's more general than that, even
if MADV_FREE is the only thing that takes advantage of it. I like
very much that you've made page reclaim sane, freeing non-dirty
anonymous pages instead of swapping them out, without having to
think of whether it's for MADV_FREE or not.
Would you mind if we replace your patch by a re-commented version?
[PATCH] mm: mark stable page dirty in KSM
The MADV_FREE patchset changes page reclaim to simply free a clean
anonymous page with no dirty ptes, instead of swapping it out; but
KSM uses clean write-protected ptes to reference the stable ksm page.
So be sure to mark that page dirty, so it's never mistakenly discarded.
Signed-off-by: Minchan Kim <minchan@kernel.org>
Signed-off-by: Hugh Dickins <hughd@google.com>
---
mm/ksm.c | 6 ++++++
1 file changed, 6 insertions(+)
diff -puN mm/ksm.c~mm-mark-stable-page-dirty-in-ksm mm/ksm.c
--- a/mm/ksm.c~mm-mark-stable-page-dirty-in-ksm
+++ a/mm/ksm.c
@@ -1050,6 +1050,12 @@ static int try_to_merge_one_page(struct
*/
set_page_stable_node(page, NULL);
mark_page_accessed(page);
+ /*
+ * Page reclaim just frees a clean page with no dirty
+ * ptes: make sure that the ksm page would be swapped.
+ */
+ if (!PageDirty(page))
+ SetPageDirty(page);
err = 0;
} else if (pages_identical(page, kpage))
err = replace_page(vma, page, kpage, orig_pte);
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2015-10-27 2:23 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-10-19 6:31 [PATCH 0/5] MADV_FREE refactoring and fix KSM page Minchan Kim
2015-10-19 6:31 ` [PATCH 1/5] mm: MADV_FREE trivial clean up Minchan Kim
2015-10-19 6:31 ` [PATCH 2/5] mm: skip huge zero page in MADV_FREE Minchan Kim
2015-10-19 6:31 ` [PATCH 3/5] mm: clear PG_dirty to mark page freeable Minchan Kim
2015-10-27 1:28 ` Hugh Dickins
2015-10-27 6:50 ` Minchan Kim
2015-10-19 6:31 ` [PATCH 4/5] mm: simplify reclaim path for MADV_FREE Minchan Kim
2015-10-27 2:09 ` Hugh Dickins
2015-10-27 3:44 ` yalin wang
2015-10-27 7:09 ` Minchan Kim
2015-10-27 7:39 ` yalin wang
2015-10-27 8:10 ` Minchan Kim
2015-10-27 8:52 ` yalin wang
2015-10-28 4:03 ` yalin wang
2015-10-27 6:54 ` Minchan Kim
2015-10-19 6:31 ` [PATCH 5/5] mm: mark stable page dirty in KSM Minchan Kim
2015-10-27 2:23 ` Hugh Dickins [this message]
2015-10-27 6:58 ` Minchan Kim
2015-10-19 10:01 ` [PATCH 0/5] MADV_FREE refactoring and fix KSM page Minchan Kim
2015-10-20 1:38 ` Minchan Kim
2015-10-20 7:21 ` Minchan Kim
2015-10-20 7:27 ` Minchan Kim
2015-10-20 21:36 ` Andrew Morton
2015-10-20 22:43 ` Kirill A. Shutemov
2015-10-21 5:11 ` Minchan Kim
2015-10-21 7:50 ` Kirill A. Shutemov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=alpine.LSU.2.11.1510261909250.10825@eggly.anvils \
--to=hughd@google.com \
--cc=akpm@linux-foundation.org \
--cc=hannes@cmpxchg.org \
--cc=kirill@shutemov.name \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mgorman@suse.de \
--cc=mhocko@suse.cz \
--cc=minchan@kernel.org \
--cc=riel@redhat.com \
--cc=vbabka@suse.cz \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox