linux-kernel - Re: [PATCH 00/11] mm: lru related cleanups

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <54bdbe42-023a-4e32-9b94-173d0ad2dc16@linux.alibaba.com>
Date:   Thu, 10 Dec 2020 17:28:08 +0800
From:   Alex Shi <alex.shi@...ux.alibaba.com>
To:     Yu Zhao <yuzhao@...gle.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Hugh Dickins <hughd@...gle.com>
Cc:     Michal Hocko <mhocko@...nel.org>,
        Johannes Weiner <hannes@...xchg.org>,
        Vladimir Davydov <vdavydov.dev@...il.com>,
        Roman Gushchin <guro@...com>, Vlastimil Babka <vbabka@...e.cz>,
        Matthew Wilcox <willy@...radead.org>, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 00/11] mm: lru related cleanups

Hi Yu,

btw, after this patchset, to do cacheline alignment on each of lru lists
are possible, so did you try that to see performance changes?

Thanks
Alex

在 2020/12/8 上午6:09, Yu Zhao 写道:
> The cleanups are intended to reduce the verbosity in lru list
> operations and make them less error-prone. A typical example
> would be how the patches change __activate_page():
> 
>  static void __activate_page(struct page *page, struct lruvec *lruvec)
>  {
>  	if (!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);
> -		lru += LRU_ACTIVE;
> -		add_page_to_lru_list(page, lruvec, lru);
> +		add_page_to_lru_list(page, lruvec);
>  		trace_mm_lru_activate(page);
>  
> There are a few more places like __activate_page() and they are
> unnecessarily repetitive in terms of figuring out which list a page
> should be added onto or deleted from. And with the duplicated code
> removed, they are easier to read, IMO.
> 
> Patch 1 to 5 basically cover the above. Patch 6 and 7 make code more
> robust by improving bug reporting. Patch 8, 9 and 10 take care of
> some dangling helpers left in header files. Patch 11 isn't strictly a
> clean-up patch, but it seems still relevant to include it here.
> 
> Yu Zhao (11):
>   mm: use add_page_to_lru_list()
>   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: add __clear_page_lru_flags() to replace page_off_lru()
>   mm: VM_BUG_ON lru page flags
>   mm: fold page_lru_base_type() into its sole caller
>   mm: fold __update_lru_size() into its sole caller
>   mm: make lruvec_lru_size() static
>   mm: enlarge the "int nr_pages" parameter of update_lru_size()
> 
>  include/linux/memcontrol.h     |  10 +--
>  include/linux/mm_inline.h      | 115 ++++++++++++++-------------------
>  include/linux/mmzone.h         |   2 -
>  include/linux/vmstat.h         |   6 +-
>  include/trace/events/pagemap.h |  11 ++--
>  mm/compaction.c                |   2 +-
>  mm/memcontrol.c                |  10 +--
>  mm/mlock.c                     |   3 +-
>  mm/swap.c                      |  50 ++++++--------
>  mm/vmscan.c                    |  21 ++----
>  10 files changed, 91 insertions(+), 139 deletions(-)
>