[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e1d44e36-06e4-4d1c-8daf-315d149ea1b3@arm.com>
Date: Tue, 6 Aug 2024 10:46:00 +0100
From: Ryan Roberts <ryan.roberts@....com>
To: David Hildenbrand <david@...hat.com>, linux-kernel@...r.kernel.org
Cc: linux-mm@...ck.org, linux-doc@...r.kernel.org, kvm@...r.kernel.org,
linux-s390@...r.kernel.org, linux-fsdevel@...r.kernel.org,
Andrew Morton <akpm@...ux-foundation.org>,
"Matthew Wilcox (Oracle)" <willy@...radead.org>,
Jonathan Corbet <corbet@....net>,
Christian Borntraeger <borntraeger@...ux.ibm.com>,
Janosch Frank <frankja@...ux.ibm.com>,
Claudio Imbrenda <imbrenda@...ux.ibm.com>, Heiko Carstens
<hca@...ux.ibm.com>, Vasily Gorbik <gor@...ux.ibm.com>,
Alexander Gordeev <agordeev@...ux.ibm.com>,
Sven Schnelle <svens@...ux.ibm.com>,
Gerald Schaefer <gerald.schaefer@...ux.ibm.com>,
Mark Brown <broonie@...nel.org>
Subject: Re: [PATCH v1 07/11] mm/huge_memory: convert split_huge_pages_pid()
from follow_page() to folio_walk
On 02/08/2024 16:55, David Hildenbrand wrote:
> Let's remove yet another follow_page() user. Note that we have to do the
> split without holding the PTL, after folio_walk_end(). We don't care
> about losing the secretmem check in follow_page().
Hi David,
Our (arm64) CI is showing a regression in split_huge_page_test from mm selftests from next-20240805 onwards. Navigating around a couple of other lurking bugs, I was able to bisect to this change (which smells about right).
Newly failing test:
# # ------------------------------
# # running ./split_huge_page_test
# # ------------------------------
# # TAP version 13
# # 1..12
# # Bail out! Still AnonHugePages not split
# # # Planned tests != run tests (12 != 0)
# # # Totals: pass:0 fail:0 xfail:0 xpass:0 skip:0 error:0
# # [FAIL]
# not ok 52 split_huge_page_test # exit=1
It's trying to split some pmd-mapped THPs then checking and finding that they are not split. The split is requested via /sys/kernel/debug/split_huge_pages, which I believe ends up in this function you are modifying here. Although I'll admit that looking at the change, there is nothing obviously wrong! Any ideas?
bisect log:
# bad: [1e391b34f6aa043c7afa40a2103163a0ef06d179] Add linux-next specific files for 20240806
git bisect bad 1e391b34f6aa043c7afa40a2103163a0ef06d179
# good: [de9c2c66ad8e787abec7c9d7eff4f8c3cdd28aed] Linux 6.11-rc2
git bisect good de9c2c66ad8e787abec7c9d7eff4f8c3cdd28aed
# bad: [01c2d56f2c52e8af01dfd91af1fe9affc76c4c9e] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git
git bisect bad 01c2d56f2c52e8af01dfd91af1fe9affc76c4c9e
# bad: [01c2d56f2c52e8af01dfd91af1fe9affc76c4c9e] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git
git bisect bad 01c2d56f2c52e8af01dfd91af1fe9affc76c4c9e
# bad: [3610638e967f32f02c56c7cc8f7d6a815972f8c2] Merge branch 'for-linux-next' of git://git.kernel.org/pub/scm/linux/kernel/git/sudeep.holla/linux.git
git bisect bad 3610638e967f32f02c56c7cc8f7d6a815972f8c2
# bad: [3610638e967f32f02c56c7cc8f7d6a815972f8c2] Merge branch 'for-linux-next' of git://git.kernel.org/pub/scm/linux/kernel/git/sudeep.holla/linux.git
git bisect bad 3610638e967f32f02c56c7cc8f7d6a815972f8c2
# bad: [d35ef6c9d106eedff36908c21699e1b7f3e55584] Merge branch 'clang-format' of https://github.com/ojeda/linux.git
git bisect bad d35ef6c9d106eedff36908c21699e1b7f3e55584
# good: [e1a15959d75c9ba4b45e07e37bcf843c85750010] Merge branch 'for-linux-next-fixes' of https://gitlab.freedesktop.org/drm/misc/kernel.git
git bisect good e1a15959d75c9ba4b45e07e37bcf843c85750010
# good: [6d66cb9bdeceb769ce62591f56580ebe80f6267a] mm: swap: add a adaptive full cluster cache reclaim
git bisect good 6d66cb9bdeceb769ce62591f56580ebe80f6267a
# bad: [2b820b576dfc4aa9b65f18b68f468cb5b38ece84] mm: optimization on page allocation when CMA enabled
git bisect bad 2b820b576dfc4aa9b65f18b68f468cb5b38ece84
# bad: [ab70279848c8623027791799492a3f6e7c38a9b2] MIPS: sgi-ip27: drop HAVE_ARCH_NODEDATA_EXTENSION
git bisect bad ab70279848c8623027791799492a3f6e7c38a9b2
# bad: [539bc09ff00b29eb60f3dc8ed2d82ad2050a582d] mm/huge_memory: convert split_huge_pages_pid() from follow_page() to folio_walk
git bisect bad 539bc09ff00b29eb60f3dc8ed2d82ad2050a582d
# good: [1a37544d0e35340ce740d377d7d6c746a84e2aae] include/linux/mmzone.h: clean up watermark accessors
git bisect good 1a37544d0e35340ce740d377d7d6c746a84e2aae
# good: [22adafb60d6e1a607a3d99da90927ddd7df928ad] mm/migrate: convert do_pages_stat_array() from follow_page() to folio_walk
git bisect good 22adafb60d6e1a607a3d99da90927ddd7df928ad
# good: [57e1ccf54dba4dda6d6f0264b76e2b86eec3d401] mm/ksm: convert get_mergeable_page() from follow_page() to folio_walk
git bisect good 57e1ccf54dba4dda6d6f0264b76e2b86eec3d401
# good: [285aa1a963f310530351b0e4a2e64bc4b806e518] mm/ksm: convert scan_get_next_rmap_item() from follow_page() to folio_walk
git bisect good 285aa1a963f310530351b0e4a2e64bc4b806e518
# first bad commit: [539bc09ff00b29eb60f3dc8ed2d82ad2050a582d] mm/huge_memory: convert split_huge_pages_pid() from follow_page() to folio_walk
Thanks,
Ryan
>
> Signed-off-by: David Hildenbrand <david@...hat.com>
> ---
> mm/huge_memory.c | 18 +++++++++++-------
> 1 file changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 0167dc27e365..697fcf89f975 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -40,6 +40,7 @@
> #include <linux/memory-tiers.h>
> #include <linux/compat.h>
> #include <linux/pgalloc_tag.h>
> +#include <linux/pagewalk.h>
>
> #include <asm/tlb.h>
> #include <asm/pgalloc.h>
> @@ -3507,7 +3508,7 @@ static int split_huge_pages_pid(int pid, unsigned long vaddr_start,
> */
> for (addr = vaddr_start; addr < vaddr_end; addr += PAGE_SIZE) {
> struct vm_area_struct *vma = vma_lookup(mm, addr);
> - struct page *page;
> + struct folio_walk fw;
> struct folio *folio;
>
> if (!vma)
> @@ -3519,13 +3520,10 @@ static int split_huge_pages_pid(int pid, unsigned long vaddr_start,
> continue;
> }
>
> - /* FOLL_DUMP to ignore special (like zero) pages */
> - page = follow_page(vma, addr, FOLL_GET | FOLL_DUMP);
> -
> - if (IS_ERR_OR_NULL(page))
> + folio = folio_walk_start(&fw, vma, addr, 0);
> + if (!folio)
> continue;
>
> - folio = page_folio(page);
> if (!is_transparent_hugepage(folio))
> goto next;
>
> @@ -3544,13 +3542,19 @@ static int split_huge_pages_pid(int pid, unsigned long vaddr_start,
>
> if (!folio_trylock(folio))
> goto next;
> + folio_get(folio);
> + folio_walk_end(&fw, vma);
>
> if (!split_folio_to_order(folio, new_order))
> split++;
>
> folio_unlock(folio);
> -next:
> folio_put(folio);
> +
> + cond_resched();
> + continue;
> +next:
> + folio_walk_end(&fw, vma);
> cond_resched();
> }
> mmap_read_unlock(mm);
Powered by blists - more mailing lists