From: Johannes Weiner <hannes@cmpxchg.org>
To: Minchan Kim <minchan@kernel.org>
Cc: Shaohua Li <shli@fb.com>,
linux-mm@kvack.org, linux-kernel@vger.kernel.org,
Kernel-team@fb.com, mhocko@suse.com, hughd@google.com,
riel@redhat.com, mgorman@techsingularity.net,
akpm@linux-foundation.org
Subject: Re: [PATCH V3 3/7] mm: reclaim MADV_FREE pages
Date: Fri, 17 Feb 2017 11:15:32 -0500 [thread overview]
Message-ID: <20170217161532.GC23735@cmpxchg.org> (raw)
In-Reply-To: <20170217054108.GA3653@bbox>
On Fri, Feb 17, 2017 at 02:41:08PM +0900, Minchan Kim wrote:
> Hi Johannes,
>
> On Thu, Feb 16, 2017 at 01:40:18PM -0500, Johannes Weiner wrote:
> > On Tue, Feb 14, 2017 at 11:36:09AM -0800, Shaohua Li wrote:
> > > @@ -911,7 +911,7 @@ static void page_check_dirty_writeback(struct page *page,
> > > * Anonymous pages are not handled by flushers and must be written
> > > * from reclaim context. Do not stall reclaim based on them
> > > */
> > > - if (!page_is_file_cache(page)) {
> > > + if (!page_is_file_cache(page) || page_is_lazyfree(page)) {
> >
> > Do we need this? MADV_FREE clears the dirty bit off the page; we could
> > just let them go through with the function without any special-casing.
>
> I thought some driver potentially can do GUP with FOLL_TOUCH so that the
> lazyfree page can have PG_dirty with !PG_swapbacked. In this case,
> throttling logic of shrink_page_list can be confused?
Yep, agreed. We should filter these pages here.
> > > @@ -1142,7 +1144,7 @@ static unsigned long shrink_page_list(struct list_head *page_list,
> > > * The page is mapped into the page tables of one or more
> > > * processes. Try to unmap it here.
> > > */
> > > - if (page_mapped(page) && mapping) {
> > > + if (page_mapped(page) && (mapping || lazyfree)) {
> >
> > Do we actually need to filter for mapping || lazyfree? If we fail to
> > allocate swap, we don't reach here. If the page is a truncated file
> > page, ttu returns pretty much instantly with SWAP_AGAIN. We should be
> > able to just check for page_mapped() alone, no?
>
> try_to_unmap_one assumes every anonymous pages reached will have swp_entry
> so it should be changed to check PageSwapCache if we go to the way.
Yep, I think it should check page_mapping(). To me that would make the
most sense, see other email: "Don't unmap a ram page with valid data
when there is no secondary storage mapping to maintain integrity."
--
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: Johannes Weiner <hannes@cmpxchg.org>
To: Minchan Kim <minchan@kernel.org>
Cc: Shaohua Li <shli@fb.com>,
linux-mm@kvack.org, linux-kernel@vger.kernel.org,
Kernel-team@fb.com, mhocko@suse.com, hughd@google.com,
riel@redhat.com, mgorman@techsingularity.net,
akpm@linux-foundation.org
Subject: Re: [PATCH V3 3/7] mm: reclaim MADV_FREE pages
Date: Fri, 17 Feb 2017 11:15:32 -0500 [thread overview]
Message-ID: <20170217161532.GC23735@cmpxchg.org> (raw)
In-Reply-To: <20170217054108.GA3653@bbox>
On Fri, Feb 17, 2017 at 02:41:08PM +0900, Minchan Kim wrote:
> Hi Johannes,
>
> On Thu, Feb 16, 2017 at 01:40:18PM -0500, Johannes Weiner wrote:
> > On Tue, Feb 14, 2017 at 11:36:09AM -0800, Shaohua Li wrote:
> > > @@ -911,7 +911,7 @@ static void page_check_dirty_writeback(struct page *page,
> > > * Anonymous pages are not handled by flushers and must be written
> > > * from reclaim context. Do not stall reclaim based on them
> > > */
> > > - if (!page_is_file_cache(page)) {
> > > + if (!page_is_file_cache(page) || page_is_lazyfree(page)) {
> >
> > Do we need this? MADV_FREE clears the dirty bit off the page; we could
> > just let them go through with the function without any special-casing.
>
> I thought some driver potentially can do GUP with FOLL_TOUCH so that the
> lazyfree page can have PG_dirty with !PG_swapbacked. In this case,
> throttling logic of shrink_page_list can be confused?
Yep, agreed. We should filter these pages here.
> > > @@ -1142,7 +1144,7 @@ static unsigned long shrink_page_list(struct list_head *page_list,
> > > * The page is mapped into the page tables of one or more
> > > * processes. Try to unmap it here.
> > > */
> > > - if (page_mapped(page) && mapping) {
> > > + if (page_mapped(page) && (mapping || lazyfree)) {
> >
> > Do we actually need to filter for mapping || lazyfree? If we fail to
> > allocate swap, we don't reach here. If the page is a truncated file
> > page, ttu returns pretty much instantly with SWAP_AGAIN. We should be
> > able to just check for page_mapped() alone, no?
>
> try_to_unmap_one assumes every anonymous pages reached will have swp_entry
> so it should be changed to check PageSwapCache if we go to the way.
Yep, I think it should check page_mapping(). To me that would make the
most sense, see other email: "Don't unmap a ram page with valid data
when there is no secondary storage mapping to maintain integrity."
next prev parent reply other threads:[~2017-02-17 16:15 UTC|newest]
Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-02-14 19:36 [PATCH V3 0/7] mm: fix some MADV_FREE issues Shaohua Li
2017-02-14 19:36 ` Shaohua Li
2017-02-14 19:36 ` [PATCH V3 1/7] mm: don't assume anonymous pages have SwapBacked flag Shaohua Li
2017-02-14 19:36 ` Shaohua Li
2017-02-16 17:39 ` Johannes Weiner
2017-02-16 17:39 ` Johannes Weiner
2017-02-14 19:36 ` [PATCH V3 2/7] mm: move MADV_FREE pages into LRU_INACTIVE_FILE list Shaohua Li
2017-02-14 19:36 ` Shaohua Li
2017-02-16 17:52 ` Johannes Weiner
2017-02-16 17:52 ` Johannes Weiner
2017-02-17 0:35 ` Shaohua Li
2017-02-17 0:35 ` Shaohua Li
2017-02-17 16:22 ` Johannes Weiner
2017-02-17 16:22 ` Johannes Weiner
2017-02-14 19:36 ` [PATCH V3 3/7] mm: reclaim MADV_FREE pages Shaohua Li
2017-02-14 19:36 ` Shaohua Li
2017-02-16 18:40 ` Johannes Weiner
2017-02-16 18:40 ` Johannes Weiner
2017-02-17 0:27 ` Shaohua Li
2017-02-17 0:27 ` Shaohua Li
2017-02-17 5:45 ` Minchan Kim
2017-02-17 5:45 ` Minchan Kim
2017-02-17 16:11 ` Johannes Weiner
2017-02-17 16:11 ` Johannes Weiner
2017-02-17 16:01 ` Johannes Weiner
2017-02-17 16:01 ` Johannes Weiner
2017-02-17 18:43 ` Shaohua Li
2017-02-17 18:43 ` Shaohua Li
2017-02-17 20:03 ` Johannes Weiner
2017-02-17 20:03 ` Johannes Weiner
2017-02-17 5:41 ` Minchan Kim
2017-02-17 5:41 ` Minchan Kim
2017-02-17 9:27 ` Minchan Kim
2017-02-17 9:27 ` Minchan Kim
2017-02-17 16:15 ` Johannes Weiner [this message]
2017-02-17 16:15 ` Johannes Weiner
2017-02-14 19:36 ` [PATCH V3 4/7] mm: enable MADV_FREE for swapless system Shaohua Li
2017-02-14 19:36 ` Shaohua Li
2017-02-17 16:16 ` Johannes Weiner
2017-02-17 16:16 ` Johannes Weiner
2017-02-14 19:36 ` [PATCH V3 5/7] mm: add vmstat account for MADV_FREE pages Shaohua Li
2017-02-14 19:36 ` Shaohua Li
2017-02-14 19:36 ` [PATCH V3 6/7] proc: show MADV_FREE pages info in smaps Shaohua Li
2017-02-14 19:36 ` Shaohua Li
2017-02-14 19:36 ` [PATCH V3 7/7] mm: add a separate RSS for MADV_FREE pages Shaohua Li
2017-02-14 19:36 ` Shaohua Li
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=20170217161532.GC23735@cmpxchg.org \
--to=hannes@cmpxchg.org \
--cc=Kernel-team@fb.com \
--cc=akpm@linux-foundation.org \
--cc=hughd@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mgorman@techsingularity.net \
--cc=mhocko@suse.com \
--cc=minchan@kernel.org \
--cc=riel@redhat.com \
--cc=shli@fb.com \
/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.