From: Alex Shi <alex.shi@linux.alibaba.com>
To: Alexander Duyck <alexander.duyck@gmail.com>
Cc: yang.shi@linux.alibaba.com, lkp@intel.com, rong.a.chen@intel.com,
khlebnikov@yandex-team.ru, kirill@shutemov.name,
hughd@google.com, linux-kernel@vger.kernel.org,
daniel.m.jordan@oracle.com, linux-mm@kvack.org,
shakeelb@google.com, willy@infradead.org, hannes@cmpxchg.org,
tj@kernel.org, cgroups@vger.kernel.org,
akpm@linux-foundation.org, richard.weiyang@gmail.com,
mgorman@techsingularity.net, iamjoonsoo.kim@lge.com
Subject: Re: [RFC PATCH v2 5/5] mm: Split move_pages_to_lru into 3 separate passes
Date: Wed, 19 Aug 2020 15:56:43 +0800 [thread overview]
Message-ID: <084c58a7-7aac-820c-9606-19391c35b9b5@linux.alibaba.com> (raw)
In-Reply-To: <20200819042738.23414.60815.stgit@localhost.localdomain>
在 2020/8/19 下午12:27, Alexander Duyck 写道:
> From: Alexander Duyck <alexander.h.duyck@linux.intel.com>
>
> The current code for move_pages_to_lru is meant to release the LRU lock
> every time it encounters an unevictable page or a compound page that must
> be freed. This results in a fair amount of code bulk because the lruvec has
> to be reacquired every time the lock is released and reacquired.
>
> Instead of doing this I believe we can break the code up into 3 passes. The
> first pass will identify the pages we can move to LRU and move those. In
> addition it will sort the list out leaving the unevictable pages in the
> list and moving those pages that have dropped to a reference count of 0 to
> pages_to_free. The second pass will return the unevictable pages to the
> LRU. The final pass will free any compound pages we have in the
> pages_to_free list before we merge it back with the original list and
> return from the function.
>
> The advantage of doing it this way is that we only have to release the lock
> between pass 1 and 2, and then we reacquire the lock after pass 3 after we
> merge the pages_to_free back into the original list. As such we only have
> to release the lock at most once in an entire call instead of having to
> test to see if we need to relock with each page.
>
> Signed-off-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> ---
> mm/vmscan.c | 68 ++++++++++++++++++++++++++++++++++-------------------------
> 1 file changed, 39 insertions(+), 29 deletions(-)
>
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 3ebe3f9b653b..6a2bdbc1a9eb 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1850,22 +1850,21 @@ static unsigned noinline_for_stack move_pages_to_lru(struct lruvec *lruvec,
> {
> int nr_pages, nr_moved = 0;
> LIST_HEAD(pages_to_free);
> - struct page *page;
> - struct lruvec *orig_lruvec = lruvec;
> + struct page *page, *next;
> enum lru_list lru;
>
> - while (!list_empty(list)) {
> - page = lru_to_page(list);
> + list_for_each_entry_safe(page, next, list, lru) {
> VM_BUG_ON_PAGE(PageLRU(page), page);
> - list_del(&page->lru);
> - if (unlikely(!page_evictable(page))) {
> - if (lruvec) {
> - spin_unlock_irq(&lruvec->lru_lock);
> - lruvec = NULL;
> - }
> - putback_lru_page(page);
> +
> + /*
> + * if page is unevictable leave it on the list to be returned
> + * to the LRU after we have finished processing the other
> + * entries in the list.
> + */
> + if (unlikely(!page_evictable(page)))
> continue;
> - }
> +
> + list_del(&page->lru);
>
> /*
> * The SetPageLRU needs to be kept here for list intergrity.
> @@ -1878,20 +1877,14 @@ static unsigned noinline_for_stack move_pages_to_lru(struct lruvec *lruvec,
> * list_add(&page->lru,)
> * list_add(&page->lru,)
> */
> - lruvec = relock_page_lruvec_irq(page, lruvec);
It's actually changed the meaning from current func. which I had seen a bug if no relock.
but after move to 5.9 kernel, I can not reprodce the bug any more. I am not sure if 5.9 fixed
the problem, and we don't need relock here.
For the rest of this patch.
Reviewed-by: Alex Shi <alex.shi@linux.alibaba.com>
> SetPageLRU(page);
>
> if (unlikely(put_page_testzero(page))) {
> __ClearPageLRU(page);
> __ClearPageActive(page);
>
> - if (unlikely(PageCompound(page))) {
> - spin_unlock_irq(&lruvec->lru_lock);
> - lruvec = NULL;
> - destroy_compound_page(page);
> - } else
> - list_add(&page->lru, &pages_to_free);
> -
> + /* defer freeing until we can release lru_lock */
> + list_add(&page->lru, &pages_to_free);
> continue;
> }
>
> @@ -1904,16 +1897,33 @@ static unsigned noinline_for_stack move_pages_to_lru(struct lruvec *lruvec,
> if (PageActive(page))
> workingset_age_nonresident(lruvec, nr_pages);
> }
> - if (orig_lruvec != lruvec) {
> - if (lruvec)
> - spin_unlock_irq(&lruvec->lru_lock);
> - spin_lock_irq(&orig_lruvec->lru_lock);
> - }
>
> - /*
> - * To save our caller's stack, now use input list for pages to free.
> - */
> - list_splice(&pages_to_free, list);
> + if (unlikely(!list_empty(list) || !list_empty(&pages_to_free))) {
> + spin_unlock_irq(&lruvec->lru_lock);
> +
> + /* return any unevictable pages to the LRU list */
> + while (!list_empty(list)) {
> + page = lru_to_page(list);
> + list_del(&page->lru);
> + putback_lru_page(page);
> + }
> +
> + /*
> + * To save our caller's stack use input
> + * list for pages to free.
> + */
> + list_splice(&pages_to_free, list);
> +
> + /* free any compound pages we have in the list */
> + list_for_each_entry_safe(page, next, list, lru) {
> + if (likely(!PageCompound(page)))
> + continue;
> + list_del(&page->lru);
> + destroy_compound_page(page);
> + }
> +
> + spin_lock_irq(&lruvec->lru_lock);
> + }
>
> return nr_moved;
> }
>
next prev parent reply other threads:[~2020-08-19 7:58 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-19 4:26 [RFC PATCH v2 0/5] Minor cleanups and performance optimizations for LRU rework Alexander Duyck
2020-08-19 4:27 ` [RFC PATCH v2 1/5] mm: Identify compound pages sooner in isolate_migratepages_block Alexander Duyck
2020-08-19 7:48 ` Alex Shi
2020-08-19 11:43 ` Matthew Wilcox
2020-08-19 14:48 ` Alexander Duyck
2020-08-19 4:27 ` [RFC PATCH v2 2/5] mm: Drop use of test_and_set_skip in favor of just setting skip Alexander Duyck
2020-08-19 7:50 ` Alex Shi
2020-08-19 4:27 ` [RFC PATCH v2 3/5] mm: Add explicit page decrement in exception path for isolate_lru_pages Alexander Duyck
2020-08-19 7:50 ` Alex Shi
2020-08-19 14:52 ` Alexander Duyck
2020-08-19 4:27 ` [RFC PATCH v2 4/5] mm: Split release_pages work into 3 passes Alexander Duyck
2020-08-19 7:53 ` Alex Shi
2020-08-19 14:57 ` Alexander Duyck
2020-08-20 9:49 ` Alex Shi
2020-08-20 14:13 ` Alexander Duyck
2020-08-19 4:27 ` [RFC PATCH v2 5/5] mm: Split move_pages_to_lru into 3 separate passes Alexander Duyck
2020-08-19 7:56 ` Alex Shi [this message]
2020-08-19 14:42 ` Alexander Duyck
2020-08-20 9:56 ` Alex Shi
2020-08-20 17:15 ` Alexander Duyck
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=084c58a7-7aac-820c-9606-19391c35b9b5@linux.alibaba.com \
--to=alex.shi@linux.alibaba.com \
--cc=akpm@linux-foundation.org \
--cc=alexander.duyck@gmail.com \
--cc=cgroups@vger.kernel.org \
--cc=daniel.m.jordan@oracle.com \
--cc=hannes@cmpxchg.org \
--cc=hughd@google.com \
--cc=iamjoonsoo.kim@lge.com \
--cc=khlebnikov@yandex-team.ru \
--cc=kirill@shutemov.name \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=lkp@intel.com \
--cc=mgorman@techsingularity.net \
--cc=richard.weiyang@gmail.com \
--cc=rong.a.chen@intel.com \
--cc=shakeelb@google.com \
--cc=tj@kernel.org \
--cc=willy@infradead.org \
--cc=yang.shi@linux.alibaba.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).