From: Yu Zhao <yuzhao@google.com>
To: Michal Hocko <mhocko@suse.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Alex Shi <alex.shi@linux.alibaba.com>,
linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/2] mm: use add_page_to_lru_list()/page_lru()/page_off_lru()
Date: Fri, 4 Sep 2020 10:54:37 -0600 [thread overview]
Message-ID: <20200904165437.GA77761@google.com> (raw)
In-Reply-To: <20200904105001.GH15277@dhcp22.suse.cz>
On Fri, Sep 04, 2020 at 12:50:01PM +0200, Michal Hocko wrote:
> On Thu 03-09-20 21:24:00, Yu Zhao wrote:
> > On Thu, Sep 03, 2020 at 10:28:32AM +0200, Michal Hocko wrote:
> > > On Mon 31-08-20 11:50:41, Yu Zhao wrote:
> > > [...]
> > > > @@ -1860,16 +1859,11 @@ static unsigned noinline_for_stack move_pages_to_lru(struct lruvec *lruvec,
> > > > lruvec = mem_cgroup_page_lruvec(page, pgdat);
> > > >
> > > > SetPageLRU(page);
> > > > - lru = page_lru(page);
> > > > -
> > > > - nr_pages = thp_nr_pages(page);
> > > > - update_lru_size(lruvec, lru, page_zonenum(page), nr_pages);
> > > > - list_move(&page->lru, &lruvec->lists[lru]);
> > > > + add_page_to_lru_list(page, lruvec, page_lru(page));
> > > >
> > > > if (put_page_testzero(page)) {
> > > > __ClearPageLRU(page);
> > > > - __ClearPageActive(page);
> > >
> > > This should go into its own patch. The rest is a mechanical and clear.
> >
> > Thanks for reviewing.
> >
> > I assume you are worrying about PG_unevictable being set on the page
> > because page_off_lru() checks it first.
>
> No, I was referring to __ClearPageActive. You are right that this is
> cleared in page_off_lru but that is hidden in a release path and e.g.
> compound pages are released via their destructor which for some might
> not involve releasing the page - e.g. hugetlb pages. This should be fine
> because hugetlb pages are not on LRU so as I've said this is fine but it
> belongs to its own patch because it is not a pure mechanical change like
> the rest of the patch.
Please bear with me. This is the change in question:
if (put_page_testzero(page)) {
__ClearPageLRU(page);
- __ClearPageActive(page);
- del_page_from_lru_list(page, lruvec, lru);
+ del_page_from_lru_list(page, lruvec, page_off_lru(page));
if (unlikely(PageCompound(page))) {
If the PageUnevictable() check in page_off_lru() is not a concern,
I'm trying to understand what else is different between them:
Before this path: After this patch:
page_off_lru()
__ClearPageActive() __ClearPageActive()
add_page_to_lru_list() add_page_to_lru_list()
And why is page_off_lru() hidden in a release path?
prev parent reply other threads:[~2020-09-04 16:55 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-31 17:50 [PATCH v2 1/2] mm: use add_page_to_lru_list()/page_lru()/page_off_lru() Yu Zhao
2020-08-31 17:50 ` [PATCH v2 2/2] mm: use self-explanatory macros rather than "2" Yu Zhao
2020-09-03 8:28 ` [PATCH v2 1/2] mm: use add_page_to_lru_list()/page_lru()/page_off_lru() Michal Hocko
2020-09-04 3:24 ` Yu Zhao
2020-09-04 10:50 ` Michal Hocko
2020-09-04 16:54 ` Yu Zhao [this message]
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=20200904165437.GA77761@google.com \
--to=yuzhao@google.com \
--cc=akpm@linux-foundation.org \
--cc=alex.shi@linux.alibaba.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mhocko@suse.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).