From: Alex Shi <alex.shi@linux.alibaba.com>
To: Yu Zhao <yuzhao@google.com>,
Andrew Morton <akpm@linux-foundation.org>,
Matthew Wilcox <willy@infradead.org>,
Vlastimil Babka <vbabka@suse.cz>
Cc: Hugh Dickins <hughd@google.com>, Michal Hocko <mhocko@kernel.org>,
Johannes Weiner <hannes@cmpxchg.org>,
Vladimir Davydov <vdavydov.dev@gmail.com>,
Roman Gushchin <guro@fb.com>,
linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 03/10] mm: don't pass "enum lru_list" to lru list addition functions
Date: Wed, 24 Feb 2021 16:06:45 +0800 [thread overview]
Message-ID: <ce13ff94-e534-a4ed-4653-d9915f35d45a@linux.alibaba.com> (raw)
In-Reply-To: <YDXkPaKb+nVzkfZX@google.com>
在 2021/2/24 下午1:29, Yu Zhao 写道:
> On Tue, Feb 23, 2021 at 02:50:11PM -0800, Andrew Morton wrote:
>> On Tue, 26 Jan 2021 15:14:38 -0700 Yu Zhao <yuzhao@google.com> wrote:
>>
>>> On Tue, Jan 26, 2021 at 10:01:11PM +0000, Matthew Wilcox wrote:
>>>> On Fri, Jan 22, 2021 at 03:05:53PM -0700, Yu Zhao wrote:
>>>>> +++ b/mm/swap.c
>>>>> @@ -231,7 +231,7 @@ static void pagevec_move_tail_fn(struct page *page, struct lruvec *lruvec)
>>>>> if (!PageUnevictable(page)) {
>>>>> del_page_from_lru_list(page, lruvec, page_lru(page));
>>>>> ClearPageActive(page);
>>>>> - add_page_to_lru_list_tail(page, lruvec, page_lru(page));
>>>>> + add_page_to_lru_list_tail(page, lruvec);
>>>>> __count_vm_events(PGROTATED, thp_nr_pages(page));
>>>>> }
>>>>
>>>> Is it profitable to do ...
>>>>
>>>> - del_page_from_lru_list(page, lruvec, page_lru(page));
>>>> + enum lru_list lru = page_lru(page);
>>>> + del_page_from_lru_list(page, lruvec, lru);
>>>> ClearPageActive(page);
>>>> - add_page_to_lru_list_tail(page, lruvec, page_lru(page));
>>>> + lru &= ~LRU_ACTIVE;
>>>> + add_page_to_lru_list_tail(page, lruvec, lru);
>>>
>>> Ok, now we want to trade readability for size. Sure, I'll see how
>>> much we could squeeze.
>>
>> As nothing has happened here and the code bloat issue remains, I'll
>> hold this series out of 5.12-rc1.
>
> Sorry for the slow response. I was trying to ascertain why
> page_lru(), a tiny helper, could bloat vmlinux by O(KB). It turned out
> compound_head() included in Page{Active,Unevictable} is a nuisance in
> our case. Testing PG_{active,unevictable} against
> compound_head(page)->flags is really unnecessary because all lru
> operations are eventually done on page->lru not
> compound_head(page)->lru. With the following change, which sacrifices
> the readability a bit, we gain 998 bytes with Clang but lose 227 bytes
> with GCC, which IMO is a win. (We use Clang by default.)
>
>
> diff --git a/include/linux/mm_inline.h b/include/linux/mm_inline.h
> index 355ea1ee32bd..ec0878a3cdfe 100644
> --- a/include/linux/mm_inline.h
> +++ b/include/linux/mm_inline.h
> @@ -46,14 +46,12 @@ 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() */
> - if (PageActive(page) && PageUnevictable(page))
> + if ((page->flags & (BIT(PG_active) | BIT(PG_unevictable))) ==
> + (BIT(PG_active) | BIT(PG_unevictable)))
> return;
>
> - __ClearPageActive(page);
> - __ClearPageUnevictable(page);
> + page->flags &= ~(BIT(PG_lru) | BIT(PG_active) | BIT(PG_unevictable));
> }
>
> /**
> @@ -65,18 +63,12 @@ static __always_inline void __clear_page_lru_flags(struct page *page)
> */
> static __always_inline enum lru_list page_lru(struct page *page)
> {
> - enum lru_list lru;
> + unsigned long flags = READ_ONCE(page->flags);
>
> VM_BUG_ON_PAGE(PageActive(page) && PageUnevictable(page), page);
>
> - if (PageUnevictable(page))
> - return LRU_UNEVICTABLE;
> -
> - lru = page_is_file_lru(page) ? LRU_INACTIVE_FILE : LRU_INACTIVE_ANON;
> - if (PageActive(page))
> - lru += LRU_ACTIVE;
> -
> - return lru;
> + return (flags & BIT(PG_unevictable)) ? LRU_UNEVICTABLE :
> + (LRU_FILE * !(flags & BIT(PG_swapbacked)) + !!(flags & BIT(PG_active)));
Currently each of page flags used different flags policy, does this mean above flags could be
change to PF_ANY policy?
Thanks
Alex
> }
>
> static __always_inline void add_page_to_lru_list(struct page *page,
>
>
> I'll post this as a separate patch. Below the bloat-o-meter collected
> on top of c03c21ba6f4e.
>
> $ ./scripts/bloat-o-meter ../vmlinux.clang.orig ../vmlinux.clang
> add/remove: 0/1 grow/shrink: 7/10 up/down: 191/-1189 (-998)
> Function old new delta
> lru_lazyfree_fn 848 893 +45
> lru_deactivate_file_fn 1037 1075 +38
> perf_trace_mm_lru_insertion 515 548 +33
> check_move_unevictable_pages 983 1006 +23
> __activate_page 706 729 +23
> trace_event_raw_event_mm_lru_insertion 476 497 +21
> lru_deactivate_fn 691 699 +8
> __bpf_trace_mm_lru_insertion 13 11 -2
> __traceiter_mm_lru_insertion 67 62 -5
> move_pages_to_lru 964 881 -83
> __pagevec_lru_add_fn 665 581 -84
> isolate_lru_page 524 419 -105
> __munlock_pagevec 1609 1481 -128
> isolate_migratepages_block 3370 3237 -133
> __page_cache_release 556 413 -143
> lruvec_lru_size 151 - -151
> release_pages 1025 866 -159
> pagevec_move_tail_fn 805 609 -196
> Total: Before=19502982, After=19501984, chg -0.01%
>
> $ ./scripts/bloat-o-meter ../vmlinux.gcc.orig ../vmlinux.gcc
> add/remove: 0/1 grow/shrink: 9/9 up/down: 1010/-783 (227)
> Function old new delta
> shrink_lruvec 1690 1950 +260
> lru_deactivate_file_fn 961 1128 +167
> isolate_migratepages_block 3286 3427 +141
> check_move_unevictable_pages 1042 1170 +128
> lru_lazyfree_fn 709 822 +113
> lru_deactivate_fn 665 724 +59
> __activate_page 703 760 +57
> trace_event_raw_event_mm_lru_insertion 432 478 +46
> perf_trace_mm_lru_insertion 464 503 +39
> __bpf_trace_mm_lru_insertion 13 11 -2
> __traceiter_mm_lru_insertion 66 57 -9
> isolate_lru_page 472 405 -67
> __munlock_pagevec 1282 1212 -70
> __pagevec_lru_add 976 893 -83
> __page_cache_release 508 418 -90
> release_pages 978 887 -91
> move_pages_to_lru 954 853 -101
> lruvec_lru_size 131 - -131
> pagevec_move_tail_fn 770 631 -139
> Total: Before=19237248, After=19237475, chg +0.00%
>
next prev parent reply other threads:[~2021-02-24 8:07 UTC|newest]
Thread overview: 66+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-22 22:05 [PATCH v2 00/10] mm: lru related cleanups Yu Zhao
2021-01-22 22:05 ` Yu Zhao
2021-01-22 22:05 ` [PATCH v2 01/10] mm: use add_page_to_lru_list() Yu Zhao
2021-01-22 22:05 ` Yu Zhao
2021-01-26 18:57 ` Vlastimil Babka
2021-01-27 2:12 ` Miaohe Lin
2021-01-22 22:05 ` [PATCH v2 02/10] mm: shuffle lru list addition and deletion functions Yu Zhao
2021-01-22 22:05 ` Yu Zhao
2021-01-26 18:58 ` Vlastimil Babka
2021-01-27 2:14 ` Miaohe Lin
2021-01-22 22:05 ` [PATCH v2 03/10] mm: don't pass "enum lru_list" to lru list addition functions Yu Zhao
2021-01-22 22:05 ` Yu Zhao
2021-01-26 19:13 ` Vlastimil Babka
2021-01-26 21:34 ` Yu Zhao
2021-01-27 10:51 ` Vlastimil Babka
2021-01-26 22:01 ` Matthew Wilcox
2021-01-26 22:14 ` Yu Zhao
2021-02-23 22:50 ` Andrew Morton
2021-02-24 5:29 ` Yu Zhao
2021-02-24 8:06 ` Alex Shi [this message]
2021-02-24 8:37 ` Yu Zhao
2021-02-24 9:01 ` Alex Shi
2021-01-22 22:05 ` [PATCH v2 04/10] mm: don't pass "enum lru_list" to trace_mm_lru_insertion() Yu Zhao
2021-01-22 22:05 ` Yu Zhao
2021-01-22 22:05 ` [PATCH v2 05/10] mm: don't pass "enum lru_list" to del_page_from_lru_list() Yu Zhao
2021-01-22 22:05 ` Yu Zhao
2021-01-22 22:05 ` [PATCH v2 06/10] mm: add __clear_page_lru_flags() to replace page_off_lru() Yu Zhao
2021-01-22 22:05 ` Yu Zhao
2021-01-22 22:05 ` [PATCH v2 07/10] mm: VM_BUG_ON lru page flags Yu Zhao
2021-01-22 22:05 ` Yu Zhao
2021-01-22 22:05 ` [PATCH v2 08/10] mm: fold page_lru_base_type() into its sole caller Yu Zhao
2021-01-22 22:05 ` Yu Zhao
2021-01-22 22:05 ` [PATCH v2 09/10] mm: fold __update_lru_size() " Yu Zhao
2021-01-22 22:05 ` Yu Zhao
2021-01-22 22:06 ` [PATCH v2 10/10] mm: make lruvec_lru_size() static Yu Zhao
2021-01-22 22:06 ` Yu Zhao
2021-02-24 8:48 ` [PATCH] mm: test page->flags directly in page_lru() Yu Zhao
2021-02-24 8:48 ` Yu Zhao
2021-02-24 13:15 ` Andrew Morton
2021-02-24 19:57 ` Yu Zhao
2021-02-24 21:56 ` Matthew Wilcox
2021-02-24 22:34 ` Yu Zhao
2021-02-24 22:48 ` Matthew Wilcox
2021-02-24 23:50 ` Yu Zhao
2021-02-25 3:55 ` Matthew Wilcox
2021-02-25 5:22 ` Yu Zhao
2021-02-25 12:12 ` Matthew Wilcox
2021-02-26 9:17 ` [PATCH v2 0/3] trim the uses of compound_head() Yu Zhao
2021-02-26 9:17 ` Yu Zhao
2021-02-26 9:17 ` [PATCH v2 1/3] mm: bypass compound_head() for PF_NO_TAIL when enforce=1 Yu Zhao
2021-02-26 9:17 ` Yu Zhao
2021-02-26 9:17 ` [PATCH v2 2/3] mm: use PF_NO_TAIL for PG_lru Yu Zhao
2021-02-26 9:17 ` Yu Zhao
2021-02-26 20:22 ` Yu Zhao
2021-02-26 9:17 ` [PATCH v2 3/3] mm: use PF_ONLY_HEAD for PG_active and PG_unevictable Yu Zhao
2021-02-26 9:17 ` Yu Zhao
2021-02-26 12:13 ` Matthew Wilcox
2021-02-26 19:49 ` Yu Zhao
2021-02-26 20:27 ` Matthew Wilcox
2021-03-01 11:50 ` Kirill A. Shutemov
2021-03-01 19:58 ` Yu Zhao
2021-03-01 20:16 ` Hugh Dickins
2021-03-01 20:16 ` Hugh Dickins
2021-03-01 20:26 ` Matthew Wilcox
2021-02-26 10:52 ` [PATCH v2 0/3] trim the uses of compound_head() Vlastimil Babka
2021-02-26 19:04 ` Yu Zhao
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ce13ff94-e534-a4ed-4653-d9915f35d45a@linux.alibaba.com \
--to=alex.shi@linux.alibaba.com \
--cc=akpm@linux-foundation.org \
--cc=guro@fb.com \
--cc=hannes@cmpxchg.org \
--cc=hughd@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mhocko@kernel.org \
--cc=vbabka@suse.cz \
--cc=vdavydov.dev@gmail.com \
--cc=willy@infradead.org \
--cc=yuzhao@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.