* [PATCH 01/13] mm: use add_page_to_lru_list()
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 03/13] mm: move __ClearPageLRU() into page_off_lru() Yu Zhao
` (4 subsequent siblings)
5 siblings, 0 replies; 51+ messages in thread
From: Yu Zhao @ 2020-09-18 3:00 UTC (permalink / raw)
To: Andrew Morton, Michal Hocko
Cc: Alex Shi, Steven Rostedt, Ingo Molnar, Johannes Weiner,
Vladimir Davydov, Roman Gushchin, Shakeel Butt, Chris Down,
Yafang Shao, Vlastimil Babka, Huang Ying, Pankaj Gupta,
Matthew Wilcox, Konstantin Khlebnikov, Minchan Kim, Jaewon Kim,
cgroups-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Yu Zhao
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.
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
^ permalink raw reply related [flat|nested] 51+ messages in thread* [PATCH 01/13] mm: use add_page_to_lru_list()
@ 2020-09-18 3:00 ` Yu Zhao
0 siblings, 0 replies; 51+ messages in thread
From: Yu Zhao @ 2020-09-18 3:00 UTC (permalink / raw)
To: Andrew Morton, Michal Hocko
Cc: Alex Shi, Steven Rostedt, Ingo Molnar, Johannes Weiner,
Vladimir Davydov, Roman Gushchin, Shakeel Butt, Chris Down,
Yafang Shao, Vlastimil Babka, Huang Ying, Pankaj Gupta,
Matthew Wilcox, Konstantin Khlebnikov, Minchan Kim, Jaewon Kim,
cgroups, linux-mm, linux-kernel, Yu Zhao
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.
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
^ permalink raw reply related [flat|nested] 51+ messages in thread[parent not found: <20200918030051.650890-2-yuzhao-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>]
* Re: [PATCH 01/13] mm: use add_page_to_lru_list()
2020-09-18 3:00 ` Yu Zhao
@ 2020-09-18 7:32 ` Michal Hocko
-1 siblings, 0 replies; 51+ messages in thread
From: Michal Hocko @ 2020-09-18 7:32 UTC (permalink / raw)
To: Yu Zhao
Cc: Andrew Morton, Alex Shi, Steven Rostedt, Ingo Molnar,
Johannes Weiner, Vladimir Davydov, Roman Gushchin, Shakeel Butt,
Chris Down, Yafang Shao, Vlastimil Babka, Huang Ying,
Pankaj Gupta, Matthew Wilcox, Konstantin Khlebnikov, Minchan Kim,
Jaewon Kim, cgroups-u79uwXL29TY76Z2rM5mHXA,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
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
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH 01/13] mm: use add_page_to_lru_list()
@ 2020-09-18 7:32 ` Michal Hocko
0 siblings, 0 replies; 51+ messages in thread
From: Michal Hocko @ 2020-09-18 7:32 UTC (permalink / raw)
To: Yu Zhao
Cc: Andrew Morton, Alex Shi, Steven Rostedt, Ingo Molnar,
Johannes Weiner, Vladimir Davydov, Roman Gushchin, Shakeel Butt,
Chris Down, Yafang Shao, Vlastimil Babka, Huang Ying,
Pankaj Gupta, Matthew Wilcox, Konstantin Khlebnikov, Minchan Kim,
Jaewon Kim, cgroups, linux-mm, linux-kernel
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
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH 01/13] mm: use add_page_to_lru_list()
2020-09-18 7:32 ` Michal Hocko
(?)
@ 2020-09-18 10:05 ` Yu Zhao
-1 siblings, 0 replies; 51+ messages in thread
From: Yu Zhao @ 2020-09-18 10:05 UTC (permalink / raw)
To: Michal Hocko
Cc: Andrew Morton, Alex Shi, Steven Rostedt, Ingo Molnar,
Johannes Weiner, Vladimir Davydov, Roman Gushchin, Shakeel Butt,
Chris Down, Yafang Shao, Vlastimil Babka, Huang Ying,
Pankaj Gupta, Matthew Wilcox, Konstantin Khlebnikov, Minchan Kim,
Jaewon Kim, cgroups, linux-mm, linux-kernel
On Fri, Sep 18, 2020 at 09:32:40AM +0200, Michal Hocko wrote:
> 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.
It's not my reasoning. We are simply assuming different contexts this
discussion is under. In the context I'm assuming, we all know we are
under lru lock, which is rule 101 of lru list operations. But I'd be
happy to remove such assumption and update the commit message.
Any concern with the code change?
>
> > 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
^ permalink raw reply [flat|nested] 51+ messages in thread
* [PATCH 06/13] mm: don't pass enum lru_list to trace_mm_lru_insertion()
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 03/13] mm: move __ClearPageLRU() into page_off_lru() Yu Zhao
` (4 subsequent siblings)
5 siblings, 0 replies; 51+ messages in thread
From: Yu Zhao @ 2020-09-18 3:00 UTC (permalink / raw)
To: Andrew Morton, Michal Hocko
Cc: Alex Shi, Steven Rostedt, Ingo Molnar, Johannes Weiner,
Vladimir Davydov, Roman Gushchin, Shakeel Butt, Chris Down,
Yafang Shao, Vlastimil Babka, Huang Ying, Pankaj Gupta,
Matthew Wilcox, Konstantin Khlebnikov, Minchan Kim, Jaewon Kim,
cgroups-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Yu Zhao
The parameter is redundant in the sense that it can be extracted
from the struct page parameter by page_lru() correctly.
This change should have no side effects.
Signed-off-by: Yu Zhao <yuzhao-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
---
include/trace/events/pagemap.h | 11 ++++-------
mm/swap.c | 5 +----
2 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/include/trace/events/pagemap.h b/include/trace/events/pagemap.h
index 8fd1babae761..e1735fe7c76a 100644
--- a/include/trace/events/pagemap.h
+++ b/include/trace/events/pagemap.h
@@ -27,24 +27,21 @@
TRACE_EVENT(mm_lru_insertion,
- TP_PROTO(
- struct page *page,
- int lru
- ),
+ TP_PROTO(struct page *page),
- TP_ARGS(page, lru),
+ TP_ARGS(page),
TP_STRUCT__entry(
__field(struct page *, page )
__field(unsigned long, pfn )
- __field(int, lru )
+ __field(enum lru_list, lru )
__field(unsigned long, flags )
),
TP_fast_assign(
__entry->page = page;
__entry->pfn = page_to_pfn(page);
- __entry->lru = lru;
+ __entry->lru = page_lru(page);
__entry->flags = trace_pagemap_flags(page);
),
diff --git a/mm/swap.c b/mm/swap.c
index 8d0e31d43852..3c89a7276359 100644
--- a/mm/swap.c
+++ b/mm/swap.c
@@ -962,7 +962,6 @@ void lru_add_page_tail(struct page *page, struct page *page_tail,
static void __pagevec_lru_add_fn(struct page *page, struct lruvec *lruvec,
void *arg)
{
- enum lru_list lru;
int was_unevictable = TestClearPageUnevictable(page);
int nr_pages = thp_nr_pages(page);
@@ -998,11 +997,9 @@ static void __pagevec_lru_add_fn(struct page *page, struct lruvec *lruvec,
smp_mb__after_atomic();
if (page_evictable(page)) {
- lru = page_lru(page);
if (was_unevictable)
__count_vm_events(UNEVICTABLE_PGRESCUED, nr_pages);
} else {
- lru = LRU_UNEVICTABLE;
ClearPageActive(page);
SetPageUnevictable(page);
if (!was_unevictable)
@@ -1010,7 +1007,7 @@ static void __pagevec_lru_add_fn(struct page *page, struct lruvec *lruvec,
}
add_page_to_lru_list(page, lruvec);
- trace_mm_lru_insertion(page, lru);
+ trace_mm_lru_insertion(page);
}
/*
--
2.28.0.681.g6f77f65b4e-goog
^ permalink raw reply related [flat|nested] 51+ messages in thread* [PATCH 06/13] mm: don't pass enum lru_list to trace_mm_lru_insertion()
@ 2020-09-18 3:00 ` Yu Zhao
0 siblings, 0 replies; 51+ messages in thread
From: Yu Zhao @ 2020-09-18 3:00 UTC (permalink / raw)
To: Andrew Morton, Michal Hocko
Cc: Alex Shi, Steven Rostedt, Ingo Molnar, Johannes Weiner,
Vladimir Davydov, Roman Gushchin, Shakeel Butt, Chris Down,
Yafang Shao, Vlastimil Babka, Huang Ying, Pankaj Gupta,
Matthew Wilcox, Konstantin Khlebnikov, Minchan Kim, Jaewon Kim,
cgroups, linux-mm, linux-kernel, Yu Zhao
The parameter is redundant in the sense that it can be extracted
from the struct page parameter by page_lru() correctly.
This change should have no side effects.
Signed-off-by: Yu Zhao <yuzhao@google.com>
---
include/trace/events/pagemap.h | 11 ++++-------
mm/swap.c | 5 +----
2 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/include/trace/events/pagemap.h b/include/trace/events/pagemap.h
index 8fd1babae761..e1735fe7c76a 100644
--- a/include/trace/events/pagemap.h
+++ b/include/trace/events/pagemap.h
@@ -27,24 +27,21 @@
TRACE_EVENT(mm_lru_insertion,
- TP_PROTO(
- struct page *page,
- int lru
- ),
+ TP_PROTO(struct page *page),
- TP_ARGS(page, lru),
+ TP_ARGS(page),
TP_STRUCT__entry(
__field(struct page *, page )
__field(unsigned long, pfn )
- __field(int, lru )
+ __field(enum lru_list, lru )
__field(unsigned long, flags )
),
TP_fast_assign(
__entry->page = page;
__entry->pfn = page_to_pfn(page);
- __entry->lru = lru;
+ __entry->lru = page_lru(page);
__entry->flags = trace_pagemap_flags(page);
),
diff --git a/mm/swap.c b/mm/swap.c
index 8d0e31d43852..3c89a7276359 100644
--- a/mm/swap.c
+++ b/mm/swap.c
@@ -962,7 +962,6 @@ void lru_add_page_tail(struct page *page, struct page *page_tail,
static void __pagevec_lru_add_fn(struct page *page, struct lruvec *lruvec,
void *arg)
{
- enum lru_list lru;
int was_unevictable = TestClearPageUnevictable(page);
int nr_pages = thp_nr_pages(page);
@@ -998,11 +997,9 @@ static void __pagevec_lru_add_fn(struct page *page, struct lruvec *lruvec,
smp_mb__after_atomic();
if (page_evictable(page)) {
- lru = page_lru(page);
if (was_unevictable)
__count_vm_events(UNEVICTABLE_PGRESCUED, nr_pages);
} else {
- lru = LRU_UNEVICTABLE;
ClearPageActive(page);
SetPageUnevictable(page);
if (!was_unevictable)
@@ -1010,7 +1007,7 @@ static void __pagevec_lru_add_fn(struct page *page, struct lruvec *lruvec,
}
add_page_to_lru_list(page, lruvec);
- trace_mm_lru_insertion(page, lru);
+ trace_mm_lru_insertion(page);
}
/*
--
2.28.0.681.g6f77f65b4e-goog
^ permalink raw reply related [flat|nested] 51+ messages in thread
* [PATCH 07/13] mm: don't pass enum lru_list to del_page_from_lru_list()
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 03/13] mm: move __ClearPageLRU() into page_off_lru() Yu Zhao
` (4 subsequent siblings)
5 siblings, 0 replies; 51+ messages in thread
From: Yu Zhao @ 2020-09-18 3:00 UTC (permalink / raw)
To: Andrew Morton, Michal Hocko
Cc: Alex Shi, Steven Rostedt, Ingo Molnar, Johannes Weiner,
Vladimir Davydov, Roman Gushchin, Shakeel Butt, Chris Down,
Yafang Shao, Vlastimil Babka, Huang Ying, Pankaj Gupta,
Matthew Wilcox, Konstantin Khlebnikov, Minchan Kim, Jaewon Kim,
cgroups-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Yu Zhao
The parameter is redundant in the sense that it can be extracted
from the struct page parameter by page_lru().
To do this, we need to make sure PageActive() or PageUnevictable()
is correctly set or cleared before calling the function. In
check_move_unevictable_pages(), we have:
ClearPageUnevictable()
del_page_from_lru_list(lru_list = LRU_UNEVICTABLE)
And we need to reorder them to make page_lru() return
LRU_UNEVICTABLE:
del_page_from_lru_list()
page_lru()
ClearPageUnevictable()
We also need to deal with the deletions on releasing paths that
clear PageLRU() and PageActive()/PageUnevictable():
del_page_from_lru_list(lru_list = page_off_lru())
It's done by another recording like this:
del_page_from_lru_list()
page_lru()
page_off_lru()
In both cases, the recording should have no side effects.
Signed-off-by: Yu Zhao <yuzhao-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
---
include/linux/mm_inline.h | 5 +++--
mm/compaction.c | 2 +-
mm/mlock.c | 2 +-
mm/swap.c | 26 ++++++++++----------------
mm/vmscan.c | 8 ++++----
5 files changed, 19 insertions(+), 24 deletions(-)
diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h
index 199ff51bf2a0..03796021f0fe 100644
--- a/include/linux/mm_inline.h
+++ b/include/linux/mm_inline.h
@@ -125,9 +125,10 @@ static __always_inline void add_page_to_lru_list_tail(struct page *page,
}
static __always_inline void del_page_from_lru_list(struct page *page,
- struct lruvec *lruvec, enum lru_list lru)
+ struct lruvec *lruvec)
{
list_del(&page->lru);
- update_lru_size(lruvec, lru, page_zonenum(page), -thp_nr_pages(page));
+ update_lru_size(lruvec, page_lru(page), page_zonenum(page),
+ -thp_nr_pages(page));
}
#endif
diff --git a/mm/compaction.c b/mm/compaction.c
index 176dcded298e..ec4af21d2867 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -1006,7 +1006,7 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
low_pfn += compound_nr(page) - 1;
/* Successfully isolated */
- del_page_from_lru_list(page, lruvec, page_lru(page));
+ del_page_from_lru_list(page, lruvec);
mod_node_page_state(page_pgdat(page),
NR_ISOLATED_ANON + page_is_file_lru(page),
thp_nr_pages(page));
diff --git a/mm/mlock.c b/mm/mlock.c
index 93ca2bf30b4f..647487912d0a 100644
--- a/mm/mlock.c
+++ b/mm/mlock.c
@@ -114,7 +114,7 @@ static bool __munlock_isolate_lru_page(struct page *page, bool getpage)
if (getpage)
get_page(page);
ClearPageLRU(page);
- del_page_from_lru_list(page, lruvec, page_lru(page));
+ del_page_from_lru_list(page, lruvec);
return true;
}
diff --git a/mm/swap.c b/mm/swap.c
index 3c89a7276359..8bbeabc582c1 100644
--- a/mm/swap.c
+++ b/mm/swap.c
@@ -86,7 +86,8 @@ static void __page_cache_release(struct page *page)
spin_lock_irqsave(&pgdat->lru_lock, flags);
lruvec = mem_cgroup_page_lruvec(page, pgdat);
VM_BUG_ON_PAGE(!PageLRU(page), page);
- del_page_from_lru_list(page, lruvec, page_off_lru(page));
+ del_page_from_lru_list(page, lruvec);
+ page_off_lru(page);
spin_unlock_irqrestore(&pgdat->lru_lock, flags);
}
}
@@ -236,7 +237,7 @@ static void pagevec_move_tail_fn(struct page *page, struct lruvec *lruvec,
int *pgmoved = arg;
if (PageLRU(page) && !PageUnevictable(page)) {
- del_page_from_lru_list(page, lruvec, page_lru(page));
+ del_page_from_lru_list(page, lruvec);
ClearPageActive(page);
add_page_to_lru_list_tail(page, lruvec);
(*pgmoved) += thp_nr_pages(page);
@@ -317,10 +318,9 @@ static void __activate_page(struct page *page, struct lruvec *lruvec,
void *arg)
{
if (PageLRU(page) && !PageActive(page) && !PageUnevictable(page)) {
- int lru = page_lru_base_type(page);
int nr_pages = thp_nr_pages(page);
- del_page_from_lru_list(page, lruvec, lru);
+ del_page_from_lru_list(page, lruvec);
SetPageActive(page);
add_page_to_lru_list(page, lruvec);
trace_mm_lru_activate(page);
@@ -527,8 +527,7 @@ void lru_cache_add_inactive_or_unevictable(struct page *page,
static void lru_deactivate_file_fn(struct page *page, struct lruvec *lruvec,
void *arg)
{
- int lru;
- bool active;
+ bool active = PageActive(page);
int nr_pages = thp_nr_pages(page);
if (!PageLRU(page))
@@ -541,10 +540,7 @@ static void lru_deactivate_file_fn(struct page *page, struct lruvec *lruvec,
if (page_mapped(page))
return;
- active = PageActive(page);
- lru = page_lru_base_type(page);
-
- del_page_from_lru_list(page, lruvec, lru + active);
+ del_page_from_lru_list(page, lruvec);
ClearPageActive(page);
ClearPageReferenced(page);
@@ -576,10 +572,9 @@ static void lru_deactivate_fn(struct page *page, struct lruvec *lruvec,
void *arg)
{
if (PageLRU(page) && PageActive(page) && !PageUnevictable(page)) {
- int lru = page_lru_base_type(page);
int nr_pages = thp_nr_pages(page);
- del_page_from_lru_list(page, lruvec, lru + LRU_ACTIVE);
+ del_page_from_lru_list(page, lruvec);
ClearPageActive(page);
ClearPageReferenced(page);
add_page_to_lru_list(page, lruvec);
@@ -595,11 +590,9 @@ static void lru_lazyfree_fn(struct page *page, struct lruvec *lruvec,
{
if (PageLRU(page) && PageAnon(page) && PageSwapBacked(page) &&
!PageSwapCache(page) && !PageUnevictable(page)) {
- bool active = PageActive(page);
int nr_pages = thp_nr_pages(page);
- del_page_from_lru_list(page, lruvec,
- LRU_INACTIVE_ANON + active);
+ del_page_from_lru_list(page, lruvec);
ClearPageActive(page);
ClearPageReferenced(page);
/*
@@ -893,7 +886,8 @@ void release_pages(struct page **pages, int nr)
lruvec = mem_cgroup_page_lruvec(page, locked_pgdat);
VM_BUG_ON_PAGE(!PageLRU(page), page);
- del_page_from_lru_list(page, lruvec, page_off_lru(page));
+ del_page_from_lru_list(page, lruvec);
+ page_off_lru(page);
}
list_add(&page->lru, &pages_to_free);
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 895be9fb96ec..47a4e8ba150f 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1770,10 +1770,9 @@ int isolate_lru_page(struct page *page)
spin_lock_irq(&pgdat->lru_lock);
lruvec = mem_cgroup_page_lruvec(page, pgdat);
if (PageLRU(page)) {
- int lru = page_lru(page);
get_page(page);
ClearPageLRU(page);
- del_page_from_lru_list(page, lruvec, lru);
+ del_page_from_lru_list(page, lruvec);
ret = 0;
}
spin_unlock_irq(&pgdat->lru_lock);
@@ -1862,7 +1861,8 @@ static unsigned noinline_for_stack move_pages_to_lru(struct lruvec *lruvec,
add_page_to_lru_list(page, lruvec);
if (put_page_testzero(page)) {
- del_page_from_lru_list(page, lruvec, page_off_lru(page));
+ del_page_from_lru_list(page, lruvec);
+ page_off_lru(page);
if (unlikely(PageCompound(page))) {
spin_unlock_irq(&pgdat->lru_lock);
@@ -4277,8 +4277,8 @@ void check_move_unevictable_pages(struct pagevec *pvec)
if (page_evictable(page)) {
VM_BUG_ON_PAGE(PageActive(page), page);
+ del_page_from_lru_list(page, lruvec);
ClearPageUnevictable(page);
- del_page_from_lru_list(page, lruvec, LRU_UNEVICTABLE);
add_page_to_lru_list(page, lruvec);
pgrescued++;
}
--
2.28.0.681.g6f77f65b4e-goog
^ permalink raw reply related [flat|nested] 51+ messages in thread* [PATCH 07/13] mm: don't pass enum lru_list to del_page_from_lru_list()
@ 2020-09-18 3:00 ` Yu Zhao
0 siblings, 0 replies; 51+ messages in thread
From: Yu Zhao @ 2020-09-18 3:00 UTC (permalink / raw)
To: Andrew Morton, Michal Hocko
Cc: Alex Shi, Steven Rostedt, Ingo Molnar, Johannes Weiner,
Vladimir Davydov, Roman Gushchin, Shakeel Butt, Chris Down,
Yafang Shao, Vlastimil Babka, Huang Ying, Pankaj Gupta,
Matthew Wilcox, Konstantin Khlebnikov, Minchan Kim, Jaewon Kim,
cgroups, linux-mm, linux-kernel, Yu Zhao
The parameter is redundant in the sense that it can be extracted
from the struct page parameter by page_lru().
To do this, we need to make sure PageActive() or PageUnevictable()
is correctly set or cleared before calling the function. In
check_move_unevictable_pages(), we have:
ClearPageUnevictable()
del_page_from_lru_list(lru_list = LRU_UNEVICTABLE)
And we need to reorder them to make page_lru() return
LRU_UNEVICTABLE:
del_page_from_lru_list()
page_lru()
ClearPageUnevictable()
We also need to deal with the deletions on releasing paths that
clear PageLRU() and PageActive()/PageUnevictable():
del_page_from_lru_list(lru_list = page_off_lru())
It's done by another recording like this:
del_page_from_lru_list()
page_lru()
page_off_lru()
In both cases, the recording should have no side effects.
Signed-off-by: Yu Zhao <yuzhao@google.com>
---
include/linux/mm_inline.h | 5 +++--
mm/compaction.c | 2 +-
mm/mlock.c | 2 +-
mm/swap.c | 26 ++++++++++----------------
mm/vmscan.c | 8 ++++----
5 files changed, 19 insertions(+), 24 deletions(-)
diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h
index 199ff51bf2a0..03796021f0fe 100644
--- a/include/linux/mm_inline.h
+++ b/include/linux/mm_inline.h
@@ -125,9 +125,10 @@ static __always_inline void add_page_to_lru_list_tail(struct page *page,
}
static __always_inline void del_page_from_lru_list(struct page *page,
- struct lruvec *lruvec, enum lru_list lru)
+ struct lruvec *lruvec)
{
list_del(&page->lru);
- update_lru_size(lruvec, lru, page_zonenum(page), -thp_nr_pages(page));
+ update_lru_size(lruvec, page_lru(page), page_zonenum(page),
+ -thp_nr_pages(page));
}
#endif
diff --git a/mm/compaction.c b/mm/compaction.c
index 176dcded298e..ec4af21d2867 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -1006,7 +1006,7 @@ isolate_migratepages_block(struct compact_control *cc, unsigned long low_pfn,
low_pfn += compound_nr(page) - 1;
/* Successfully isolated */
- del_page_from_lru_list(page, lruvec, page_lru(page));
+ del_page_from_lru_list(page, lruvec);
mod_node_page_state(page_pgdat(page),
NR_ISOLATED_ANON + page_is_file_lru(page),
thp_nr_pages(page));
diff --git a/mm/mlock.c b/mm/mlock.c
index 93ca2bf30b4f..647487912d0a 100644
--- a/mm/mlock.c
+++ b/mm/mlock.c
@@ -114,7 +114,7 @@ static bool __munlock_isolate_lru_page(struct page *page, bool getpage)
if (getpage)
get_page(page);
ClearPageLRU(page);
- del_page_from_lru_list(page, lruvec, page_lru(page));
+ del_page_from_lru_list(page, lruvec);
return true;
}
diff --git a/mm/swap.c b/mm/swap.c
index 3c89a7276359..8bbeabc582c1 100644
--- a/mm/swap.c
+++ b/mm/swap.c
@@ -86,7 +86,8 @@ static void __page_cache_release(struct page *page)
spin_lock_irqsave(&pgdat->lru_lock, flags);
lruvec = mem_cgroup_page_lruvec(page, pgdat);
VM_BUG_ON_PAGE(!PageLRU(page), page);
- del_page_from_lru_list(page, lruvec, page_off_lru(page));
+ del_page_from_lru_list(page, lruvec);
+ page_off_lru(page);
spin_unlock_irqrestore(&pgdat->lru_lock, flags);
}
}
@@ -236,7 +237,7 @@ static void pagevec_move_tail_fn(struct page *page, struct lruvec *lruvec,
int *pgmoved = arg;
if (PageLRU(page) && !PageUnevictable(page)) {
- del_page_from_lru_list(page, lruvec, page_lru(page));
+ del_page_from_lru_list(page, lruvec);
ClearPageActive(page);
add_page_to_lru_list_tail(page, lruvec);
(*pgmoved) += thp_nr_pages(page);
@@ -317,10 +318,9 @@ static void __activate_page(struct page *page, struct lruvec *lruvec,
void *arg)
{
if (PageLRU(page) && !PageActive(page) && !PageUnevictable(page)) {
- int lru = page_lru_base_type(page);
int nr_pages = thp_nr_pages(page);
- del_page_from_lru_list(page, lruvec, lru);
+ del_page_from_lru_list(page, lruvec);
SetPageActive(page);
add_page_to_lru_list(page, lruvec);
trace_mm_lru_activate(page);
@@ -527,8 +527,7 @@ void lru_cache_add_inactive_or_unevictable(struct page *page,
static void lru_deactivate_file_fn(struct page *page, struct lruvec *lruvec,
void *arg)
{
- int lru;
- bool active;
+ bool active = PageActive(page);
int nr_pages = thp_nr_pages(page);
if (!PageLRU(page))
@@ -541,10 +540,7 @@ static void lru_deactivate_file_fn(struct page *page, struct lruvec *lruvec,
if (page_mapped(page))
return;
- active = PageActive(page);
- lru = page_lru_base_type(page);
-
- del_page_from_lru_list(page, lruvec, lru + active);
+ del_page_from_lru_list(page, lruvec);
ClearPageActive(page);
ClearPageReferenced(page);
@@ -576,10 +572,9 @@ static void lru_deactivate_fn(struct page *page, struct lruvec *lruvec,
void *arg)
{
if (PageLRU(page) && PageActive(page) && !PageUnevictable(page)) {
- int lru = page_lru_base_type(page);
int nr_pages = thp_nr_pages(page);
- del_page_from_lru_list(page, lruvec, lru + LRU_ACTIVE);
+ del_page_from_lru_list(page, lruvec);
ClearPageActive(page);
ClearPageReferenced(page);
add_page_to_lru_list(page, lruvec);
@@ -595,11 +590,9 @@ static void lru_lazyfree_fn(struct page *page, struct lruvec *lruvec,
{
if (PageLRU(page) && PageAnon(page) && PageSwapBacked(page) &&
!PageSwapCache(page) && !PageUnevictable(page)) {
- bool active = PageActive(page);
int nr_pages = thp_nr_pages(page);
- del_page_from_lru_list(page, lruvec,
- LRU_INACTIVE_ANON + active);
+ del_page_from_lru_list(page, lruvec);
ClearPageActive(page);
ClearPageReferenced(page);
/*
@@ -893,7 +886,8 @@ void release_pages(struct page **pages, int nr)
lruvec = mem_cgroup_page_lruvec(page, locked_pgdat);
VM_BUG_ON_PAGE(!PageLRU(page), page);
- del_page_from_lru_list(page, lruvec, page_off_lru(page));
+ del_page_from_lru_list(page, lruvec);
+ page_off_lru(page);
}
list_add(&page->lru, &pages_to_free);
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 895be9fb96ec..47a4e8ba150f 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1770,10 +1770,9 @@ int isolate_lru_page(struct page *page)
spin_lock_irq(&pgdat->lru_lock);
lruvec = mem_cgroup_page_lruvec(page, pgdat);
if (PageLRU(page)) {
- int lru = page_lru(page);
get_page(page);
ClearPageLRU(page);
- del_page_from_lru_list(page, lruvec, lru);
+ del_page_from_lru_list(page, lruvec);
ret = 0;
}
spin_unlock_irq(&pgdat->lru_lock);
@@ -1862,7 +1861,8 @@ static unsigned noinline_for_stack move_pages_to_lru(struct lruvec *lruvec,
add_page_to_lru_list(page, lruvec);
if (put_page_testzero(page)) {
- del_page_from_lru_list(page, lruvec, page_off_lru(page));
+ del_page_from_lru_list(page, lruvec);
+ page_off_lru(page);
if (unlikely(PageCompound(page))) {
spin_unlock_irq(&pgdat->lru_lock);
@@ -4277,8 +4277,8 @@ void check_move_unevictable_pages(struct pagevec *pvec)
if (page_evictable(page)) {
VM_BUG_ON_PAGE(PageActive(page), page);
+ del_page_from_lru_list(page, lruvec);
ClearPageUnevictable(page);
- del_page_from_lru_list(page, lruvec, LRU_UNEVICTABLE);
add_page_to_lru_list(page, lruvec);
pgrescued++;
}
--
2.28.0.681.g6f77f65b4e-goog
^ permalink raw reply related [flat|nested] 51+ messages in thread
* [PATCH 09/13] mm: inline page_lru_base_type()
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 03/13] mm: move __ClearPageLRU() into page_off_lru() Yu Zhao
` (4 subsequent siblings)
5 siblings, 0 replies; 51+ messages in thread
From: Yu Zhao @ 2020-09-18 3:00 UTC (permalink / raw)
To: Andrew Morton, Michal Hocko
Cc: Alex Shi, Steven Rostedt, Ingo Molnar, Johannes Weiner,
Vladimir Davydov, Roman Gushchin, Shakeel Butt, Chris Down,
Yafang Shao, Vlastimil Babka, Huang Ying, Pankaj Gupta,
Matthew Wilcox, Konstantin Khlebnikov, Minchan Kim, Jaewon Kim,
cgroups-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Yu Zhao
We've removed all other references to this function.
This change should have no side effects.
Signed-off-by: Yu Zhao <yuzhao-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
---
include/linux/mm_inline.h | 27 ++++++---------------------
1 file changed, 6 insertions(+), 21 deletions(-)
diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h
index ef3fd79222e5..07d9a0286635 100644
--- a/include/linux/mm_inline.h
+++ b/include/linux/mm_inline.h
@@ -45,21 +45,6 @@ static __always_inline void update_lru_size(struct lruvec *lruvec,
#endif
}
-/**
- * page_lru_base_type - which LRU list type should a page be on?
- * @page: the page to test
- *
- * Used for LRU list index arithmetic.
- *
- * Returns the base LRU type - file or anon - @page should be on.
- */
-static inline enum lru_list page_lru_base_type(struct page *page)
-{
- if (page_is_file_lru(page))
- return LRU_INACTIVE_FILE;
- return LRU_INACTIVE_ANON;
-}
-
/**
* __clear_page_lru_flags - clear page lru flags before releasing a page
* @page: the page that was on lru and now has a zero reference
@@ -88,12 +73,12 @@ static __always_inline enum lru_list page_lru(struct page *page)
enum lru_list lru;
if (PageUnevictable(page))
- lru = LRU_UNEVICTABLE;
- else {
- lru = page_lru_base_type(page);
- if (PageActive(page))
- lru += LRU_ACTIVE;
- }
+ return LRU_UNEVICTABLE;
+
+ lru = page_is_file_lru(page) ? LRU_INACTIVE_FILE : LRU_INACTIVE_ANON;
+ if (PageActive(page))
+ lru += LRU_ACTIVE;
+
return lru;
}
--
2.28.0.681.g6f77f65b4e-goog
^ permalink raw reply related [flat|nested] 51+ messages in thread* [PATCH 09/13] mm: inline page_lru_base_type()
@ 2020-09-18 3:00 ` Yu Zhao
0 siblings, 0 replies; 51+ messages in thread
From: Yu Zhao @ 2020-09-18 3:00 UTC (permalink / raw)
To: Andrew Morton, Michal Hocko
Cc: Alex Shi, Steven Rostedt, Ingo Molnar, Johannes Weiner,
Vladimir Davydov, Roman Gushchin, Shakeel Butt, Chris Down,
Yafang Shao, Vlastimil Babka, Huang Ying, Pankaj Gupta,
Matthew Wilcox, Konstantin Khlebnikov, Minchan Kim, Jaewon Kim,
cgroups, linux-mm, linux-kernel, Yu Zhao
We've removed all other references to this function.
This change should have no side effects.
Signed-off-by: Yu Zhao <yuzhao@google.com>
---
include/linux/mm_inline.h | 27 ++++++---------------------
1 file changed, 6 insertions(+), 21 deletions(-)
diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h
index ef3fd79222e5..07d9a0286635 100644
--- a/include/linux/mm_inline.h
+++ b/include/linux/mm_inline.h
@@ -45,21 +45,6 @@ static __always_inline void update_lru_size(struct lruvec *lruvec,
#endif
}
-/**
- * page_lru_base_type - which LRU list type should a page be on?
- * @page: the page to test
- *
- * Used for LRU list index arithmetic.
- *
- * Returns the base LRU type - file or anon - @page should be on.
- */
-static inline enum lru_list page_lru_base_type(struct page *page)
-{
- if (page_is_file_lru(page))
- return LRU_INACTIVE_FILE;
- return LRU_INACTIVE_ANON;
-}
-
/**
* __clear_page_lru_flags - clear page lru flags before releasing a page
* @page: the page that was on lru and now has a zero reference
@@ -88,12 +73,12 @@ static __always_inline enum lru_list page_lru(struct page *page)
enum lru_list lru;
if (PageUnevictable(page))
- lru = LRU_UNEVICTABLE;
- else {
- lru = page_lru_base_type(page);
- if (PageActive(page))
- lru += LRU_ACTIVE;
- }
+ return LRU_UNEVICTABLE;
+
+ lru = page_is_file_lru(page) ? LRU_INACTIVE_FILE : LRU_INACTIVE_ANON;
+ if (PageActive(page))
+ lru += LRU_ACTIVE;
+
return lru;
}
--
2.28.0.681.g6f77f65b4e-goog
^ permalink raw reply related [flat|nested] 51+ messages in thread
* [PATCH 10/13] mm: VM_BUG_ON lru page flags
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 03/13] mm: move __ClearPageLRU() into page_off_lru() Yu Zhao
` (4 subsequent siblings)
5 siblings, 0 replies; 51+ messages in thread
From: Yu Zhao @ 2020-09-18 3:00 UTC (permalink / raw)
To: Andrew Morton, Michal Hocko
Cc: Alex Shi, Steven Rostedt, Ingo Molnar, Johannes Weiner,
Vladimir Davydov, Roman Gushchin, Shakeel Butt, Chris Down,
Yafang Shao, Vlastimil Babka, Huang Ying, Pankaj Gupta,
Matthew Wilcox, Konstantin Khlebnikov, Minchan Kim, Jaewon Kim,
cgroups-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Yu Zhao
Move scattered VM_BUG_ONs to two essential places that cover all
lru list additions and deletions.
Signed-off-by: Yu Zhao <yuzhao-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
---
include/linux/mm_inline.h | 4 ++++
mm/swap.c | 2 --
mm/vmscan.c | 1 -
3 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h
index 07d9a0286635..7183c7a03f09 100644
--- a/include/linux/mm_inline.h
+++ b/include/linux/mm_inline.h
@@ -51,6 +51,8 @@ static __always_inline void update_lru_size(struct lruvec *lruvec,
*/
static __always_inline void __clear_page_lru_flags(struct page *page)
{
+ VM_BUG_ON_PAGE(!PageLRU(page), page);
+
__ClearPageLRU(page);
/* this shouldn't happen, so leave the flags to bad_page() */
@@ -72,6 +74,8 @@ static __always_inline enum lru_list page_lru(struct page *page)
{
enum lru_list lru;
+ VM_BUG_ON_PAGE(PageActive(page) && PageUnevictable(page), page);
+
if (PageUnevictable(page))
return LRU_UNEVICTABLE;
diff --git a/mm/swap.c b/mm/swap.c
index b252f3593c57..4daa46907dd5 100644
--- a/mm/swap.c
+++ b/mm/swap.c
@@ -85,7 +85,6 @@ static void __page_cache_release(struct page *page)
spin_lock_irqsave(&pgdat->lru_lock, flags);
lruvec = mem_cgroup_page_lruvec(page, pgdat);
- VM_BUG_ON_PAGE(!PageLRU(page), page);
del_page_from_lru_list(page, lruvec);
__clear_page_lru_flags(page);
spin_unlock_irqrestore(&pgdat->lru_lock, flags);
@@ -885,7 +884,6 @@ void release_pages(struct page **pages, int nr)
}
lruvec = mem_cgroup_page_lruvec(page, locked_pgdat);
- VM_BUG_ON_PAGE(!PageLRU(page), page);
del_page_from_lru_list(page, lruvec);
__clear_page_lru_flags(page);
}
diff --git a/mm/vmscan.c b/mm/vmscan.c
index d93033407200..4688e495c242 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -4276,7 +4276,6 @@ void check_move_unevictable_pages(struct pagevec *pvec)
continue;
if (page_evictable(page)) {
- VM_BUG_ON_PAGE(PageActive(page), page);
del_page_from_lru_list(page, lruvec);
ClearPageUnevictable(page);
add_page_to_lru_list(page, lruvec);
--
2.28.0.681.g6f77f65b4e-goog
^ permalink raw reply related [flat|nested] 51+ messages in thread* [PATCH 10/13] mm: VM_BUG_ON lru page flags
@ 2020-09-18 3:00 ` Yu Zhao
0 siblings, 0 replies; 51+ messages in thread
From: Yu Zhao @ 2020-09-18 3:00 UTC (permalink / raw)
To: Andrew Morton, Michal Hocko
Cc: Alex Shi, Steven Rostedt, Ingo Molnar, Johannes Weiner,
Vladimir Davydov, Roman Gushchin, Shakeel Butt, Chris Down,
Yafang Shao, Vlastimil Babka, Huang Ying, Pankaj Gupta,
Matthew Wilcox, Konstantin Khlebnikov, Minchan Kim, Jaewon Kim,
cgroups, linux-mm, linux-kernel, Yu Zhao
Move scattered VM_BUG_ONs to two essential places that cover all
lru list additions and deletions.
Signed-off-by: Yu Zhao <yuzhao@google.com>
---
include/linux/mm_inline.h | 4 ++++
mm/swap.c | 2 --
mm/vmscan.c | 1 -
3 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h
index 07d9a0286635..7183c7a03f09 100644
--- a/include/linux/mm_inline.h
+++ b/include/linux/mm_inline.h
@@ -51,6 +51,8 @@ static __always_inline void update_lru_size(struct lruvec *lruvec,
*/
static __always_inline void __clear_page_lru_flags(struct page *page)
{
+ VM_BUG_ON_PAGE(!PageLRU(page), page);
+
__ClearPageLRU(page);
/* this shouldn't happen, so leave the flags to bad_page() */
@@ -72,6 +74,8 @@ static __always_inline enum lru_list page_lru(struct page *page)
{
enum lru_list lru;
+ VM_BUG_ON_PAGE(PageActive(page) && PageUnevictable(page), page);
+
if (PageUnevictable(page))
return LRU_UNEVICTABLE;
diff --git a/mm/swap.c b/mm/swap.c
index b252f3593c57..4daa46907dd5 100644
--- a/mm/swap.c
+++ b/mm/swap.c
@@ -85,7 +85,6 @@ static void __page_cache_release(struct page *page)
spin_lock_irqsave(&pgdat->lru_lock, flags);
lruvec = mem_cgroup_page_lruvec(page, pgdat);
- VM_BUG_ON_PAGE(!PageLRU(page), page);
del_page_from_lru_list(page, lruvec);
__clear_page_lru_flags(page);
spin_unlock_irqrestore(&pgdat->lru_lock, flags);
@@ -885,7 +884,6 @@ void release_pages(struct page **pages, int nr)
}
lruvec = mem_cgroup_page_lruvec(page, locked_pgdat);
- VM_BUG_ON_PAGE(!PageLRU(page), page);
del_page_from_lru_list(page, lruvec);
__clear_page_lru_flags(page);
}
diff --git a/mm/vmscan.c b/mm/vmscan.c
index d93033407200..4688e495c242 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -4276,7 +4276,6 @@ void check_move_unevictable_pages(struct pagevec *pvec)
continue;
if (page_evictable(page)) {
- VM_BUG_ON_PAGE(PageActive(page), page);
del_page_from_lru_list(page, lruvec);
ClearPageUnevictable(page);
add_page_to_lru_list(page, lruvec);
--
2.28.0.681.g6f77f65b4e-goog
^ permalink raw reply related [flat|nested] 51+ messages in thread
* [PATCH 11/13] mm: inline __update_lru_size()
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 03/13] mm: move __ClearPageLRU() into page_off_lru() Yu Zhao
` (4 subsequent siblings)
5 siblings, 0 replies; 51+ messages in thread
From: Yu Zhao @ 2020-09-18 3:00 UTC (permalink / raw)
To: Andrew Morton, Michal Hocko
Cc: Alex Shi, Steven Rostedt, Ingo Molnar, Johannes Weiner,
Vladimir Davydov, Roman Gushchin, Shakeel Butt, Chris Down,
Yafang Shao, Vlastimil Babka, Huang Ying, Pankaj Gupta,
Matthew Wilcox, Konstantin Khlebnikov, Minchan Kim, Jaewon Kim,
cgroups-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Yu Zhao
All other references to the function were removed after
commit a892cb6b977f ("mm/vmscan.c: use update_lru_size() in update_lru_sizes()")
This change should have no side effects.
Signed-off-by: Yu Zhao <yuzhao-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
---
include/linux/mm_inline.h | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h
index 7183c7a03f09..355ea1ee32bd 100644
--- a/include/linux/mm_inline.h
+++ b/include/linux/mm_inline.h
@@ -24,7 +24,7 @@ static inline int page_is_file_lru(struct page *page)
return !PageSwapBacked(page);
}
-static __always_inline void __update_lru_size(struct lruvec *lruvec,
+static __always_inline void update_lru_size(struct lruvec *lruvec,
enum lru_list lru, enum zone_type zid,
int nr_pages)
{
@@ -33,13 +33,6 @@ static __always_inline void __update_lru_size(struct lruvec *lruvec,
__mod_lruvec_state(lruvec, NR_LRU_BASE + lru, nr_pages);
__mod_zone_page_state(&pgdat->node_zones[zid],
NR_ZONE_LRU_BASE + lru, nr_pages);
-}
-
-static __always_inline void update_lru_size(struct lruvec *lruvec,
- enum lru_list lru, enum zone_type zid,
- int nr_pages)
-{
- __update_lru_size(lruvec, lru, zid, nr_pages);
#ifdef CONFIG_MEMCG
mem_cgroup_update_lru_size(lruvec, lru, zid, nr_pages);
#endif
--
2.28.0.681.g6f77f65b4e-goog
^ permalink raw reply related [flat|nested] 51+ messages in thread* [PATCH 11/13] mm: inline __update_lru_size()
@ 2020-09-18 3:00 ` Yu Zhao
0 siblings, 0 replies; 51+ messages in thread
From: Yu Zhao @ 2020-09-18 3:00 UTC (permalink / raw)
To: Andrew Morton, Michal Hocko
Cc: Alex Shi, Steven Rostedt, Ingo Molnar, Johannes Weiner,
Vladimir Davydov, Roman Gushchin, Shakeel Butt, Chris Down,
Yafang Shao, Vlastimil Babka, Huang Ying, Pankaj Gupta,
Matthew Wilcox, Konstantin Khlebnikov, Minchan Kim, Jaewon Kim,
cgroups, linux-mm, linux-kernel, Yu Zhao
All other references to the function were removed after
commit a892cb6b977f ("mm/vmscan.c: use update_lru_size() in update_lru_sizes()")
This change should have no side effects.
Signed-off-by: Yu Zhao <yuzhao@google.com>
---
include/linux/mm_inline.h | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h
index 7183c7a03f09..355ea1ee32bd 100644
--- a/include/linux/mm_inline.h
+++ b/include/linux/mm_inline.h
@@ -24,7 +24,7 @@ static inline int page_is_file_lru(struct page *page)
return !PageSwapBacked(page);
}
-static __always_inline void __update_lru_size(struct lruvec *lruvec,
+static __always_inline void update_lru_size(struct lruvec *lruvec,
enum lru_list lru, enum zone_type zid,
int nr_pages)
{
@@ -33,13 +33,6 @@ static __always_inline void __update_lru_size(struct lruvec *lruvec,
__mod_lruvec_state(lruvec, NR_LRU_BASE + lru, nr_pages);
__mod_zone_page_state(&pgdat->node_zones[zid],
NR_ZONE_LRU_BASE + lru, nr_pages);
-}
-
-static __always_inline void update_lru_size(struct lruvec *lruvec,
- enum lru_list lru, enum zone_type zid,
- int nr_pages)
-{
- __update_lru_size(lruvec, lru, zid, nr_pages);
#ifdef CONFIG_MEMCG
mem_cgroup_update_lru_size(lruvec, lru, zid, nr_pages);
#endif
--
2.28.0.681.g6f77f65b4e-goog
^ permalink raw reply related [flat|nested] 51+ messages in thread
* [PATCH 12/13] mm: make lruvec_lru_size() static
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 03/13] mm: move __ClearPageLRU() into page_off_lru() Yu Zhao
` (4 subsequent siblings)
5 siblings, 0 replies; 51+ messages in thread
From: Yu Zhao @ 2020-09-18 3:00 UTC (permalink / raw)
To: Andrew Morton, Michal Hocko
Cc: Alex Shi, Steven Rostedt, Ingo Molnar, Johannes Weiner,
Vladimir Davydov, Roman Gushchin, Shakeel Butt, Chris Down,
Yafang Shao, Vlastimil Babka, Huang Ying, Pankaj Gupta,
Matthew Wilcox, Konstantin Khlebnikov, Minchan Kim, Jaewon Kim,
cgroups-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Yu Zhao
All other references to the function were removed after
commit b910718a948a ("mm: vmscan: detect file thrashing at the reclaim root")
This change should have no side effects.
Signed-off-by: Yu Zhao <yuzhao-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
---
include/linux/mmzone.h | 2 --
mm/vmscan.c | 3 ++-
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 8379432f4f2f..c2b1f1d363cc 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -842,8 +842,6 @@ static inline struct pglist_data *lruvec_pgdat(struct lruvec *lruvec)
#endif
}
-extern unsigned long lruvec_lru_size(struct lruvec *lruvec, enum lru_list lru, int zone_idx);
-
#ifdef CONFIG_HAVE_MEMORYLESS_NODES
int local_memory_node(int node_id);
#else
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 4688e495c242..367843296c21 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -312,7 +312,8 @@ unsigned long zone_reclaimable_pages(struct zone *zone)
* @lru: lru to use
* @zone_idx: zones to consider (use MAX_NR_ZONES for the whole LRU list)
*/
-unsigned long lruvec_lru_size(struct lruvec *lruvec, enum lru_list lru, int zone_idx)
+static unsigned long lruvec_lru_size(struct lruvec *lruvec, enum lru_list lru,
+ int zone_idx)
{
unsigned long size = 0;
int zid;
--
2.28.0.681.g6f77f65b4e-goog
^ permalink raw reply related [flat|nested] 51+ messages in thread* [PATCH 12/13] mm: make lruvec_lru_size() static
@ 2020-09-18 3:00 ` Yu Zhao
0 siblings, 0 replies; 51+ messages in thread
From: Yu Zhao @ 2020-09-18 3:00 UTC (permalink / raw)
To: Andrew Morton, Michal Hocko
Cc: Alex Shi, Steven Rostedt, Ingo Molnar, Johannes Weiner,
Vladimir Davydov, Roman Gushchin, Shakeel Butt, Chris Down,
Yafang Shao, Vlastimil Babka, Huang Ying, Pankaj Gupta,
Matthew Wilcox, Konstantin Khlebnikov, Minchan Kim, Jaewon Kim,
cgroups, linux-mm, linux-kernel, Yu Zhao
All other references to the function were removed after
commit b910718a948a ("mm: vmscan: detect file thrashing at the reclaim root")
This change should have no side effects.
Signed-off-by: Yu Zhao <yuzhao@google.com>
---
include/linux/mmzone.h | 2 --
mm/vmscan.c | 3 ++-
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 8379432f4f2f..c2b1f1d363cc 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -842,8 +842,6 @@ static inline struct pglist_data *lruvec_pgdat(struct lruvec *lruvec)
#endif
}
-extern unsigned long lruvec_lru_size(struct lruvec *lruvec, enum lru_list lru, int zone_idx);
-
#ifdef CONFIG_HAVE_MEMORYLESS_NODES
int local_memory_node(int node_id);
#else
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 4688e495c242..367843296c21 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -312,7 +312,8 @@ unsigned long zone_reclaimable_pages(struct zone *zone)
* @lru: lru to use
* @zone_idx: zones to consider (use MAX_NR_ZONES for the whole LRU list)
*/
-unsigned long lruvec_lru_size(struct lruvec *lruvec, enum lru_list lru, int zone_idx)
+static unsigned long lruvec_lru_size(struct lruvec *lruvec, enum lru_list lru,
+ int zone_idx)
{
unsigned long size = 0;
int zid;
--
2.28.0.681.g6f77f65b4e-goog
^ permalink raw reply related [flat|nested] 51+ messages in thread
* [PATCH 13/13] mm: enlarge the int parameter of update_lru_size()
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 03/13] mm: move __ClearPageLRU() into page_off_lru() Yu Zhao
` (4 subsequent siblings)
5 siblings, 0 replies; 51+ messages in thread
From: Yu Zhao @ 2020-09-18 3:00 UTC (permalink / raw)
To: Andrew Morton, Michal Hocko
Cc: Alex Shi, Steven Rostedt, Ingo Molnar, Johannes Weiner,
Vladimir Davydov, Roman Gushchin, Shakeel Butt, Chris Down,
Yafang Shao, Vlastimil Babka, Huang Ying, Pankaj Gupta,
Matthew Wilcox, Konstantin Khlebnikov, Minchan Kim, Jaewon Kim,
cgroups-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Yu Zhao
In update_lru_sizes(), we call update_lru_size() with a long
argument, whereas the callee only takes an int parameter. Though this
isn't causing any overflow I'm aware of, it's not a good idea to
go through the truncation since the underlying counters are already
in long.
This patch enlarges all relevant parameters on the path to the final
underlying counters:
update_lru_size(int -> long)
if memcg:
__mod_lruvec_state(int -> long)
if smp:
__mod_node_page_state(long)
else:
__mod_node_page_state(int -> long)
__mod_memcg_lruvec_state(int -> long)
__mod_memcg_state(int -> long)
else:
__mod_lruvec_state(int -> long)
if smp:
__mod_node_page_state(long)
else:
__mod_node_page_state(int -> long)
__mod_zone_page_state(long)
if memcg:
mem_cgroup_update_lru_size(int -> long)
Note that __mod_node_page_state() for the smp case and
__mod_zone_page_state() already use long. So this change also fixes
the inconsistency.
Signed-off-by: Yu Zhao <yuzhao-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
---
include/linux/memcontrol.h | 14 +++++++-------
include/linux/mm_inline.h | 2 +-
include/linux/vmstat.h | 2 +-
mm/memcontrol.c | 10 +++++-----
4 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index d0b036123c6a..fcd1829f8382 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -621,7 +621,7 @@ static inline bool mem_cgroup_online(struct mem_cgroup *memcg)
int mem_cgroup_select_victim_node(struct mem_cgroup *memcg);
void mem_cgroup_update_lru_size(struct lruvec *lruvec, enum lru_list lru,
- int zid, int nr_pages);
+ int zid, long nr_pages);
static inline
unsigned long mem_cgroup_get_zone_lru_size(struct lruvec *lruvec,
@@ -707,7 +707,7 @@ static inline unsigned long memcg_page_state_local(struct mem_cgroup *memcg,
return x;
}
-void __mod_memcg_state(struct mem_cgroup *memcg, int idx, int val);
+void __mod_memcg_state(struct mem_cgroup *memcg, int idx, long val);
/* idx can be of type enum memcg_stat_item or node_stat_item */
static inline void mod_memcg_state(struct mem_cgroup *memcg,
@@ -790,9 +790,9 @@ static inline unsigned long lruvec_page_state_local(struct lruvec *lruvec,
}
void __mod_memcg_lruvec_state(struct lruvec *lruvec, enum node_stat_item idx,
- int val);
+ long val);
void __mod_lruvec_state(struct lruvec *lruvec, enum node_stat_item idx,
- int val);
+ long val);
void __mod_lruvec_slab_state(void *p, enum node_stat_item idx, int val);
void mod_memcg_obj_state(void *p, int idx, int val);
@@ -1166,7 +1166,7 @@ static inline unsigned long memcg_page_state_local(struct mem_cgroup *memcg,
static inline void __mod_memcg_state(struct mem_cgroup *memcg,
int idx,
- int nr)
+ long nr)
{
}
@@ -1201,12 +1201,12 @@ static inline unsigned long lruvec_page_state_local(struct lruvec *lruvec,
}
static inline void __mod_memcg_lruvec_state(struct lruvec *lruvec,
- enum node_stat_item idx, int val)
+ enum node_stat_item idx, long val)
{
}
static inline void __mod_lruvec_state(struct lruvec *lruvec,
- enum node_stat_item idx, int val)
+ enum node_stat_item idx, long val)
{
__mod_node_page_state(lruvec_pgdat(lruvec), idx, val);
}
diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h
index 355ea1ee32bd..18e85071b44a 100644
--- a/include/linux/mm_inline.h
+++ b/include/linux/mm_inline.h
@@ -26,7 +26,7 @@ static inline int page_is_file_lru(struct page *page)
static __always_inline void update_lru_size(struct lruvec *lruvec,
enum lru_list lru, enum zone_type zid,
- int nr_pages)
+ long nr_pages)
{
struct pglist_data *pgdat = lruvec_pgdat(lruvec);
diff --git a/include/linux/vmstat.h b/include/linux/vmstat.h
index 91220ace31da..2ae35e8c45f0 100644
--- a/include/linux/vmstat.h
+++ b/include/linux/vmstat.h
@@ -310,7 +310,7 @@ static inline void __mod_zone_page_state(struct zone *zone,
}
static inline void __mod_node_page_state(struct pglist_data *pgdat,
- enum node_stat_item item, int delta)
+ enum node_stat_item item, long delta)
{
node_page_state_add(delta, pgdat, item);
}
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index cfa6cbad21d5..11bc4bb36882 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -774,7 +774,7 @@ mem_cgroup_largest_soft_limit_node(struct mem_cgroup_tree_per_node *mctz)
* @idx: the stat item - can be enum memcg_stat_item or enum node_stat_item
* @val: delta to add to the counter, can be negative
*/
-void __mod_memcg_state(struct mem_cgroup *memcg, int idx, int val)
+void __mod_memcg_state(struct mem_cgroup *memcg, int idx, long val)
{
long x, threshold = MEMCG_CHARGE_BATCH;
@@ -812,7 +812,7 @@ parent_nodeinfo(struct mem_cgroup_per_node *pn, int nid)
}
void __mod_memcg_lruvec_state(struct lruvec *lruvec, enum node_stat_item idx,
- int val)
+ long val)
{
struct mem_cgroup_per_node *pn;
struct mem_cgroup *memcg;
@@ -853,7 +853,7 @@ void __mod_memcg_lruvec_state(struct lruvec *lruvec, enum node_stat_item idx,
* change of state at this level: per-node, per-cgroup, per-lruvec.
*/
void __mod_lruvec_state(struct lruvec *lruvec, enum node_stat_item idx,
- int val)
+ long val)
{
/* Update node */
__mod_node_page_state(lruvec_pgdat(lruvec), idx, val);
@@ -1354,7 +1354,7 @@ struct lruvec *mem_cgroup_page_lruvec(struct page *page, struct pglist_data *pgd
* so as to allow it to check that lru_size 0 is consistent with list_empty).
*/
void mem_cgroup_update_lru_size(struct lruvec *lruvec, enum lru_list lru,
- int zid, int nr_pages)
+ int zid, long nr_pages)
{
struct mem_cgroup_per_node *mz;
unsigned long *lru_size;
@@ -1371,7 +1371,7 @@ void mem_cgroup_update_lru_size(struct lruvec *lruvec, enum lru_list lru,
size = *lru_size;
if (WARN_ONCE(size < 0,
- "%s(%p, %d, %d): lru_size %ld\n",
+ "%s(%p, %d, %ld): lru_size %ld\n",
__func__, lruvec, lru, nr_pages, size)) {
VM_BUG_ON(1);
*lru_size = 0;
--
2.28.0.681.g6f77f65b4e-goog
^ permalink raw reply related [flat|nested] 51+ messages in thread* [PATCH 13/13] mm: enlarge the int parameter of update_lru_size()
@ 2020-09-18 3:00 ` Yu Zhao
0 siblings, 0 replies; 51+ messages in thread
From: Yu Zhao @ 2020-09-18 3:00 UTC (permalink / raw)
To: Andrew Morton, Michal Hocko
Cc: Alex Shi, Steven Rostedt, Ingo Molnar, Johannes Weiner,
Vladimir Davydov, Roman Gushchin, Shakeel Butt, Chris Down,
Yafang Shao, Vlastimil Babka, Huang Ying, Pankaj Gupta,
Matthew Wilcox, Konstantin Khlebnikov, Minchan Kim, Jaewon Kim,
cgroups, linux-mm, linux-kernel, Yu Zhao
In update_lru_sizes(), we call update_lru_size() with a long
argument, whereas the callee only takes an int parameter. Though this
isn't causing any overflow I'm aware of, it's not a good idea to
go through the truncation since the underlying counters are already
in long.
This patch enlarges all relevant parameters on the path to the final
underlying counters:
update_lru_size(int -> long)
if memcg:
__mod_lruvec_state(int -> long)
if smp:
__mod_node_page_state(long)
else:
__mod_node_page_state(int -> long)
__mod_memcg_lruvec_state(int -> long)
__mod_memcg_state(int -> long)
else:
__mod_lruvec_state(int -> long)
if smp:
__mod_node_page_state(long)
else:
__mod_node_page_state(int -> long)
__mod_zone_page_state(long)
if memcg:
mem_cgroup_update_lru_size(int -> long)
Note that __mod_node_page_state() for the smp case and
__mod_zone_page_state() already use long. So this change also fixes
the inconsistency.
Signed-off-by: Yu Zhao <yuzhao@google.com>
---
include/linux/memcontrol.h | 14 +++++++-------
include/linux/mm_inline.h | 2 +-
include/linux/vmstat.h | 2 +-
mm/memcontrol.c | 10 +++++-----
4 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index d0b036123c6a..fcd1829f8382 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -621,7 +621,7 @@ static inline bool mem_cgroup_online(struct mem_cgroup *memcg)
int mem_cgroup_select_victim_node(struct mem_cgroup *memcg);
void mem_cgroup_update_lru_size(struct lruvec *lruvec, enum lru_list lru,
- int zid, int nr_pages);
+ int zid, long nr_pages);
static inline
unsigned long mem_cgroup_get_zone_lru_size(struct lruvec *lruvec,
@@ -707,7 +707,7 @@ static inline unsigned long memcg_page_state_local(struct mem_cgroup *memcg,
return x;
}
-void __mod_memcg_state(struct mem_cgroup *memcg, int idx, int val);
+void __mod_memcg_state(struct mem_cgroup *memcg, int idx, long val);
/* idx can be of type enum memcg_stat_item or node_stat_item */
static inline void mod_memcg_state(struct mem_cgroup *memcg,
@@ -790,9 +790,9 @@ static inline unsigned long lruvec_page_state_local(struct lruvec *lruvec,
}
void __mod_memcg_lruvec_state(struct lruvec *lruvec, enum node_stat_item idx,
- int val);
+ long val);
void __mod_lruvec_state(struct lruvec *lruvec, enum node_stat_item idx,
- int val);
+ long val);
void __mod_lruvec_slab_state(void *p, enum node_stat_item idx, int val);
void mod_memcg_obj_state(void *p, int idx, int val);
@@ -1166,7 +1166,7 @@ static inline unsigned long memcg_page_state_local(struct mem_cgroup *memcg,
static inline void __mod_memcg_state(struct mem_cgroup *memcg,
int idx,
- int nr)
+ long nr)
{
}
@@ -1201,12 +1201,12 @@ static inline unsigned long lruvec_page_state_local(struct lruvec *lruvec,
}
static inline void __mod_memcg_lruvec_state(struct lruvec *lruvec,
- enum node_stat_item idx, int val)
+ enum node_stat_item idx, long val)
{
}
static inline void __mod_lruvec_state(struct lruvec *lruvec,
- enum node_stat_item idx, int val)
+ enum node_stat_item idx, long val)
{
__mod_node_page_state(lruvec_pgdat(lruvec), idx, val);
}
diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h
index 355ea1ee32bd..18e85071b44a 100644
--- a/include/linux/mm_inline.h
+++ b/include/linux/mm_inline.h
@@ -26,7 +26,7 @@ static inline int page_is_file_lru(struct page *page)
static __always_inline void update_lru_size(struct lruvec *lruvec,
enum lru_list lru, enum zone_type zid,
- int nr_pages)
+ long nr_pages)
{
struct pglist_data *pgdat = lruvec_pgdat(lruvec);
diff --git a/include/linux/vmstat.h b/include/linux/vmstat.h
index 91220ace31da..2ae35e8c45f0 100644
--- a/include/linux/vmstat.h
+++ b/include/linux/vmstat.h
@@ -310,7 +310,7 @@ static inline void __mod_zone_page_state(struct zone *zone,
}
static inline void __mod_node_page_state(struct pglist_data *pgdat,
- enum node_stat_item item, int delta)
+ enum node_stat_item item, long delta)
{
node_page_state_add(delta, pgdat, item);
}
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index cfa6cbad21d5..11bc4bb36882 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -774,7 +774,7 @@ mem_cgroup_largest_soft_limit_node(struct mem_cgroup_tree_per_node *mctz)
* @idx: the stat item - can be enum memcg_stat_item or enum node_stat_item
* @val: delta to add to the counter, can be negative
*/
-void __mod_memcg_state(struct mem_cgroup *memcg, int idx, int val)
+void __mod_memcg_state(struct mem_cgroup *memcg, int idx, long val)
{
long x, threshold = MEMCG_CHARGE_BATCH;
@@ -812,7 +812,7 @@ parent_nodeinfo(struct mem_cgroup_per_node *pn, int nid)
}
void __mod_memcg_lruvec_state(struct lruvec *lruvec, enum node_stat_item idx,
- int val)
+ long val)
{
struct mem_cgroup_per_node *pn;
struct mem_cgroup *memcg;
@@ -853,7 +853,7 @@ void __mod_memcg_lruvec_state(struct lruvec *lruvec, enum node_stat_item idx,
* change of state at this level: per-node, per-cgroup, per-lruvec.
*/
void __mod_lruvec_state(struct lruvec *lruvec, enum node_stat_item idx,
- int val)
+ long val)
{
/* Update node */
__mod_node_page_state(lruvec_pgdat(lruvec), idx, val);
@@ -1354,7 +1354,7 @@ struct lruvec *mem_cgroup_page_lruvec(struct page *page, struct pglist_data *pgd
* so as to allow it to check that lru_size 0 is consistent with list_empty).
*/
void mem_cgroup_update_lru_size(struct lruvec *lruvec, enum lru_list lru,
- int zid, int nr_pages)
+ int zid, long nr_pages)
{
struct mem_cgroup_per_node *mz;
unsigned long *lru_size;
@@ -1371,7 +1371,7 @@ void mem_cgroup_update_lru_size(struct lruvec *lruvec, enum lru_list lru,
size = *lru_size;
if (WARN_ONCE(size < 0,
- "%s(%p, %d, %d): lru_size %ld\n",
+ "%s(%p, %d, %ld): lru_size %ld\n",
__func__, lruvec, lru, nr_pages, size)) {
VM_BUG_ON(1);
*lru_size = 0;
--
2.28.0.681.g6f77f65b4e-goog
^ permalink raw reply related [flat|nested] 51+ messages in thread
* Re: [PATCH 00/13] mm: clean up some lru related pieces
2020-09-18 3:00 [PATCH 00/13] mm: clean up some lru related pieces Yu Zhao
@ 2020-09-18 7:45 ` Michal Hocko
2020-09-18 3:00 ` [PATCH 03/13] mm: move __ClearPageLRU() into page_off_lru() Yu Zhao
` (4 subsequent siblings)
5 siblings, 0 replies; 51+ messages in thread
From: Michal Hocko @ 2020-09-18 7:45 UTC (permalink / raw)
To: Yu Zhao
Cc: Andrew Morton, Alex Shi, Steven Rostedt, Ingo Molnar,
Johannes Weiner, Vladimir Davydov, Roman Gushchin, Shakeel Butt,
Chris Down, Yafang Shao, Vlastimil Babka, Huang Ying,
Pankaj Gupta, Matthew Wilcox, Konstantin Khlebnikov, Minchan Kim,
Jaewon Kim, cgroups-u79uwXL29TY76Z2rM5mHXA,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
On Thu 17-09-20 21:00:38, Yu Zhao wrote:
> Hi Andrew,
>
> I see you have taken this:
> mm: use add_page_to_lru_list()/page_lru()/page_off_lru()
> Do you mind dropping it?
>
> Michal asked to do a bit of additional work. So I thought I probably
> should create a series to do more cleanups I've been meaning to.
>
> This series contains the change in the patch above and goes a few
> more steps farther. It's intended to improve readability and should
> not have any performance impacts. There are minor behavior changes in
> terms of debugging and error reporting, which I have all highlighted
> in the individual patches. All patches were properly tested on 5.8
> running Chrome OS, with various debug options turned on.
>
> Michal,
>
> Do you mind taking a looking at the entire series?
I have stopped at patch 3 as all patches until then are really missing
any justification. What is the point for all this to be done? The code
is far from trivial and just shifting around sounds like a risk. You are
removing ~50 LOC which is always nice but I am not sure the resulting
code is better maintainble or easier to read and understand. Just
consider __ClearPageLRU moving to page_off_lru patch. What is the
additional value of having the flag moved and burry it into a function
to have even more side effects? I found the way how __ClearPageLRU is
nicely close to removing it from LRU easier to follow. This is likely
subjective and other might think differently but as it is not clear what
is your actual goal here it is hard to judge pros and cons.
--
Michal Hocko
SUSE Labs
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH 00/13] mm: clean up some lru related pieces
@ 2020-09-18 7:45 ` Michal Hocko
0 siblings, 0 replies; 51+ messages in thread
From: Michal Hocko @ 2020-09-18 7:45 UTC (permalink / raw)
To: Yu Zhao
Cc: Andrew Morton, Alex Shi, Steven Rostedt, Ingo Molnar,
Johannes Weiner, Vladimir Davydov, Roman Gushchin, Shakeel Butt,
Chris Down, Yafang Shao, Vlastimil Babka, Huang Ying,
Pankaj Gupta, Matthew Wilcox, Konstantin Khlebnikov, Minchan Kim,
Jaewon Kim, cgroups, linux-mm, linux-kernel
On Thu 17-09-20 21:00:38, Yu Zhao wrote:
> Hi Andrew,
>
> I see you have taken this:
> mm: use add_page_to_lru_list()/page_lru()/page_off_lru()
> Do you mind dropping it?
>
> Michal asked to do a bit of additional work. So I thought I probably
> should create a series to do more cleanups I've been meaning to.
>
> This series contains the change in the patch above and goes a few
> more steps farther. It's intended to improve readability and should
> not have any performance impacts. There are minor behavior changes in
> terms of debugging and error reporting, which I have all highlighted
> in the individual patches. All patches were properly tested on 5.8
> running Chrome OS, with various debug options turned on.
>
> Michal,
>
> Do you mind taking a looking at the entire series?
I have stopped at patch 3 as all patches until then are really missing
any justification. What is the point for all this to be done? The code
is far from trivial and just shifting around sounds like a risk. You are
removing ~50 LOC which is always nice but I am not sure the resulting
code is better maintainble or easier to read and understand. Just
consider __ClearPageLRU moving to page_off_lru patch. What is the
additional value of having the flag moved and burry it into a function
to have even more side effects? I found the way how __ClearPageLRU is
nicely close to removing it from LRU easier to follow. This is likely
subjective and other might think differently but as it is not clear what
is your actual goal here it is hard to judge pros and cons.
--
Michal Hocko
SUSE Labs
^ permalink raw reply [flat|nested] 51+ messages in thread
[parent not found: <20200918074549.GG28827-2MMpYkNvuYDjFM9bn6wA6Q@public.gmane.org>]
* Re: [PATCH 00/13] mm: clean up some lru related pieces
2020-09-18 7:45 ` Michal Hocko
@ 2020-09-18 9:36 ` Yu Zhao
-1 siblings, 0 replies; 51+ messages in thread
From: Yu Zhao @ 2020-09-18 9:36 UTC (permalink / raw)
To: Michal Hocko
Cc: Andrew Morton, Alex Shi, Steven Rostedt, Ingo Molnar,
Johannes Weiner, Vladimir Davydov, Roman Gushchin, Shakeel Butt,
Chris Down, Yafang Shao, Vlastimil Babka, Huang Ying,
Pankaj Gupta, Matthew Wilcox, Konstantin Khlebnikov, Minchan Kim,
Jaewon Kim, cgroups-u79uwXL29TY76Z2rM5mHXA,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
On Fri, Sep 18, 2020 at 09:45:49AM +0200, Michal Hocko wrote:
> On Thu 17-09-20 21:00:38, Yu Zhao wrote:
> > Hi Andrew,
> >
> > I see you have taken this:
> > mm: use add_page_to_lru_list()/page_lru()/page_off_lru()
> > Do you mind dropping it?
> >
> > Michal asked to do a bit of additional work. So I thought I probably
> > should create a series to do more cleanups I've been meaning to.
> >
> > This series contains the change in the patch above and goes a few
> > more steps farther. It's intended to improve readability and should
> > not have any performance impacts. There are minor behavior changes in
> > terms of debugging and error reporting, which I have all highlighted
> > in the individual patches. All patches were properly tested on 5.8
> > running Chrome OS, with various debug options turned on.
> >
> > Michal,
> >
> > Do you mind taking a looking at the entire series?
>
> I have stopped at patch 3 as all patches until then are really missing
> any justification. What is the point for all this to be done? The code
> is far from trivial and just shifting around sounds like a risk. You are
I appreciate your caution, and if you let me know what exactly your
concerns are, we could probably work them out together.
> removing ~50 LOC which is always nice but I am not sure the resulting
> code is better maintainble or easier to read and understand. Just
> consider __ClearPageLRU moving to page_off_lru patch. What is the
> additional value of having the flag moved and burry it into a function
> to have even more side effects? I found the way how __ClearPageLRU is
Mind elaborating the side effects?
> nicely close to removing it from LRU easier to follow. This is likely
> subjective and other might think differently but as it is not clear what
> is your actual goal here it is hard to judge pros and cons.
I like this specific example from patch 3. Here is what it does: we
have three places using the same boilerplate, i.e., page_off_lru() +
__ClearPageLRU(), the patch moves __ClearPageLRU() into page_off_lru(),
which already does __ClearPageActive() and __ClearPageUnevictable().
Later on, we rename page_off_lru() to __clear_page_lru_flags() (patch
8).
Its point seems quite clear to me. Why would *anybody* want to use
two helper functions *repeatedly* when the job can be done with just
one? Nobody is paid by the number of lines they add, right? :) And
for that matter, why would anybody want any boilerplate to be open
coded from the same group of helper functions arranged in various
ways? I don't think the answer is subjective, but I don't expect
everybody to agree with me.
Now back to your general question: what's the point of this series?
Readability -- less error prone and easier to maintain. This series
consolidate open-coded boilerplate like the following in many places.
Take lru_lazyfree_fn() as an example:
- bool active = PageActive(page);
int nr_pages = thp_nr_pages(page);
- del_page_from_lru_list(page, lruvec,
- LRU_INACTIVE_ANON + active);
+ del_page_from_lru_list(page, lruvec);
ClearPageActive(page);
ClearPageReferenced(page);
<snipped>
ClearPageSwapBacked(page);
- add_page_to_lru_list(page, lruvec, LRU_INACTIVE_FILE);
+ add_page_to_lru_list(page, lruvec);
I hope this helps, but if it doesn't, I'd be more than happy to have
more discussions on the details. And not that I don't appreciate your
review, but please be more specific than 'sounds like a risk' or 'have
even more side effects' so I can address your concerns effectively.
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH 00/13] mm: clean up some lru related pieces
@ 2020-09-18 9:36 ` Yu Zhao
0 siblings, 0 replies; 51+ messages in thread
From: Yu Zhao @ 2020-09-18 9:36 UTC (permalink / raw)
To: Michal Hocko
Cc: Andrew Morton, Alex Shi, Steven Rostedt, Ingo Molnar,
Johannes Weiner, Vladimir Davydov, Roman Gushchin, Shakeel Butt,
Chris Down, Yafang Shao, Vlastimil Babka, Huang Ying,
Pankaj Gupta, Matthew Wilcox, Konstantin Khlebnikov, Minchan Kim,
Jaewon Kim, cgroups, linux-mm, linux-kernel
On Fri, Sep 18, 2020 at 09:45:49AM +0200, Michal Hocko wrote:
> On Thu 17-09-20 21:00:38, Yu Zhao wrote:
> > Hi Andrew,
> >
> > I see you have taken this:
> > mm: use add_page_to_lru_list()/page_lru()/page_off_lru()
> > Do you mind dropping it?
> >
> > Michal asked to do a bit of additional work. So I thought I probably
> > should create a series to do more cleanups I've been meaning to.
> >
> > This series contains the change in the patch above and goes a few
> > more steps farther. It's intended to improve readability and should
> > not have any performance impacts. There are minor behavior changes in
> > terms of debugging and error reporting, which I have all highlighted
> > in the individual patches. All patches were properly tested on 5.8
> > running Chrome OS, with various debug options turned on.
> >
> > Michal,
> >
> > Do you mind taking a looking at the entire series?
>
> I have stopped at patch 3 as all patches until then are really missing
> any justification. What is the point for all this to be done? The code
> is far from trivial and just shifting around sounds like a risk. You are
I appreciate your caution, and if you let me know what exactly your
concerns are, we could probably work them out together.
> removing ~50 LOC which is always nice but I am not sure the resulting
> code is better maintainble or easier to read and understand. Just
> consider __ClearPageLRU moving to page_off_lru patch. What is the
> additional value of having the flag moved and burry it into a function
> to have even more side effects? I found the way how __ClearPageLRU is
Mind elaborating the side effects?
> nicely close to removing it from LRU easier to follow. This is likely
> subjective and other might think differently but as it is not clear what
> is your actual goal here it is hard to judge pros and cons.
I like this specific example from patch 3. Here is what it does: we
have three places using the same boilerplate, i.e., page_off_lru() +
__ClearPageLRU(), the patch moves __ClearPageLRU() into page_off_lru(),
which already does __ClearPageActive() and __ClearPageUnevictable().
Later on, we rename page_off_lru() to __clear_page_lru_flags() (patch
8).
Its point seems quite clear to me. Why would *anybody* want to use
two helper functions *repeatedly* when the job can be done with just
one? Nobody is paid by the number of lines they add, right? :) And
for that matter, why would anybody want any boilerplate to be open
coded from the same group of helper functions arranged in various
ways? I don't think the answer is subjective, but I don't expect
everybody to agree with me.
Now back to your general question: what's the point of this series?
Readability -- less error prone and easier to maintain. This series
consolidate open-coded boilerplate like the following in many places.
Take lru_lazyfree_fn() as an example:
- bool active = PageActive(page);
int nr_pages = thp_nr_pages(page);
- del_page_from_lru_list(page, lruvec,
- LRU_INACTIVE_ANON + active);
+ del_page_from_lru_list(page, lruvec);
ClearPageActive(page);
ClearPageReferenced(page);
<snipped>
ClearPageSwapBacked(page);
- add_page_to_lru_list(page, lruvec, LRU_INACTIVE_FILE);
+ add_page_to_lru_list(page, lruvec);
I hope this helps, but if it doesn't, I'd be more than happy to have
more discussions on the details. And not that I don't appreciate your
review, but please be more specific than 'sounds like a risk' or 'have
even more side effects' so I can address your concerns effectively.
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH 00/13] mm: clean up some lru related pieces
2020-09-18 3:00 [PATCH 00/13] mm: clean up some lru related pieces Yu Zhao
@ 2020-09-18 20:46 ` Hugh Dickins
2020-09-18 3:00 ` [PATCH 03/13] mm: move __ClearPageLRU() into page_off_lru() Yu Zhao
` (4 subsequent siblings)
5 siblings, 0 replies; 51+ messages in thread
From: Hugh Dickins @ 2020-09-18 20:46 UTC (permalink / raw)
To: Yu Zhao
Cc: Andrew Morton, Michal Hocko, Alex Shi, Steven Rostedt,
Ingo Molnar, Johannes Weiner, Vladimir Davydov, Roman Gushchin,
Shakeel Butt, Chris Down, Yafang Shao, Vlastimil Babka,
Huang Ying, Pankaj Gupta, Matthew Wilcox, Konstantin Khlebnikov,
Minchan Kim, Jaewon Kim, cgroups-u79uwXL29TY76Z2rM5mHXA,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
On Thu, 17 Sep 2020, Yu Zhao wrote:
> Hi Andrew,
>
> I see you have taken this:
> mm: use add_page_to_lru_list()/page_lru()/page_off_lru()
> Do you mind dropping it?
>
> Michal asked to do a bit of additional work. So I thought I probably
> should create a series to do more cleanups I've been meaning to.
>
> This series contains the change in the patch above and goes a few
> more steps farther. It's intended to improve readability and should
> not have any performance impacts. There are minor behavior changes in
> terms of debugging and error reporting, which I have all highlighted
> in the individual patches. All patches were properly tested on 5.8
> running Chrome OS, with various debug options turned on.
>
> Michal,
>
> Do you mind taking a looking at the entire series?
>
> Thank you.
>
> Yu Zhao (13):
> mm: use add_page_to_lru_list()
> mm: use page_off_lru()
> mm: move __ClearPageLRU() into page_off_lru()
> mm: shuffle lru list addition and deletion functions
> mm: don't pass enum lru_list to lru list addition functions
> mm: don't pass enum lru_list to trace_mm_lru_insertion()
> mm: don't pass enum lru_list to del_page_from_lru_list()
> mm: rename page_off_lru() to __clear_page_lru_flags()
> mm: inline page_lru_base_type()
> mm: VM_BUG_ON lru page flags
> mm: inline __update_lru_size()
> mm: make lruvec_lru_size() static
> mm: enlarge the int parameter of update_lru_size()
>
> include/linux/memcontrol.h | 14 ++--
> include/linux/mm_inline.h | 115 ++++++++++++++-------------------
> include/linux/mmzone.h | 2 -
> include/linux/vmstat.h | 2 +-
> include/trace/events/pagemap.h | 11 ++--
> mm/compaction.c | 2 +-
> mm/memcontrol.c | 10 +--
> mm/mlock.c | 2 +-
> mm/swap.c | 53 ++++++---------
> mm/vmscan.c | 28 +++-----
> 10 files changed, 95 insertions(+), 144 deletions(-)
>
> --
> 2.28.0.681.g6f77f65b4e-goog
Sorry, Yu, I may be out-of-line in sending this: but as you know,
Alex Shi has a long per-memcg lru_lock series playing in much the
same area (particularly conflicting in mm/swap.c and mm/vmscan.c):
a patchset that makes useful changes, that I'm very keen to help
into mmotm a.s.a.p (but not before I've completed diligence).
We've put a lot of effort into its testing, I'm currently reviewing
it patch by patch (my general silence indicating that I'm busy on that,
but slow as ever): so I'm a bit discouraged to have its stability
potentially undermined by conflicting cleanups at this stage.
If there's general agreement that your cleanups are safe and welcome
(Michal's initial reaction sheds some doubt on that), great: I hope
that Andrew can fast-track them into mmotm, then Alex rebase on top
of them, and I then re-test and re-review.
But if that quick agreement is not forthcoming, may I ask you please
to hold back, and resend based on top of Alex's next posting?
Thanks,
Hugh
^ permalink raw reply [flat|nested] 51+ messages in thread* Re: [PATCH 00/13] mm: clean up some lru related pieces
@ 2020-09-18 20:46 ` Hugh Dickins
0 siblings, 0 replies; 51+ messages in thread
From: Hugh Dickins @ 2020-09-18 20:46 UTC (permalink / raw)
To: Yu Zhao
Cc: Andrew Morton, Michal Hocko, Alex Shi, Steven Rostedt,
Ingo Molnar, Johannes Weiner, Vladimir Davydov, Roman Gushchin,
Shakeel Butt, Chris Down, Yafang Shao, Vlastimil Babka,
Huang Ying, Pankaj Gupta, Matthew Wilcox, Konstantin Khlebnikov,
Minchan Kim, Jaewon Kim, cgroups, linux-mm, linux-kernel
On Thu, 17 Sep 2020, Yu Zhao wrote:
> Hi Andrew,
>
> I see you have taken this:
> mm: use add_page_to_lru_list()/page_lru()/page_off_lru()
> Do you mind dropping it?
>
> Michal asked to do a bit of additional work. So I thought I probably
> should create a series to do more cleanups I've been meaning to.
>
> This series contains the change in the patch above and goes a few
> more steps farther. It's intended to improve readability and should
> not have any performance impacts. There are minor behavior changes in
> terms of debugging and error reporting, which I have all highlighted
> in the individual patches. All patches were properly tested on 5.8
> running Chrome OS, with various debug options turned on.
>
> Michal,
>
> Do you mind taking a looking at the entire series?
>
> Thank you.
>
> Yu Zhao (13):
> mm: use add_page_to_lru_list()
> mm: use page_off_lru()
> mm: move __ClearPageLRU() into page_off_lru()
> mm: shuffle lru list addition and deletion functions
> mm: don't pass enum lru_list to lru list addition functions
> mm: don't pass enum lru_list to trace_mm_lru_insertion()
> mm: don't pass enum lru_list to del_page_from_lru_list()
> mm: rename page_off_lru() to __clear_page_lru_flags()
> mm: inline page_lru_base_type()
> mm: VM_BUG_ON lru page flags
> mm: inline __update_lru_size()
> mm: make lruvec_lru_size() static
> mm: enlarge the int parameter of update_lru_size()
>
> include/linux/memcontrol.h | 14 ++--
> include/linux/mm_inline.h | 115 ++++++++++++++-------------------
> include/linux/mmzone.h | 2 -
> include/linux/vmstat.h | 2 +-
> include/trace/events/pagemap.h | 11 ++--
> mm/compaction.c | 2 +-
> mm/memcontrol.c | 10 +--
> mm/mlock.c | 2 +-
> mm/swap.c | 53 ++++++---------
> mm/vmscan.c | 28 +++-----
> 10 files changed, 95 insertions(+), 144 deletions(-)
>
> --
> 2.28.0.681.g6f77f65b4e-goog
Sorry, Yu, I may be out-of-line in sending this: but as you know,
Alex Shi has a long per-memcg lru_lock series playing in much the
same area (particularly conflicting in mm/swap.c and mm/vmscan.c):
a patchset that makes useful changes, that I'm very keen to help
into mmotm a.s.a.p (but not before I've completed diligence).
We've put a lot of effort into its testing, I'm currently reviewing
it patch by patch (my general silence indicating that I'm busy on that,
but slow as ever): so I'm a bit discouraged to have its stability
potentially undermined by conflicting cleanups at this stage.
If there's general agreement that your cleanups are safe and welcome
(Michal's initial reaction sheds some doubt on that), great: I hope
that Andrew can fast-track them into mmotm, then Alex rebase on top
of them, and I then re-test and re-review.
But if that quick agreement is not forthcoming, may I ask you please
to hold back, and resend based on top of Alex's next posting?
Thanks,
Hugh
^ permalink raw reply [flat|nested] 51+ messages in thread
[parent not found: <alpine.LSU.2.11.2009181317350.11298-fupSdm12i1nKWymIFiNcPA@public.gmane.org>]
* Re: [PATCH 00/13] mm: clean up some lru related pieces
2020-09-18 20:46 ` Hugh Dickins
@ 2020-09-18 21:01 ` Yu Zhao
-1 siblings, 0 replies; 51+ messages in thread
From: Yu Zhao @ 2020-09-18 21:01 UTC (permalink / raw)
To: Hugh Dickins
Cc: Andrew Morton, Michal Hocko, Alex Shi, Steven Rostedt,
Ingo Molnar, Johannes Weiner, Vladimir Davydov, Roman Gushchin,
Shakeel Butt, Chris Down, Yafang Shao, Vlastimil Babka,
Huang Ying, Pankaj Gupta, Matthew Wilcox, Konstantin Khlebnikov,
Minchan Kim, Jaewon Kim, cgroups-u79uwXL29TY76Z2rM5mHXA,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
On Fri, Sep 18, 2020 at 01:46:59PM -0700, Hugh Dickins wrote:
> On Thu, 17 Sep 2020, Yu Zhao wrote:
>
> > Hi Andrew,
> >
> > I see you have taken this:
> > mm: use add_page_to_lru_list()/page_lru()/page_off_lru()
> > Do you mind dropping it?
> >
> > Michal asked to do a bit of additional work. So I thought I probably
> > should create a series to do more cleanups I've been meaning to.
> >
> > This series contains the change in the patch above and goes a few
> > more steps farther. It's intended to improve readability and should
> > not have any performance impacts. There are minor behavior changes in
> > terms of debugging and error reporting, which I have all highlighted
> > in the individual patches. All patches were properly tested on 5.8
> > running Chrome OS, with various debug options turned on.
> >
> > Michal,
> >
> > Do you mind taking a looking at the entire series?
> >
> > Thank you.
> >
> > Yu Zhao (13):
> > mm: use add_page_to_lru_list()
> > mm: use page_off_lru()
> > mm: move __ClearPageLRU() into page_off_lru()
> > mm: shuffle lru list addition and deletion functions
> > mm: don't pass enum lru_list to lru list addition functions
> > mm: don't pass enum lru_list to trace_mm_lru_insertion()
> > mm: don't pass enum lru_list to del_page_from_lru_list()
> > mm: rename page_off_lru() to __clear_page_lru_flags()
> > mm: inline page_lru_base_type()
> > mm: VM_BUG_ON lru page flags
> > mm: inline __update_lru_size()
> > mm: make lruvec_lru_size() static
> > mm: enlarge the int parameter of update_lru_size()
> >
> > include/linux/memcontrol.h | 14 ++--
> > include/linux/mm_inline.h | 115 ++++++++++++++-------------------
> > include/linux/mmzone.h | 2 -
> > include/linux/vmstat.h | 2 +-
> > include/trace/events/pagemap.h | 11 ++--
> > mm/compaction.c | 2 +-
> > mm/memcontrol.c | 10 +--
> > mm/mlock.c | 2 +-
> > mm/swap.c | 53 ++++++---------
> > mm/vmscan.c | 28 +++-----
> > 10 files changed, 95 insertions(+), 144 deletions(-)
> >
> > --
> > 2.28.0.681.g6f77f65b4e-goog
>
> Sorry, Yu, I may be out-of-line in sending this: but as you know,
> Alex Shi has a long per-memcg lru_lock series playing in much the
> same area (particularly conflicting in mm/swap.c and mm/vmscan.c):
> a patchset that makes useful changes, that I'm very keen to help
> into mmotm a.s.a.p (but not before I've completed diligence).
>
> We've put a lot of effort into its testing, I'm currently reviewing
> it patch by patch (my general silence indicating that I'm busy on that,
> but slow as ever): so I'm a bit discouraged to have its stability
> potentially undermined by conflicting cleanups at this stage.
>
> If there's general agreement that your cleanups are safe and welcome
> (Michal's initial reaction sheds some doubt on that), great: I hope
> that Andrew can fast-track them into mmotm, then Alex rebase on top
> of them, and I then re-test and re-review.
>
> But if that quick agreement is not forthcoming, may I ask you please
> to hold back, and resend based on top of Alex's next posting?
The per-memcg lru lock series seems a high priority, and I have
absolutely no problem accommodate your request.
In return, may I ask you or Alex to review this series after you
have finished with per-memcg lru lock (to make sure that I resolve
all the conflicts correctly at least)?
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH 00/13] mm: clean up some lru related pieces
@ 2020-09-18 21:01 ` Yu Zhao
0 siblings, 0 replies; 51+ messages in thread
From: Yu Zhao @ 2020-09-18 21:01 UTC (permalink / raw)
To: Hugh Dickins
Cc: Andrew Morton, Michal Hocko, Alex Shi, Steven Rostedt,
Ingo Molnar, Johannes Weiner, Vladimir Davydov, Roman Gushchin,
Shakeel Butt, Chris Down, Yafang Shao, Vlastimil Babka,
Huang Ying, Pankaj Gupta, Matthew Wilcox, Konstantin Khlebnikov,
Minchan Kim, Jaewon Kim, cgroups, linux-mm, linux-kernel
On Fri, Sep 18, 2020 at 01:46:59PM -0700, Hugh Dickins wrote:
> On Thu, 17 Sep 2020, Yu Zhao wrote:
>
> > Hi Andrew,
> >
> > I see you have taken this:
> > mm: use add_page_to_lru_list()/page_lru()/page_off_lru()
> > Do you mind dropping it?
> >
> > Michal asked to do a bit of additional work. So I thought I probably
> > should create a series to do more cleanups I've been meaning to.
> >
> > This series contains the change in the patch above and goes a few
> > more steps farther. It's intended to improve readability and should
> > not have any performance impacts. There are minor behavior changes in
> > terms of debugging and error reporting, which I have all highlighted
> > in the individual patches. All patches were properly tested on 5.8
> > running Chrome OS, with various debug options turned on.
> >
> > Michal,
> >
> > Do you mind taking a looking at the entire series?
> >
> > Thank you.
> >
> > Yu Zhao (13):
> > mm: use add_page_to_lru_list()
> > mm: use page_off_lru()
> > mm: move __ClearPageLRU() into page_off_lru()
> > mm: shuffle lru list addition and deletion functions
> > mm: don't pass enum lru_list to lru list addition functions
> > mm: don't pass enum lru_list to trace_mm_lru_insertion()
> > mm: don't pass enum lru_list to del_page_from_lru_list()
> > mm: rename page_off_lru() to __clear_page_lru_flags()
> > mm: inline page_lru_base_type()
> > mm: VM_BUG_ON lru page flags
> > mm: inline __update_lru_size()
> > mm: make lruvec_lru_size() static
> > mm: enlarge the int parameter of update_lru_size()
> >
> > include/linux/memcontrol.h | 14 ++--
> > include/linux/mm_inline.h | 115 ++++++++++++++-------------------
> > include/linux/mmzone.h | 2 -
> > include/linux/vmstat.h | 2 +-
> > include/trace/events/pagemap.h | 11 ++--
> > mm/compaction.c | 2 +-
> > mm/memcontrol.c | 10 +--
> > mm/mlock.c | 2 +-
> > mm/swap.c | 53 ++++++---------
> > mm/vmscan.c | 28 +++-----
> > 10 files changed, 95 insertions(+), 144 deletions(-)
> >
> > --
> > 2.28.0.681.g6f77f65b4e-goog
>
> Sorry, Yu, I may be out-of-line in sending this: but as you know,
> Alex Shi has a long per-memcg lru_lock series playing in much the
> same area (particularly conflicting in mm/swap.c and mm/vmscan.c):
> a patchset that makes useful changes, that I'm very keen to help
> into mmotm a.s.a.p (but not before I've completed diligence).
>
> We've put a lot of effort into its testing, I'm currently reviewing
> it patch by patch (my general silence indicating that I'm busy on that,
> but slow as ever): so I'm a bit discouraged to have its stability
> potentially undermined by conflicting cleanups at this stage.
>
> If there's general agreement that your cleanups are safe and welcome
> (Michal's initial reaction sheds some doubt on that), great: I hope
> that Andrew can fast-track them into mmotm, then Alex rebase on top
> of them, and I then re-test and re-review.
>
> But if that quick agreement is not forthcoming, may I ask you please
> to hold back, and resend based on top of Alex's next posting?
The per-memcg lru lock series seems a high priority, and I have
absolutely no problem accommodate your request.
In return, may I ask you or Alex to review this series after you
have finished with per-memcg lru lock (to make sure that I resolve
all the conflicts correctly at least)?
^ permalink raw reply [flat|nested] 51+ messages in thread
[parent not found: <20200918210126.GA1118730-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>]
* Re: [PATCH 00/13] mm: clean up some lru related pieces
2020-09-18 21:01 ` Yu Zhao
@ 2020-09-18 21:19 ` Hugh Dickins
-1 siblings, 0 replies; 51+ messages in thread
From: Hugh Dickins @ 2020-09-18 21:19 UTC (permalink / raw)
To: Yu Zhao
Cc: Hugh Dickins, Andrew Morton, Michal Hocko, Alex Shi,
Steven Rostedt, Ingo Molnar, Johannes Weiner, Vladimir Davydov,
Roman Gushchin, Shakeel Butt, Chris Down, Yafang Shao,
Vlastimil Babka, Huang Ying, Pankaj Gupta, Matthew Wilcox,
Konstantin Khlebnikov, Minchan Kim, Jaewon Kim,
cgroups-u79uwXL29TY76Z2rM5mHXA, linux-mm
On Fri, 18 Sep 2020, Yu Zhao wrote:
> On Fri, Sep 18, 2020 at 01:46:59PM -0700, Hugh Dickins wrote:
> > On Thu, 17 Sep 2020, Yu Zhao wrote:
> >
> > > Hi Andrew,
> > >
> > > I see you have taken this:
> > > mm: use add_page_to_lru_list()/page_lru()/page_off_lru()
> > > Do you mind dropping it?
> > >
> > > Michal asked to do a bit of additional work. So I thought I probably
> > > should create a series to do more cleanups I've been meaning to.
> > >
> > > This series contains the change in the patch above and goes a few
> > > more steps farther. It's intended to improve readability and should
> > > not have any performance impacts. There are minor behavior changes in
> > > terms of debugging and error reporting, which I have all highlighted
> > > in the individual patches. All patches were properly tested on 5.8
> > > running Chrome OS, with various debug options turned on.
> > >
> > > Michal,
> > >
> > > Do you mind taking a looking at the entire series?
> > >
> > > Thank you.
> > >
> > > Yu Zhao (13):
> > > mm: use add_page_to_lru_list()
> > > mm: use page_off_lru()
> > > mm: move __ClearPageLRU() into page_off_lru()
> > > mm: shuffle lru list addition and deletion functions
> > > mm: don't pass enum lru_list to lru list addition functions
> > > mm: don't pass enum lru_list to trace_mm_lru_insertion()
> > > mm: don't pass enum lru_list to del_page_from_lru_list()
> > > mm: rename page_off_lru() to __clear_page_lru_flags()
> > > mm: inline page_lru_base_type()
> > > mm: VM_BUG_ON lru page flags
> > > mm: inline __update_lru_size()
> > > mm: make lruvec_lru_size() static
> > > mm: enlarge the int parameter of update_lru_size()
> > >
> > > include/linux/memcontrol.h | 14 ++--
> > > include/linux/mm_inline.h | 115 ++++++++++++++-------------------
> > > include/linux/mmzone.h | 2 -
> > > include/linux/vmstat.h | 2 +-
> > > include/trace/events/pagemap.h | 11 ++--
> > > mm/compaction.c | 2 +-
> > > mm/memcontrol.c | 10 +--
> > > mm/mlock.c | 2 +-
> > > mm/swap.c | 53 ++++++---------
> > > mm/vmscan.c | 28 +++-----
> > > 10 files changed, 95 insertions(+), 144 deletions(-)
> > >
> > > --
> > > 2.28.0.681.g6f77f65b4e-goog
> >
> > Sorry, Yu, I may be out-of-line in sending this: but as you know,
> > Alex Shi has a long per-memcg lru_lock series playing in much the
> > same area (particularly conflicting in mm/swap.c and mm/vmscan.c):
> > a patchset that makes useful changes, that I'm very keen to help
> > into mmotm a.s.a.p (but not before I've completed diligence).
> >
> > We've put a lot of effort into its testing, I'm currently reviewing
> > it patch by patch (my general silence indicating that I'm busy on that,
> > but slow as ever): so I'm a bit discouraged to have its stability
> > potentially undermined by conflicting cleanups at this stage.
> >
> > If there's general agreement that your cleanups are safe and welcome
> > (Michal's initial reaction sheds some doubt on that), great: I hope
> > that Andrew can fast-track them into mmotm, then Alex rebase on top
> > of them, and I then re-test and re-review.
> >
> > But if that quick agreement is not forthcoming, may I ask you please
> > to hold back, and resend based on top of Alex's next posting?
>
> The per-memcg lru lock series seems a high priority, and I have
> absolutely no problem accommodate your request.
Many thanks!
>
> In return, may I ask you or Alex to review this series after you
> have finished with per-memcg lru lock (to make sure that I resolve
> all the conflicts correctly at least)?
Fair enough: I promise to do so.
And your rebasing will necessarily lead you to review some parts
of Alex's patchset, which will help us all too.
Andrew, Yu asked at the start:
> > > I see you have taken this:
> > > mm: use add_page_to_lru_list()/page_lru()/page_off_lru()
> > > Do you mind dropping it?
Dropping that for now will help too.
Thanks,
Hugh
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH 00/13] mm: clean up some lru related pieces
@ 2020-09-18 21:19 ` Hugh Dickins
0 siblings, 0 replies; 51+ messages in thread
From: Hugh Dickins @ 2020-09-18 21:19 UTC (permalink / raw)
To: Yu Zhao
Cc: Hugh Dickins, Andrew Morton, Michal Hocko, Alex Shi,
Steven Rostedt, Ingo Molnar, Johannes Weiner, Vladimir Davydov,
Roman Gushchin, Shakeel Butt, Chris Down, Yafang Shao,
Vlastimil Babka, Huang Ying, Pankaj Gupta, Matthew Wilcox,
Konstantin Khlebnikov, Minchan Kim, Jaewon Kim, cgroups, linux-mm,
linux-kernel
On Fri, 18 Sep 2020, Yu Zhao wrote:
> On Fri, Sep 18, 2020 at 01:46:59PM -0700, Hugh Dickins wrote:
> > On Thu, 17 Sep 2020, Yu Zhao wrote:
> >
> > > Hi Andrew,
> > >
> > > I see you have taken this:
> > > mm: use add_page_to_lru_list()/page_lru()/page_off_lru()
> > > Do you mind dropping it?
> > >
> > > Michal asked to do a bit of additional work. So I thought I probably
> > > should create a series to do more cleanups I've been meaning to.
> > >
> > > This series contains the change in the patch above and goes a few
> > > more steps farther. It's intended to improve readability and should
> > > not have any performance impacts. There are minor behavior changes in
> > > terms of debugging and error reporting, which I have all highlighted
> > > in the individual patches. All patches were properly tested on 5.8
> > > running Chrome OS, with various debug options turned on.
> > >
> > > Michal,
> > >
> > > Do you mind taking a looking at the entire series?
> > >
> > > Thank you.
> > >
> > > Yu Zhao (13):
> > > mm: use add_page_to_lru_list()
> > > mm: use page_off_lru()
> > > mm: move __ClearPageLRU() into page_off_lru()
> > > mm: shuffle lru list addition and deletion functions
> > > mm: don't pass enum lru_list to lru list addition functions
> > > mm: don't pass enum lru_list to trace_mm_lru_insertion()
> > > mm: don't pass enum lru_list to del_page_from_lru_list()
> > > mm: rename page_off_lru() to __clear_page_lru_flags()
> > > mm: inline page_lru_base_type()
> > > mm: VM_BUG_ON lru page flags
> > > mm: inline __update_lru_size()
> > > mm: make lruvec_lru_size() static
> > > mm: enlarge the int parameter of update_lru_size()
> > >
> > > include/linux/memcontrol.h | 14 ++--
> > > include/linux/mm_inline.h | 115 ++++++++++++++-------------------
> > > include/linux/mmzone.h | 2 -
> > > include/linux/vmstat.h | 2 +-
> > > include/trace/events/pagemap.h | 11 ++--
> > > mm/compaction.c | 2 +-
> > > mm/memcontrol.c | 10 +--
> > > mm/mlock.c | 2 +-
> > > mm/swap.c | 53 ++++++---------
> > > mm/vmscan.c | 28 +++-----
> > > 10 files changed, 95 insertions(+), 144 deletions(-)
> > >
> > > --
> > > 2.28.0.681.g6f77f65b4e-goog
> >
> > Sorry, Yu, I may be out-of-line in sending this: but as you know,
> > Alex Shi has a long per-memcg lru_lock series playing in much the
> > same area (particularly conflicting in mm/swap.c and mm/vmscan.c):
> > a patchset that makes useful changes, that I'm very keen to help
> > into mmotm a.s.a.p (but not before I've completed diligence).
> >
> > We've put a lot of effort into its testing, I'm currently reviewing
> > it patch by patch (my general silence indicating that I'm busy on that,
> > but slow as ever): so I'm a bit discouraged to have its stability
> > potentially undermined by conflicting cleanups at this stage.
> >
> > If there's general agreement that your cleanups are safe and welcome
> > (Michal's initial reaction sheds some doubt on that), great: I hope
> > that Andrew can fast-track them into mmotm, then Alex rebase on top
> > of them, and I then re-test and re-review.
> >
> > But if that quick agreement is not forthcoming, may I ask you please
> > to hold back, and resend based on top of Alex's next posting?
>
> The per-memcg lru lock series seems a high priority, and I have
> absolutely no problem accommodate your request.
Many thanks!
>
> In return, may I ask you or Alex to review this series after you
> have finished with per-memcg lru lock (to make sure that I resolve
> all the conflicts correctly at least)?
Fair enough: I promise to do so.
And your rebasing will necessarily lead you to review some parts
of Alex's patchset, which will help us all too.
Andrew, Yu asked at the start:
> > > I see you have taken this:
> > > mm: use add_page_to_lru_list()/page_lru()/page_off_lru()
> > > Do you mind dropping it?
Dropping that for now will help too.
Thanks,
Hugh
^ permalink raw reply [flat|nested] 51+ messages in thread
[parent not found: <alpine.LSU.2.11.2009181406410.11531-fupSdm12i1nKWymIFiNcPA@public.gmane.org>]
* Re: [PATCH 00/13] mm: clean up some lru related pieces
2020-09-18 21:19 ` Hugh Dickins
@ 2020-09-19 0:31 ` Alex Shi
-1 siblings, 0 replies; 51+ messages in thread
From: Alex Shi @ 2020-09-19 0:31 UTC (permalink / raw)
To: Hugh Dickins, Yu Zhao
Cc: Andrew Morton, Michal Hocko, Steven Rostedt, Ingo Molnar,
Johannes Weiner, Vladimir Davydov, Roman Gushchin, Shakeel Butt,
Chris Down, Yafang Shao, Vlastimil Babka, Huang Ying,
Pankaj Gupta, Matthew Wilcox, Konstantin Khlebnikov, Minchan Kim,
Jaewon Kim, cgroups-u79uwXL29TY76Z2rM5mHXA,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
ÔÚ 2020/9/19 ÉÏÎç5:19, Hugh Dickins дµÀ:
>>>> 2.28.0.681.g6f77f65b4e-goog
>>> Sorry, Yu, I may be out-of-line in sending this: but as you know,
>>> Alex Shi has a long per-memcg lru_lock series playing in much the
>>> same area (particularly conflicting in mm/swap.c and mm/vmscan.c):
>>> a patchset that makes useful changes, that I'm very keen to help
>>> into mmotm a.s.a.p (but not before I've completed diligence).
>>>
>>> We've put a lot of effort into its testing, I'm currently reviewing
>>> it patch by patch (my general silence indicating that I'm busy on that,
>>> but slow as ever): so I'm a bit discouraged to have its stability
>>> potentially undermined by conflicting cleanups at this stage.
>>>
>>> If there's general agreement that your cleanups are safe and welcome
>>> (Michal's initial reaction sheds some doubt on that), great: I hope
>>> that Andrew can fast-track them into mmotm, then Alex rebase on top
>>> of them, and I then re-test and re-review.
>>>
>>> But if that quick agreement is not forthcoming, may I ask you please
>>> to hold back, and resend based on top of Alex's next posting?
>> The per-memcg lru lock series seems a high priority, and I have
>> absolutely no problem accommodate your request.
> Many thanks!
>
>> In return, may I ask you or Alex to review this series after you
>> have finished with per-memcg lru lock (to make sure that I resolve
>> all the conflicts correctly at least)?
> Fair enough: I promise to do so.
>
> And your rebasing will necessarily lead you to review some parts
> of Alex's patchset, which will help us all too.
>
> Andrew, Yu asked at the start:
>>>> I see you have taken this:
>>>> mm: use add_page_to_lru_list()/page_lru()/page_off_lru()
>>>> Do you mind dropping it?
> Dropping that for now will help too.
Hi Hugh & Yu,
Thanks for all your considerations! I will looking into this series after thing
on lru_lock finished.
Thanks a lot!
Alex
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH 00/13] mm: clean up some lru related pieces
@ 2020-09-19 0:31 ` Alex Shi
0 siblings, 0 replies; 51+ messages in thread
From: Alex Shi @ 2020-09-19 0:31 UTC (permalink / raw)
To: Hugh Dickins, Yu Zhao
Cc: Andrew Morton, Michal Hocko, Steven Rostedt, Ingo Molnar,
Johannes Weiner, Vladimir Davydov, Roman Gushchin, Shakeel Butt,
Chris Down, Yafang Shao, Vlastimil Babka, Huang Ying,
Pankaj Gupta, Matthew Wilcox, Konstantin Khlebnikov, Minchan Kim,
Jaewon Kim, cgroups, linux-mm, linux-kernel
在 2020/9/19 上午5:19, Hugh Dickins 写道:
>>>> 2.28.0.681.g6f77f65b4e-goog
>>> Sorry, Yu, I may be out-of-line in sending this: but as you know,
>>> Alex Shi has a long per-memcg lru_lock series playing in much the
>>> same area (particularly conflicting in mm/swap.c and mm/vmscan.c):
>>> a patchset that makes useful changes, that I'm very keen to help
>>> into mmotm a.s.a.p (but not before I've completed diligence).
>>>
>>> We've put a lot of effort into its testing, I'm currently reviewing
>>> it patch by patch (my general silence indicating that I'm busy on that,
>>> but slow as ever): so I'm a bit discouraged to have its stability
>>> potentially undermined by conflicting cleanups at this stage.
>>>
>>> If there's general agreement that your cleanups are safe and welcome
>>> (Michal's initial reaction sheds some doubt on that), great: I hope
>>> that Andrew can fast-track them into mmotm, then Alex rebase on top
>>> of them, and I then re-test and re-review.
>>>
>>> But if that quick agreement is not forthcoming, may I ask you please
>>> to hold back, and resend based on top of Alex's next posting?
>> The per-memcg lru lock series seems a high priority, and I have
>> absolutely no problem accommodate your request.
> Many thanks!
>
>> In return, may I ask you or Alex to review this series after you
>> have finished with per-memcg lru lock (to make sure that I resolve
>> all the conflicts correctly at least)?
> Fair enough: I promise to do so.
>
> And your rebasing will necessarily lead you to review some parts
> of Alex's patchset, which will help us all too.
>
> Andrew, Yu asked at the start:
>>>> I see you have taken this:
>>>> mm: use add_page_to_lru_list()/page_lru()/page_off_lru()
>>>> Do you mind dropping it?
> Dropping that for now will help too.
Hi Hugh & Yu,
Thanks for all your considerations! I will looking into this series after thing
on lru_lock finished.
Thanks a lot!
Alex
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH 00/13] mm: clean up some lru related pieces
2020-09-18 21:19 ` Hugh Dickins
@ 2020-10-13 2:30 ` Hugh Dickins
-1 siblings, 0 replies; 51+ messages in thread
From: Hugh Dickins @ 2020-10-13 2:30 UTC (permalink / raw)
To: Andrew Morton
Cc: Yu Zhao, Hugh Dickins, Michal Hocko, Alex Shi, Steven Rostedt,
Ingo Molnar, Johannes Weiner, Vladimir Davydov, Roman Gushchin,
Shakeel Butt, Chris Down, Yafang Shao, Vlastimil Babka,
Huang Ying, Pankaj Gupta, Matthew Wilcox, Konstantin Khlebnikov,
Minchan Kim, Jaewon Kim, cgroups-u79uwXL29TY76Z2rM5mHXA,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg, linux-kernel
On Fri, 18 Sep 2020, Hugh Dickins wrote:
> On Fri, 18 Sep 2020, Yu Zhao wrote:
> > On Fri, Sep 18, 2020 at 01:46:59PM -0700, Hugh Dickins wrote:
> > > On Thu, 17 Sep 2020, Yu Zhao wrote:
> > >
> > > > Hi Andrew,
> > > >
> > > > I see you have taken this:
> > > > mm: use add_page_to_lru_list()/page_lru()/page_off_lru()
> > > > Do you mind dropping it?
> > > >
> > > > Michal asked to do a bit of additional work. So I thought I probably
> > > > should create a series to do more cleanups I've been meaning to.
> > > >
> > > > This series contains the change in the patch above and goes a few
> > > > more steps farther. It's intended to improve readability and should
> > > > not have any performance impacts. There are minor behavior changes in
> > > > terms of debugging and error reporting, which I have all highlighted
> > > > in the individual patches. All patches were properly tested on 5.8
> > > > running Chrome OS, with various debug options turned on.
> > > >
> > > > Michal,
> > > >
> > > > Do you mind taking a looking at the entire series?
> > > >
> > > > Thank you.
> > > >
> > > > Yu Zhao (13):
> > > > mm: use add_page_to_lru_list()
> > > > mm: use page_off_lru()
> > > > mm: move __ClearPageLRU() into page_off_lru()
> > > > mm: shuffle lru list addition and deletion functions
> > > > mm: don't pass enum lru_list to lru list addition functions
> > > > mm: don't pass enum lru_list to trace_mm_lru_insertion()
> > > > mm: don't pass enum lru_list to del_page_from_lru_list()
> > > > mm: rename page_off_lru() to __clear_page_lru_flags()
> > > > mm: inline page_lru_base_type()
> > > > mm: VM_BUG_ON lru page flags
> > > > mm: inline __update_lru_size()
> > > > mm: make lruvec_lru_size() static
> > > > mm: enlarge the int parameter of update_lru_size()
> > > >
> > > > include/linux/memcontrol.h | 14 ++--
> > > > include/linux/mm_inline.h | 115 ++++++++++++++-------------------
> > > > include/linux/mmzone.h | 2 -
> > > > include/linux/vmstat.h | 2 +-
> > > > include/trace/events/pagemap.h | 11 ++--
> > > > mm/compaction.c | 2 +-
> > > > mm/memcontrol.c | 10 +--
> > > > mm/mlock.c | 2 +-
> > > > mm/swap.c | 53 ++++++---------
> > > > mm/vmscan.c | 28 +++-----
> > > > 10 files changed, 95 insertions(+), 144 deletions(-)
> > > >
> > > > --
> > > > 2.28.0.681.g6f77f65b4e-goog
> > >
> > > Sorry, Yu, I may be out-of-line in sending this: but as you know,
> > > Alex Shi has a long per-memcg lru_lock series playing in much the
> > > same area (particularly conflicting in mm/swap.c and mm/vmscan.c):
> > > a patchset that makes useful changes, that I'm very keen to help
> > > into mmotm a.s.a.p (but not before I've completed diligence).
> > >
> > > We've put a lot of effort into its testing, I'm currently reviewing
> > > it patch by patch (my general silence indicating that I'm busy on that,
> > > but slow as ever): so I'm a bit discouraged to have its stability
> > > potentially undermined by conflicting cleanups at this stage.
> > >
> > > If there's general agreement that your cleanups are safe and welcome
> > > (Michal's initial reaction sheds some doubt on that), great: I hope
> > > that Andrew can fast-track them into mmotm, then Alex rebase on top
> > > of them, and I then re-test and re-review.
> > >
> > > But if that quick agreement is not forthcoming, may I ask you please
> > > to hold back, and resend based on top of Alex's next posting?
> >
> > The per-memcg lru lock series seems a high priority, and I have
> > absolutely no problem accommodate your request.
>
> Many thanks!
>
> >
> > In return, may I ask you or Alex to review this series after you
> > have finished with per-memcg lru lock (to make sure that I resolve
> > all the conflicts correctly at least)?
>
> Fair enough: I promise to do so.
>
> And your rebasing will necessarily lead you to review some parts
> of Alex's patchset, which will help us all too.
>
> Andrew, Yu asked at the start:
> > > > I see you have taken this:
> > > > mm: use add_page_to_lru_list()/page_lru()/page_off_lru()
> > > > Do you mind dropping it?
> Dropping that for now will help too.
Andrew, please drop Yu Zhao's
mm-use-add_page_to_lru_list-page_lru-page_off_lru.patch
from the mmotm tree.
Yu wants to replace it by this cleanup series, and he has graciously
agreed to rebase his series on top of Alex Shi's per-memcg lru_lock
series; but mm-use-add_page_to_lru_list-page_lru-page_off_lru.patch
is getting in the way of adding them to mmotm. The three of us are
all hoping that Alex's v19 series can go into mmotm when the merge
window closes, then I'll review Yu's series rebased on top of it.
Thanks,
Hugh
^ permalink raw reply [flat|nested] 51+ messages in thread
* Re: [PATCH 00/13] mm: clean up some lru related pieces
@ 2020-10-13 2:30 ` Hugh Dickins
0 siblings, 0 replies; 51+ messages in thread
From: Hugh Dickins @ 2020-10-13 2:30 UTC (permalink / raw)
To: Andrew Morton
Cc: Yu Zhao, Hugh Dickins, Michal Hocko, Alex Shi, Steven Rostedt,
Ingo Molnar, Johannes Weiner, Vladimir Davydov, Roman Gushchin,
Shakeel Butt, Chris Down, Yafang Shao, Vlastimil Babka,
Huang Ying, Pankaj Gupta, Matthew Wilcox, Konstantin Khlebnikov,
Minchan Kim, Jaewon Kim, cgroups, linux-mm, linux-kernel
On Fri, 18 Sep 2020, Hugh Dickins wrote:
> On Fri, 18 Sep 2020, Yu Zhao wrote:
> > On Fri, Sep 18, 2020 at 01:46:59PM -0700, Hugh Dickins wrote:
> > > On Thu, 17 Sep 2020, Yu Zhao wrote:
> > >
> > > > Hi Andrew,
> > > >
> > > > I see you have taken this:
> > > > mm: use add_page_to_lru_list()/page_lru()/page_off_lru()
> > > > Do you mind dropping it?
> > > >
> > > > Michal asked to do a bit of additional work. So I thought I probably
> > > > should create a series to do more cleanups I've been meaning to.
> > > >
> > > > This series contains the change in the patch above and goes a few
> > > > more steps farther. It's intended to improve readability and should
> > > > not have any performance impacts. There are minor behavior changes in
> > > > terms of debugging and error reporting, which I have all highlighted
> > > > in the individual patches. All patches were properly tested on 5.8
> > > > running Chrome OS, with various debug options turned on.
> > > >
> > > > Michal,
> > > >
> > > > Do you mind taking a looking at the entire series?
> > > >
> > > > Thank you.
> > > >
> > > > Yu Zhao (13):
> > > > mm: use add_page_to_lru_list()
> > > > mm: use page_off_lru()
> > > > mm: move __ClearPageLRU() into page_off_lru()
> > > > mm: shuffle lru list addition and deletion functions
> > > > mm: don't pass enum lru_list to lru list addition functions
> > > > mm: don't pass enum lru_list to trace_mm_lru_insertion()
> > > > mm: don't pass enum lru_list to del_page_from_lru_list()
> > > > mm: rename page_off_lru() to __clear_page_lru_flags()
> > > > mm: inline page_lru_base_type()
> > > > mm: VM_BUG_ON lru page flags
> > > > mm: inline __update_lru_size()
> > > > mm: make lruvec_lru_size() static
> > > > mm: enlarge the int parameter of update_lru_size()
> > > >
> > > > include/linux/memcontrol.h | 14 ++--
> > > > include/linux/mm_inline.h | 115 ++++++++++++++-------------------
> > > > include/linux/mmzone.h | 2 -
> > > > include/linux/vmstat.h | 2 +-
> > > > include/trace/events/pagemap.h | 11 ++--
> > > > mm/compaction.c | 2 +-
> > > > mm/memcontrol.c | 10 +--
> > > > mm/mlock.c | 2 +-
> > > > mm/swap.c | 53 ++++++---------
> > > > mm/vmscan.c | 28 +++-----
> > > > 10 files changed, 95 insertions(+), 144 deletions(-)
> > > >
> > > > --
> > > > 2.28.0.681.g6f77f65b4e-goog
> > >
> > > Sorry, Yu, I may be out-of-line in sending this: but as you know,
> > > Alex Shi has a long per-memcg lru_lock series playing in much the
> > > same area (particularly conflicting in mm/swap.c and mm/vmscan.c):
> > > a patchset that makes useful changes, that I'm very keen to help
> > > into mmotm a.s.a.p (but not before I've completed diligence).
> > >
> > > We've put a lot of effort into its testing, I'm currently reviewing
> > > it patch by patch (my general silence indicating that I'm busy on that,
> > > but slow as ever): so I'm a bit discouraged to have its stability
> > > potentially undermined by conflicting cleanups at this stage.
> > >
> > > If there's general agreement that your cleanups are safe and welcome
> > > (Michal's initial reaction sheds some doubt on that), great: I hope
> > > that Andrew can fast-track them into mmotm, then Alex rebase on top
> > > of them, and I then re-test and re-review.
> > >
> > > But if that quick agreement is not forthcoming, may I ask you please
> > > to hold back, and resend based on top of Alex's next posting?
> >
> > The per-memcg lru lock series seems a high priority, and I have
> > absolutely no problem accommodate your request.
>
> Many thanks!
>
> >
> > In return, may I ask you or Alex to review this series after you
> > have finished with per-memcg lru lock (to make sure that I resolve
> > all the conflicts correctly at least)?
>
> Fair enough: I promise to do so.
>
> And your rebasing will necessarily lead you to review some parts
> of Alex's patchset, which will help us all too.
>
> Andrew, Yu asked at the start:
> > > > I see you have taken this:
> > > > mm: use add_page_to_lru_list()/page_lru()/page_off_lru()
> > > > Do you mind dropping it?
> Dropping that for now will help too.
Andrew, please drop Yu Zhao's
mm-use-add_page_to_lru_list-page_lru-page_off_lru.patch
from the mmotm tree.
Yu wants to replace it by this cleanup series, and he has graciously
agreed to rebase his series on top of Alex Shi's per-memcg lru_lock
series; but mm-use-add_page_to_lru_list-page_lru-page_off_lru.patch
is getting in the way of adding them to mmotm. The three of us are
all hoping that Alex's v19 series can go into mmotm when the merge
window closes, then I'll review Yu's series rebased on top of it.
Thanks,
Hugh
^ permalink raw reply [flat|nested] 51+ messages in thread