From: Michal Hocko <mhocko@suse.com>
To: Yu Zhao <yuzhao@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Alex Shi <alex.shi@linux.alibaba.com>,
Steven Rostedt <rostedt@goodmis.org>,
Ingo Molnar <mingo@redhat.com>,
Johannes Weiner <hannes@cmpxchg.org>,
Vladimir Davydov <vdavydov.dev@gmail.com>,
Roman Gushchin <guro@fb.com>, Shakeel Butt <shakeelb@google.com>,
Chris Down <chris@chrisdown.name>,
Yafang Shao <laoar.shao@gmail.com>,
Vlastimil Babka <vbabka@suse.cz>,
Huang Ying <ying.huang@intel.com>,
Pankaj Gupta <pankaj.gupta.linux@gmail.com>,
Matthew Wilcox <willy@infradead.org>,
Konstantin Khlebnikov <koct9i@gmail.com>,
Minchan Kim <minchan@kernel.org>,
Jaewon Kim <jaewon31.kim@samsung.com>,
cgroups@vger.kernel.org, linux-mm@kvack.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 00/13] mm: clean up some lru related pieces
Date: Fri, 18 Sep 2020 09:45:49 +0200 [thread overview]
Message-ID: <20200918074549.GG28827@dhcp22.suse.cz> (raw)
In-Reply-To: <20200918030051.650890-1-yuzhao@google.com>
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
next prev parent reply other threads:[~2020-09-18 7:45 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-18 3:00 [PATCH 00/13] mm: clean up some lru related pieces Yu Zhao
2020-09-18 3:00 ` [PATCH 01/13] mm: use add_page_to_lru_list() Yu Zhao
2020-09-18 7:32 ` Michal Hocko
2020-09-18 10:05 ` Yu Zhao
2020-09-18 3:00 ` [PATCH 02/13] mm: use page_off_lru() Yu Zhao
2020-09-18 7:37 ` Michal Hocko
2020-09-18 10:27 ` Yu Zhao
2020-09-18 11:09 ` Michal Hocko
2020-09-18 18:53 ` Yu Zhao
2020-09-21 11:16 ` Michal Hocko
2020-09-18 11:24 ` Michal Hocko
2020-09-18 3:00 ` [PATCH 03/13] mm: move __ClearPageLRU() into page_off_lru() Yu Zhao
2020-09-18 7:38 ` Michal Hocko
2020-09-18 3:00 ` [PATCH 04/13] mm: shuffle lru list addition and deletion functions Yu Zhao
2020-09-18 3:00 ` [PATCH 05/13] mm: don't pass enum lru_list to lru list addition functions Yu Zhao
2020-09-18 3:00 ` [PATCH 06/13] mm: don't pass enum lru_list to trace_mm_lru_insertion() Yu Zhao
2020-09-18 3:00 ` [PATCH 07/13] mm: don't pass enum lru_list to del_page_from_lru_list() Yu Zhao
2020-09-18 3:00 ` [PATCH 08/13] mm: rename page_off_lru() to __clear_page_lru_flags() Yu Zhao
2020-09-18 3:00 ` [PATCH 09/13] mm: inline page_lru_base_type() Yu Zhao
2020-09-18 3:00 ` [PATCH 10/13] mm: VM_BUG_ON lru page flags Yu Zhao
2020-09-18 3:00 ` [PATCH 11/13] mm: inline __update_lru_size() Yu Zhao
2020-09-18 3:00 ` [PATCH 12/13] mm: make lruvec_lru_size() static Yu Zhao
2020-09-18 3:00 ` [PATCH 13/13] mm: enlarge the int parameter of update_lru_size() Yu Zhao
2020-09-18 7:45 ` Michal Hocko [this message]
2020-09-18 9:36 ` [PATCH 00/13] mm: clean up some lru related pieces Yu Zhao
2020-09-18 20:46 ` Hugh Dickins
2020-09-18 21:01 ` Yu Zhao
2020-09-18 21:19 ` Hugh Dickins
2020-09-19 0:31 ` Alex Shi
2020-10-13 2:30 ` Hugh Dickins
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200918074549.GG28827@dhcp22.suse.cz \
--to=mhocko@suse.com \
--cc=akpm@linux-foundation.org \
--cc=alex.shi@linux.alibaba.com \
--cc=cgroups@vger.kernel.org \
--cc=chris@chrisdown.name \
--cc=guro@fb.com \
--cc=hannes@cmpxchg.org \
--cc=jaewon31.kim@samsung.com \
--cc=koct9i@gmail.com \
--cc=laoar.shao@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=minchan@kernel.org \
--cc=mingo@redhat.com \
--cc=pankaj.gupta.linux@gmail.com \
--cc=rostedt@goodmis.org \
--cc=shakeelb@google.com \
--cc=vbabka@suse.cz \
--cc=vdavydov.dev@gmail.com \
--cc=willy@infradead.org \
--cc=ying.huang@intel.com \
--cc=yuzhao@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).