All of lore.kernel.org
 help / color / mirror / Atom feed
From: Minchan Kim <minchan@kernel.org>
To: Hugh Dickins <hughd@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	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: Tue, 27 Oct 2015 15:58:45 +0900	[thread overview]
Message-ID: <20151027065845.GC26803@bbox> (raw)
In-Reply-To: <alpine.LSU.2.11.1510261909250.10825@eggly.anvils>

On Mon, Oct 26, 2015 at 07:23:12PM -0700, Hugh Dickins wrote:
> 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>

Looks better than mine.
I will include this in my patchset when I respin.

Thanks!

--
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>

WARNING: multiple messages have this Message-ID (diff)
From: Minchan Kim <minchan@kernel.org>
To: Hugh Dickins <hughd@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	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: Tue, 27 Oct 2015 15:58:45 +0900	[thread overview]
Message-ID: <20151027065845.GC26803@bbox> (raw)
In-Reply-To: <alpine.LSU.2.11.1510261909250.10825@eggly.anvils>

On Mon, Oct 26, 2015 at 07:23:12PM -0700, Hugh Dickins wrote:
> 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>

Looks better than mine.
I will include this in my patchset when I respin.

Thanks!

  reply	other threads:[~2015-10-27  6:58 UTC|newest]

Thread overview: 52+ 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 ` Minchan Kim
2015-10-19  6:31 ` [PATCH 1/5] mm: MADV_FREE trivial clean up Minchan Kim
2015-10-19  6:31   ` 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   ` Minchan Kim
2015-10-19  6:31 ` [PATCH 3/5] mm: clear PG_dirty to mark page freeable Minchan Kim
2015-10-19  6:31   ` Minchan Kim
2015-10-27  1:28   ` Hugh Dickins
2015-10-27  1:28     ` Hugh Dickins
2015-10-27  6:50     ` Minchan Kim
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-19  6:31   ` Minchan Kim
2015-10-27  2:09   ` Hugh Dickins
2015-10-27  2:09     ` Hugh Dickins
2015-10-27  3:44     ` yalin wang
2015-10-27  3:44       ` yalin wang
2015-10-27  7:09       ` Minchan Kim
2015-10-27  7:09         ` Minchan Kim
2015-10-27  7:39         ` yalin wang
2015-10-27  7:39           ` yalin wang
2015-10-27  8:10           ` Minchan Kim
2015-10-27  8:10             ` Minchan Kim
2015-10-27  8:52             ` yalin wang
2015-10-27  8:52               ` yalin wang
2015-10-28  4:03               ` yalin wang
2015-10-28  4:03                 ` yalin wang
2015-10-27  6:54     ` Minchan Kim
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-19  6:31   ` Minchan Kim
2015-10-27  2:23   ` Hugh Dickins
2015-10-27  2:23     ` Hugh Dickins
2015-10-27  6:58     ` Minchan Kim [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-19 10:01   ` Minchan Kim
2015-10-20  1:38   ` Minchan Kim
2015-10-20  1:38     ` Minchan Kim
2015-10-20  7:21   ` Minchan Kim
2015-10-20  7:21     ` Minchan Kim
2015-10-20  7:27     ` Minchan Kim
2015-10-20  7:27       ` Minchan Kim
2015-10-20 21:36     ` Andrew Morton
2015-10-20 21:36       ` Andrew Morton
2015-10-20 22:43       ` Kirill A. Shutemov
2015-10-20 22:43         ` Kirill A. Shutemov
2015-10-21  5:11         ` Minchan Kim
2015-10-21  5:11           ` Minchan Kim
2015-10-21  7:50           ` Kirill A. Shutemov
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=20151027065845.GC26803@bbox \
    --to=minchan@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=hannes@cmpxchg.org \
    --cc=hughd@google.com \
    --cc=kirill@shutemov.name \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@suse.de \
    --cc=mhocko@suse.cz \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.