From: Michal Hocko <mhocko@suse.com> To: Yu Zhao <yuzhao@google.com> Cc: Andrew Morton <akpm@linux-foundation.org>, Alex Shi <alex.shi@linux.alibaba.com>, Steven Rostedt <rostedt@goodmis.org>, Ingo Molnar <mingo@redhat.com>, Johannes Weiner <hannes@cmpxchg.org>, Vladimir Davydov <vdavydov.dev@gmail.com>, Roman Gushchin <guro@fb.com>, Shakeel Butt <shakeelb@google.com>, Chris Down <chris@chrisdown.name>, Yafang Shao <laoar.shao@gmail.com>, Vlastimil Babka <vbabka@suse.cz>, Huang Ying <ying.huang@intel.com>, Pankaj Gupta <pankaj.gupta.linux@gmail.com>, Matthew Wilcox <willy@infradead.org>, Konstantin Khlebnikov <koct9i@gmail.com>, Minchan Kim <minchan@kernel.org>, Jaewon Kim <jaewon31.kim@samsung.com>, cgroups@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 01/13] mm: use add_page_to_lru_list() Date: Fri, 18 Sep 2020 09:32:40 +0200 [thread overview] Message-ID: <20200918073240.GD28827@dhcp22.suse.cz> (raw) In-Reply-To: <20200918030051.650890-2-yuzhao@google.com> On Thu 17-09-20 21:00:39, Yu Zhao wrote: > This patch replaces the only open-coded lru list addition with > add_page_to_lru_list(). > > Before this patch, we have: > update_lru_size() > list_move() > > After this patch, we have: > list_del() > add_page_to_lru_list() > update_lru_size() > list_add() > > The only side effect is that page->lru is temporarily poisoned > after a page is deleted from its old list, which shouldn't be a > problem. "because the lru lock is held" right? Please always be explicit about your reasoning. "It shouldn't be a problem" without further justification is just pointless for anybody reading the code later on. > Signed-off-by: Yu Zhao <yuzhao@google.com> > --- > mm/vmscan.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/mm/vmscan.c b/mm/vmscan.c > index 9727dd8e2581..503fc5e1fe32 100644 > --- a/mm/vmscan.c > +++ b/mm/vmscan.c > @@ -1850,8 +1850,8 @@ static unsigned noinline_for_stack move_pages_to_lru(struct lruvec *lruvec, > while (!list_empty(list)) { > page = lru_to_page(list); > VM_BUG_ON_PAGE(PageLRU(page), page); > + list_del(&page->lru); > if (unlikely(!page_evictable(page))) { > - list_del(&page->lru); > spin_unlock_irq(&pgdat->lru_lock); > putback_lru_page(page); > spin_lock_irq(&pgdat->lru_lock); > @@ -1862,9 +1862,7 @@ static unsigned noinline_for_stack move_pages_to_lru(struct lruvec *lruvec, > 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, lru); > > if (put_page_testzero(page)) { > __ClearPageLRU(page); > @@ -1878,6 +1876,7 @@ static unsigned noinline_for_stack move_pages_to_lru(struct lruvec *lruvec, > } else > list_add(&page->lru, &pages_to_free); > } else { > + nr_pages = thp_nr_pages(page); > nr_moved += nr_pages; > if (PageActive(page)) > workingset_age_nonresident(lruvec, nr_pages); > -- > 2.28.0.681.g6f77f65b4e-goog -- Michal Hocko SUSE Labs
WARNING: multiple messages have this Message-ID (diff)
From: Michal Hocko <mhocko-IBi9RG/b67k@public.gmane.org> To: Yu Zhao <yuzhao-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> Cc: Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>, Alex Shi <alex.shi-KPsoFbNs7GizrGE5bRqYAgC/G2K4zDHf@public.gmane.org>, Steven Rostedt <rostedt-nx8X9YLhiw1AfugRpC6u6w@public.gmane.org>, Ingo Molnar <mingo-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>, Johannes Weiner <hannes-druUgvl0LCNAfugRpC6u6w@public.gmane.org>, Vladimir Davydov <vdavydov.dev-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>, Roman Gushchin <guro-b10kYP2dOMg@public.gmane.org>, Shakeel Butt <shakeelb-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>, Chris Down <chris-6Bi1550iOqEnzZ6mRAm98g@public.gmane.org>, Yafang Shao <laoar.shao-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>, Vlastimil Babka <vbabka-AlSwsSmVLrQ@public.gmane.org>, Huang Ying <ying.huang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>, Pankaj Gupta <pankaj.gupta.linux-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>, Matthew Wilcox <willy-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>, Konstantin Khlebnikov <koct9i-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>, Minchan Kim <minchan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>, Jaewon Kim <jaewon31.kim-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>, cgroups-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org Subject: Re: [PATCH 01/13] mm: use add_page_to_lru_list() Date: Fri, 18 Sep 2020 09:32:40 +0200 [thread overview] Message-ID: <20200918073240.GD28827@dhcp22.suse.cz> (raw) In-Reply-To: <20200918030051.650890-2-yuzhao-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> On Thu 17-09-20 21:00:39, Yu Zhao wrote: > This patch replaces the only open-coded lru list addition with > add_page_to_lru_list(). > > Before this patch, we have: > update_lru_size() > list_move() > > After this patch, we have: > list_del() > add_page_to_lru_list() > update_lru_size() > list_add() > > The only side effect is that page->lru is temporarily poisoned > after a page is deleted from its old list, which shouldn't be a > problem. "because the lru lock is held" right? Please always be explicit about your reasoning. "It shouldn't be a problem" without further justification is just pointless for anybody reading the code later on. > Signed-off-by: Yu Zhao <yuzhao-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org> > --- > mm/vmscan.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/mm/vmscan.c b/mm/vmscan.c > index 9727dd8e2581..503fc5e1fe32 100644 > --- a/mm/vmscan.c > +++ b/mm/vmscan.c > @@ -1850,8 +1850,8 @@ static unsigned noinline_for_stack move_pages_to_lru(struct lruvec *lruvec, > while (!list_empty(list)) { > page = lru_to_page(list); > VM_BUG_ON_PAGE(PageLRU(page), page); > + list_del(&page->lru); > if (unlikely(!page_evictable(page))) { > - list_del(&page->lru); > spin_unlock_irq(&pgdat->lru_lock); > putback_lru_page(page); > spin_lock_irq(&pgdat->lru_lock); > @@ -1862,9 +1862,7 @@ static unsigned noinline_for_stack move_pages_to_lru(struct lruvec *lruvec, > 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, lru); > > if (put_page_testzero(page)) { > __ClearPageLRU(page); > @@ -1878,6 +1876,7 @@ static unsigned noinline_for_stack move_pages_to_lru(struct lruvec *lruvec, > } else > list_add(&page->lru, &pages_to_free); > } else { > + nr_pages = thp_nr_pages(page); > nr_moved += nr_pages; > if (PageActive(page)) > workingset_age_nonresident(lruvec, nr_pages); > -- > 2.28.0.681.g6f77f65b4e-goog -- Michal Hocko SUSE Labs
next prev parent reply other threads:[~2020-09-18 7:32 UTC|newest] Thread overview: 68+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-09-18 3:00 [PATCH 00/13] mm: clean up some lru related pieces Yu Zhao 2020-09-18 3:00 ` Yu Zhao 2020-09-18 3:00 ` [PATCH 01/13] mm: use add_page_to_lru_list() Yu Zhao 2020-09-18 3:00 ` Yu Zhao 2020-09-18 3:00 ` Yu Zhao 2020-09-18 7:32 ` Michal Hocko [this message] 2020-09-18 7:32 ` Michal Hocko 2020-09-18 10:05 ` Yu Zhao 2020-09-18 3:00 ` [PATCH 02/13] mm: use page_off_lru() Yu Zhao 2020-09-18 3:00 ` Yu Zhao 2020-09-18 7:37 ` Michal Hocko 2020-09-18 7:37 ` Michal Hocko 2020-09-18 10:27 ` Yu Zhao 2020-09-18 11:09 ` Michal Hocko 2020-09-18 18:53 ` Yu Zhao 2020-09-18 18:53 ` Yu Zhao 2020-09-21 11:16 ` Michal Hocko 2020-09-21 11:16 ` Michal Hocko 2020-09-18 11:24 ` Michal Hocko 2020-09-18 11:24 ` Michal Hocko 2020-09-18 3:00 ` [PATCH 03/13] mm: move __ClearPageLRU() into page_off_lru() Yu Zhao 2020-09-18 3:00 ` Yu Zhao 2020-09-18 7:38 ` Michal Hocko 2020-09-18 7:38 ` Michal Hocko 2020-09-18 3:00 ` [PATCH 04/13] mm: shuffle lru list addition and deletion functions Yu Zhao 2020-09-18 3:00 ` Yu Zhao 2020-09-18 3:00 ` [PATCH 05/13] mm: don't pass enum lru_list to lru list addition functions Yu Zhao 2020-09-18 3:00 ` Yu Zhao 2020-09-18 3:00 ` [PATCH 06/13] mm: don't pass enum lru_list to trace_mm_lru_insertion() Yu Zhao 2020-09-18 3:00 ` Yu Zhao 2020-09-18 3:00 ` Yu Zhao 2020-09-18 3:00 ` [PATCH 07/13] mm: don't pass enum lru_list to del_page_from_lru_list() Yu Zhao 2020-09-18 3:00 ` Yu Zhao 2020-09-18 3:00 ` Yu Zhao 2020-09-18 3:00 ` [PATCH 08/13] mm: rename page_off_lru() to __clear_page_lru_flags() Yu Zhao 2020-09-18 3:00 ` Yu Zhao 2020-09-18 3:00 ` [PATCH 09/13] mm: inline page_lru_base_type() Yu Zhao 2020-09-18 3:00 ` Yu Zhao 2020-09-18 3:00 ` Yu Zhao 2020-09-18 3:00 ` [PATCH 10/13] mm: VM_BUG_ON lru page flags Yu Zhao 2020-09-18 3:00 ` Yu Zhao 2020-09-18 3:00 ` Yu Zhao 2020-09-18 3:00 ` [PATCH 11/13] mm: inline __update_lru_size() Yu Zhao 2020-09-18 3:00 ` Yu Zhao 2020-09-18 3:00 ` Yu Zhao 2020-09-18 3:00 ` [PATCH 12/13] mm: make lruvec_lru_size() static Yu Zhao 2020-09-18 3:00 ` Yu Zhao 2020-09-18 3:00 ` Yu Zhao 2020-09-18 3:00 ` [PATCH 13/13] mm: enlarge the int parameter of update_lru_size() Yu Zhao 2020-09-18 3:00 ` Yu Zhao 2020-09-18 3:00 ` Yu Zhao 2020-09-18 7:45 ` [PATCH 00/13] mm: clean up some lru related pieces Michal Hocko 2020-09-18 7:45 ` Michal Hocko 2020-09-18 9:36 ` Yu Zhao 2020-09-18 9:36 ` Yu Zhao 2020-09-18 20:46 ` Hugh Dickins 2020-09-18 20:46 ` Hugh Dickins 2020-09-18 20:46 ` Hugh Dickins 2020-09-18 21:01 ` Yu Zhao 2020-09-18 21:01 ` Yu Zhao 2020-09-18 21:19 ` Hugh Dickins 2020-09-18 21:19 ` Hugh Dickins 2020-09-18 21:19 ` Hugh Dickins 2020-09-19 0:31 ` Alex Shi 2020-09-19 0:31 ` Alex Shi 2020-10-13 2:30 ` Hugh Dickins 2020-10-13 2:30 ` Hugh Dickins 2020-10-13 2:30 ` Hugh Dickins
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=20200918073240.GD28827@dhcp22.suse.cz \ --to=mhocko@suse.com \ --cc=akpm@linux-foundation.org \ --cc=alex.shi@linux.alibaba.com \ --cc=cgroups@vger.kernel.org \ --cc=chris@chrisdown.name \ --cc=guro@fb.com \ --cc=hannes@cmpxchg.org \ --cc=jaewon31.kim@samsung.com \ --cc=koct9i@gmail.com \ --cc=laoar.shao@gmail.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=minchan@kernel.org \ --cc=mingo@redhat.com \ --cc=pankaj.gupta.linux@gmail.com \ --cc=rostedt@goodmis.org \ --cc=shakeelb@google.com \ --cc=vbabka@suse.cz \ --cc=vdavydov.dev@gmail.com \ --cc=willy@infradead.org \ --cc=ying.huang@intel.com \ --cc=yuzhao@google.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: linkBe 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.