* Re: [PATCH v8 2/6] mm/memory-failure: surface unhandlable kernel pages as -ENOTRECOVERABLE
From: David Hildenbrand (Arm) @ 2026-06-09 7:09 UTC (permalink / raw)
To: Miaohe Lin, Breno Leitao
Cc: linux-mm, linux-kernel, linux-doc, linux-kselftest,
linux-trace-kernel, kernel-team, Lance Yang, Andrew Morton,
Lorenzo Stoakes, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Shuah Khan, Naoya Horiguchi,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Jonathan Corbet, Shuah Khan, Liam R. Howlett
In-Reply-To: <4953bcee-5a0f-2bc5-7295-63e5e7513e8b@huawei.com>
On 6/9/26 04:39, Miaohe Lin wrote:
> On 2026/6/8 22:15, Breno Leitao wrote:
>> On Fri, Jun 05, 2026 at 11:42:53AM +0200, David Hildenbrand (Arm) wrote:
>>>
>>> I mean, any such races can currently already happen one way or the other?
>>>
>>> Really, the only way to not get races is to tryget the (compound)page,
>>> revalidate that the page is still part of the compound page.
>>>
>>> I'm not sure if that's really a good idea.
>>>
>>> But my memory is a bit vague in which scenarios we already hold a page reference
>>> here to prevent any concurrent freeing?
>>
>> No, we don't hold one here in the case that matters.
>>
>> HWPoisonKernelOwned() runs at the very top of get_any_page(), before
>> try_again: and before __get_hwpoison_page(). The first refcount taken in
>> the whole path is the folio_try_get() inside __get_hwpoison_page(), which
>> runs *after* the short-circuit.
>>
>> So get_any_page() itself never holds a reference at the check -- the only way
>> one exists is if the caller passed MF_COUNT_INCREASED (count_increased ==
>> true).
>>
>> So on the MCE/GHES path -- the one this panic option exists for -- no
>> reference is held when HWPoisonKernelOwned() does its compound_head() +
>> PageSlab()/PageTable()/PageLargeKmalloc() checks.
>>
>> Given that, I'd rather keep it racy and take no refcount than add a
>> tryget + revalidate purely for this check. As I've said earleir, an operator
>
> Would it be acceptable to add a simple recheck? Something like below:
>
> retry:
> head = compound_head(page);
> PageSlab()/PageTable()/PageLargeKmalloc() checks
> if (head != compound_head(page))
> goto retry
Sure. I guess it could still be racy in some weird scenarios where we
free+allocate+free in-between.
--
Cheers,
David
^ permalink raw reply
* Re: [PATCH v2] rethook: Remove the running task check in rethook_find_ret_addr()
From: Peter Zijlstra @ 2026-06-09 7:14 UTC (permalink / raw)
To: Tengda Wu
Cc: Masami Hiramatsu, Steven Rostedt, Mathieu Desnoyers,
Alexei Starovoitov, linux-trace-kernel, linux-kernel
In-Reply-To: <20260609005728.458962-1-wutengda@huaweicloud.com>
On Tue, Jun 09, 2026 at 08:57:28AM +0800, Tengda Wu wrote:
> The current check in rethook_find_ret_addr() prevents obtaining a return
> address when the target task is marked as running. However, this condition
> is both insufficient for safety and unnecessary for its intended purpose.
Depends on what safety means. If safety means not crashing, it is
entirely superfluous. If safety means correctness, then yes, it is
insufficient.
> The check is inherently racy: a task can begin running on another CPU
> immediately after task_is_running() returns false, potentially leading to
> concurrent modification of rethook data structures while the iteration is
> in progress.
>
> Rather than attempting to fix this unreliable check deep in the unwinding
> path, remove it entirely. Callers that require consistency are expected
> to provide a safe context.
Perhaps also note that unwind_next() will hold RCU and the rethook_node
things are RCU freed, so while the iteration might go off the rails and
return invalid information, it will not crash.
> Fixes: 54ecbe6f1ed5 ("rethook: Add a generic return hook")
> Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>
With clarifications:
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
> v2: Remove the running task check.
> v1: https://lore.kernel.org/all/20260525132253.1889726-1-wutengda@huaweicloud.com/
>
> kernel/trace/rethook.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/kernel/trace/rethook.c b/kernel/trace/rethook.c
> index 5a8bdf88999a..f70f11bc6c91 100644
> --- a/kernel/trace/rethook.c
> +++ b/kernel/trace/rethook.c
> @@ -250,9 +250,6 @@ unsigned long rethook_find_ret_addr(struct task_struct *tsk, unsigned long frame
> if (WARN_ON_ONCE(!cur))
> return 0;
>
> - if (tsk != current && task_is_running(tsk))
> - return 0;
> -
> do {
> ret = __rethook_find_ret_addr(tsk, cur);
> if (!ret)
> --
> 2.34.1
>
^ permalink raw reply
* Re: [PATCH v3 1/8] scripts/sorttable: Handle RISC-V patchable ftrace entries
From: Martin Kaiser @ 2026-06-09 7:27 UTC (permalink / raw)
To: Wang Han
Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Steven Rostedt,
Alexandre Ghiti, Masami Hiramatsu, Mark Rutland, Catalin Marinas,
Chen Pei, Andy Chiu, Björn Töpel, Deepak Gupta,
Puranjay Mohan, Conor Dooley, Josh Poimboeuf, Jiri Kosina,
Miroslav Benes, Petr Mladek, Joe Lawrence, Shuah Khan,
Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, oliver.yang, xueshuai, zhuo.song, jkchen,
linux-riscv, linux-kernel, linux-trace-kernel, live-patching,
linux-kselftest, linux-perf-users
In-Reply-To: <20260609063002.3943001-1-wanghan@linux.alibaba.com>
Thus wrote Wang Han (wanghan@linux.alibaba.com):
> RISC-V uses -fpatchable-function-entry=8,4 when the compressed ISA is
> enabled and -fpatchable-function-entry=4,2 otherwise. In both cases, the
> patchable NOP area starts 8 bytes before the function symbol address.
> The __mcount_loc entries therefore point at the patchable NOP area
> associated with a function, while nm reports the function symbol at the
> entry address used for the function range check.
> After RISC-V selected HAVE_BUILDTIME_MCOUNT_SORT, sorttable started
> applying that range check at build time. Without allowing entries just
> before the reported function address, the mcount sorter treats valid
> RISC-V ftrace callsites as invalid weak-function entries and writes
> them back as zero. The resulting kernel boots with no ftrace entries,
> breaking dynamic ftrace and users such as livepatch.
> The failure is silent during the final link because zeroing weak-function
> entries is an expected sorttable operation. At boot, those zero entries
> are skipped by ftrace_process_locs(), so the only obvious symptom is that
> the vmlinux ftrace table has lost valid callsites and ftrace users cannot
> attach to them.
> CONFIG_FTRACE_SORT_STARTUP_TEST also reports the table as sorted in this
> state: it only checks that the __mcount_loc entries are in ascending
> order, which a fully zeroed table trivially satisfies. The original
> commit relied on this check and did not see the regression.
> On an affected RISC-V QEMU boot with both CONFIG_FTRACE_SORT_STARTUP_TEST
> and CONFIG_FTRACE_STARTUP_TEST enabled, the sort check still passes
> while ftrace reports zero usable entries and the early selftests fail:
> [ 0.000000] ftrace section at ffffffff8101da98 sorted properly
> [ 0.000000] ftrace: allocating 0 entries in 128 pages
> [ 0.054999] Testing tracer function: .. no entries found ..FAILED!
> [ 0.172407] tracer: function failed selftest, disabling
> [ 0.178186] Failed to init function_graph tracer, init returned -19
> Handle RISC-V like arm64 for the function-range check and allow
> patchable entries up to 8 bytes before the function address.
> With this fix, a RISC-V QEMU smoke boot with ftrace startup tests shows
> the vmlinux ftrace table is populated and dynamic ftrace still works:
> [ 0.000000] ftrace: allocating 46749 entries in 184 pages
> [ 0.051115] Testing tracer function: PASSED
> [ 1.283782] Testing dynamic ftrace: PASSED
> [ 6.275456] Testing tracer function_graph: PASSED
> Fixes: 0ca1724b56af ("riscv: ftrace: select HAVE_BUILDTIME_MCOUNT_SORT")
> Suggested-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> Reviewed-by: Steven Rostedt <rostedt@goodmis.org>
> Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>
> Reviewed-by: Chen Pei <cp0613@linux.alibaba.com>
> Link: https://lore.kernel.org/all/20260527113028.4b21a5de@fedora/
> Signed-off-by: Wang Han <wanghan@linux.alibaba.com>
> ---
> scripts/sorttable.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
> diff --git a/scripts/sorttable.c b/scripts/sorttable.c
> index e8ed11c680c6..d8dc2a1b7c31 100644
> --- a/scripts/sorttable.c
> +++ b/scripts/sorttable.c
> @@ -891,17 +891,22 @@ static int do_file(char const *const fname, void *addr)
> table_sort_t custom_sort = NULL;
> switch (elf_map_machine(ehdr)) {
> - case EM_AARCH64:
> #ifdef MCOUNT_SORT_ENABLED
> + case EM_AARCH64:
> + /* arm64 also needs RELA-based weak-function fixups. */
> sort_reloc = true;
> rela_type = 0x403;
> - /* arm64 uses patchable function entry placing before function */
> + /* fallthrough */
> + case EM_RISCV:
> + /* arm64 and RISC-V place patchable entries before the function. */
> before_func = 8;
> +#else
> + case EM_AARCH64:
> + case EM_RISCV:
> #endif
> /* fallthrough */
> case EM_386:
> case EM_LOONGARCH:
> - case EM_RISCV:
> case EM_S390:
> case EM_X86_64:
> custom_sort = sort_relative_table_with_data;
> --
> 2.43.0
I ran into this problem and came up with pretty much the same fix.
Reviewed-by: Martin Kaiser <martin@kaiser.cx>
^ permalink raw reply
* Re: [PATCH] mm/lruvec: trace LRU add drains and drain-all queuing
From: Barry Song @ 2026-06-09 7:44 UTC (permalink / raw)
To: JP Kobryn
Cc: linux-mm, willy, shakeel.butt, usama.arif, akpm, vbabka, mhocko,
rostedt, mhiramat, mathieu.desnoyers, kasong, qi.zheng,
axelrasmussen, yuanchu, weixugc, chrisl, shikemeng, nphamcs,
baoquan.he, youngjun.park, linux-kernel, linux-trace-kernel
In-Reply-To: <20260609041156.31127-1-jp.kobryn@linux.dev>
On Tue, Jun 9, 2026 at 12:12 PM JP Kobryn <jp.kobryn@linux.dev> wrote:
>
> LRU add batches can be drained before they reach capacity. This can be a
> source of LRU lock contention, but it is not currently possible to
> attribute these drains to callers with existing tracepoints.
>
> Add mm_lru_add_drain to report the CPU and lru_add batch count when an
> lru_add batch is drained. This allows tracing to distinguish full drains
> from partial drains and attribute them to the calling stack.
>
> Add mm_lru_drain_all_queue to report when lru_add_drain_all() queues
> per-CPU drain work. This captures the requester stack and target CPU for
> remote drain work. The event is named as a drain-all queue event because
> the queued work can be needed for batches other than lru_add.
>
> Signed-off-by: JP Kobryn <jp.kobryn@linux.dev>
> ---
> include/trace/events/pagemap.h | 40 ++++++++++++++++++++++++++++++++++
> mm/swap.c | 6 ++++-
> 2 files changed, 45 insertions(+), 1 deletion(-)
>
> diff --git a/include/trace/events/pagemap.h b/include/trace/events/pagemap.h
> index 171524d3526d..ea8fc46bedb0 100644
> --- a/include/trace/events/pagemap.h
> +++ b/include/trace/events/pagemap.h
> @@ -77,6 +77,46 @@ TRACE_EVENT(mm_lru_activate,
> TP_printk("folio=%p pfn=0x%lx", __entry->folio, __entry->pfn)
> );
>
> +TRACE_EVENT(mm_lru_add_drain,
> +
> + TP_PROTO(int cpu, unsigned int nr),
> +
> + TP_ARGS(cpu, nr),
> +
> + TP_STRUCT__entry(
> + __field(int, cpu )
> + __field(unsigned int, nr )
> + ),
> +
> + TP_fast_assign(
> + __entry->cpu = cpu;
> + __entry->nr = nr;
> + ),
> +
> + TP_printk("cpu=%d nr=%u", __entry->cpu, __entry->nr)
> +);
> +
> +TRACE_EVENT(mm_lru_drain_all_queue,
> +
> + TP_PROTO(int target_cpu, bool force_all_cpus),
> +
> + TP_ARGS(target_cpu, force_all_cpus),
> +
> + TP_STRUCT__entry(
> + __field(int, target_cpu )
> + __field(bool, force_all_cpus )
> + ),
> +
> + TP_fast_assign(
> + __entry->target_cpu = target_cpu;
> + __entry->force_all_cpus = force_all_cpus;
> + ),
> +
> + TP_printk("target_cpu=%d force_all_cpus=%s",
> + __entry->target_cpu,
> + __entry->force_all_cpus ? "true" : "false")
> +);
> +
> #endif /* _TRACE_PAGEMAP_H */
>
> /* This part must be outside protection */
> diff --git a/mm/swap.c b/mm/swap.c
> index 588f50d8f1a8..c385b93582eb 100644
> --- a/mm/swap.c
> +++ b/mm/swap.c
> @@ -694,9 +694,12 @@ void lru_add_drain_cpu(int cpu)
> {
> struct cpu_fbatches *fbatches = &per_cpu(cpu_fbatches, cpu);
> struct folio_batch *fbatch = &fbatches->lru_add;
> + unsigned int nr_folios_add = folio_batch_count(fbatch);
>
> - if (folio_batch_count(fbatch))
> + if (nr_folios_add) {
> folio_batch_move_lru(fbatch, lru_add);
> + trace_mm_lru_add_drain(cpu, nr_folios_add);
> + }
>
> fbatch = &fbatches->lru_move_tail;
> /* Disabling interrupts below acts as a compiler barrier. */
> @@ -928,6 +931,7 @@ static inline void __lru_add_drain_all(bool force_all_cpus)
> if (cpu_needs_drain(cpu)) {
> INIT_WORK(work, lru_add_drain_per_cpu);
> queue_work_on(cpu, mm_percpu_wq, work);
> + trace_mm_lru_drain_all_queue(cpu, force_all_cpus);
Do you need tracing on each CPU individually, or is tracing the
entire __lru_add_drain_all() invocation sufficient?
Do you also need this_gen and lru_drain_gen to be traced?
By the way, I'm not sure drain_all_queue is the best name here.
Why not simply use add_drain_all()? It would match the existing
function name better.
Best Regards
Barry
^ permalink raw reply
* [PATCH v3] rethook: Remove the running task check in rethook_find_ret_addr()
From: Tengda Wu @ 2026-06-09 8:49 UTC (permalink / raw)
To: Masami Hiramatsu, Peter Zijlstra
Cc: Steven Rostedt, Mathieu Desnoyers, Alexei Starovoitov,
linux-trace-kernel, linux-kernel, Tengda Wu
The current check in rethook_find_ret_addr() prevents obtaining a return
address when the target task is marked as running. However, this condition
is both insufficient for correctness and unnecessary for its intended
purpose.
The check is inherently racy: a task can begin running on another CPU
immediately after task_is_running() returns false, potentially leading to
concurrent modification of rethook data structures while the iteration is
in progress.
Rather than trying to fix this unreliable check deep in the unwinding
path, simply remove it. The iteration is already safe from crashes because
unwind_next_frame() holds RCU and rethook_node structures are RCU-freed;
even if the iteration goes off the rails and returns invalid information,
it will not crash. Callers that require consistency must provide a safe
context themselves.
Fixes: 54ecbe6f1ed5 ("rethook: Add a generic return hook")
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>
---
v3: Improve commit message: clarify safety semantics and document that RCU guarantees no crash.
v2: https://lore.kernel.org/all/20260609005728.458962-1-wutengda@huaweicloud.com/
v1: https://lore.kernel.org/all/20260525132253.1889726-1-wutengda@huaweicloud.com/
kernel/trace/rethook.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/kernel/trace/rethook.c b/kernel/trace/rethook.c
index 5a8bdf88999a..f70f11bc6c91 100644
--- a/kernel/trace/rethook.c
+++ b/kernel/trace/rethook.c
@@ -250,9 +250,6 @@ unsigned long rethook_find_ret_addr(struct task_struct *tsk, unsigned long frame
if (WARN_ON_ONCE(!cur))
return 0;
- if (tsk != current && task_is_running(tsk))
- return 0;
-
do {
ret = __rethook_find_ret_addr(tsk, cur);
if (!ret)
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v2] rethook: Remove the running task check in rethook_find_ret_addr()
From: Tengda Wu @ 2026-06-09 8:53 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Masami Hiramatsu, Steven Rostedt, Mathieu Desnoyers,
Alexei Starovoitov, linux-trace-kernel, linux-kernel
In-Reply-To: <20260609071412.GG3102624@noisy.programming.kicks-ass.net>
On 2026/6/9 15:14, Peter Zijlstra wrote:
> On Tue, Jun 09, 2026 at 08:57:28AM +0800, Tengda Wu wrote:
>> The current check in rethook_find_ret_addr() prevents obtaining a return
>> address when the target task is marked as running. However, this condition
>> is both insufficient for safety and unnecessary for its intended purpose.
>
> Depends on what safety means. If safety means not crashing, it is
> entirely superfluous. If safety means correctness, then yes, it is
> insufficient.
>
>> The check is inherently racy: a task can begin running on another CPU
>> immediately after task_is_running() returns false, potentially leading to
>> concurrent modification of rethook data structures while the iteration is
>> in progress.
>>
>> Rather than attempting to fix this unreliable check deep in the unwinding
>> path, remove it entirely. Callers that require consistency are expected
>> to provide a safe context.
>
> Perhaps also note that unwind_next() will hold RCU and the rethook_node
> things are RCU freed, so while the iteration might go off the rails and
> return invalid information, it will not crash.
>
>
>> Fixes: 54ecbe6f1ed5 ("rethook: Add a generic return hook")
>> Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>
>
> With clarifications:
>
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
>
Thank you for the review and suggestions, Peter.
I have incorporated your feedback into v3. The patch has been sent out.
https://lore.kernel.org/all/20260609084953.901576-1-wutengda@huaweicloud.com/
Best regards,
Tengda
^ permalink raw reply
* Re: [PATCH mm-unstable v19 11/14] mm/khugepaged: Introduce mTHP collapse support
From: Nico Pache @ 2026-06-09 9:01 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: linux-doc, linux-kernel, linux-mm, linux-trace-kernel, aarcange,
akpm, anshuman.khandual, apopple, baohua, baolin.wang, byungchul,
catalin.marinas, cl, corbet, dave.hansen, david, dev.jain, gourry,
hannes, hughd, jack, jackmanb, jannh, jglisse, joshua.hahnjy, kas,
lance.yang, liam, mathieu.desnoyers, matthew.brost, mhiramat,
mhocko, peterx, pfalcato, rakie.kim, raquini, rdunlap,
richard.weiyang, rientjes, rostedt, rppt, ryan.roberts, shivankg,
sunnanyong, surenb, thomas.hellstrom, tiwai, usamaarif642, vbabka,
vishal.moola, wangkefeng.wang, will, willy, yang, ying.huang, ziy,
zokeefe
In-Reply-To: <aiMTuXKQ5qxKYo60@lucifer>
On Fri, Jun 5, 2026 at 12:38 PM Lorenzo Stoakes <ljs@kernel.org> wrote:
>
> On Fri, Jun 05, 2026 at 10:14:18AM -0600, Nico Pache wrote:
> > Enable khugepaged to collapse to mTHP orders. This patch implements the
> > main scanning logic using a bitmap to track occupied pages and the
> > algorithm to find optimal collapse sizes.
> >
> > Previous to this patch, PMD collapse had 3 main phases, a light weight
> > scanning phase (mmap_read_lock) that determines a potential PMD
> > collapse, an alloc phase (mmap unlocked), then finally heavier collapse
> > phase (mmap_write_lock).
> >
> > To enabled mTHP collapse we make the following changes:
> >
> > During PMD scan phase, track occupied pages in a bitmap. When mTHP
> > orders are enabled, we remove the restriction of max_ptes_none during the
> > scan phase to avoid missing potential mTHP collapse candidates. Once we
> > have scanned the full PMD range and updated the bitmap to track occupied
> > pages, we use the bitmap to find the optimal mTHP size.
> >
> > Implement mthp_collapse() to walk forward through the bitmap and
> > determine the best eligible order for each naturally-aligned region. The
> > algorithm starts at the beginning of the PMD range and, for each offset,
> > tries the highest order that fits the alignment. If the number of
> > occupied PTEs in that region satisfies the max_ptes_none threshold for
> > that order, a collapse is attempted. On failure, the order is
> > decremented and the same offset is retried at the next smaller size. Once
> > the smallest enabled order is exhausted (or a collapse succeeds), the
> > offset advances past the region just processed, and the next attempt
> > starts at the highest order permitted by the new offset's natural
> > alignment.
>
> I think still it might have been nice to discuss why we are not
> e.g. greedily trying to find the biggest possible mTHP size (if we did, we
> would try the highest offset first), but we can save that for adding some
> documentation somewhere later tbh.
We are, the algorithm tries PMD, then order 8, then order 7, and so
on. Due to the required alignment, if the N-1 order succeeds, we try
the same order at the neighboring offset.
So if we collapse a order 8, the following collapse attempt will be
order 8 at 256. We always try the highest order allowed for a given
offset :)
>
> This commit message is long enough as it is :>)
>
> >
> > The algorithm works as follows:
> > 1) set offset=0 and order=HPAGE_PMD_ORDER
> > 2) if the order is not enabled, go to step (5)
> > 3) count occupied PTEs in the (offset, order) range using
> > bitmap_weight_from()
> > 4) if the count satisfies the max_ptes_none threshold, attempt
> > collapse; on success, advance to step (6)
> > 5) if a smaller enabled order exists, decrement order and retry
> > from step (2) at the same offset
> > 6) advance offset past the current region and compute the next
> > order from the new offset's natural alignment via __ffs(offset),
> > capped at HPAGE_PMD_ORDER
> > 7) repeat from step (2) until the full PMD range is covered
> >
> > mTHP collapses reject regions containing swapped out or shared pages.
> > This is because adding new entries can lead to new none pages, and these
> > may lead to constant promotion into a higher order mTHP. A similar
> > issue can occur with "max_ptes_none > HPAGE_PMD_NR/2" due to a collapse
> > introducing at least 2x the number of pages, and on a future scan will
> > satisfy the promotion condition once again. This issue is prevented via
> > the collapse_max_ptes_none() function which imposes the max_ptes_none
> > restrictions above.
> >
> > We currently only support mTHP collapse for max_ptes_none values of 0
> > and HPAGE_PMD_NR - 1. resulting in the following behavior:
> >
> > - max_ptes_none=0: Never introduce new empty pages during collapse
> > - max_ptes_none=HPAGE_PMD_NR-1: Always try collapse to the highest
> > available mTHP order
> >
> > Any other max_ptes_none value will emit a warning and default mTHP
> > collapse to max_ptes_none=0. There should be no behavior change for PMD
> > collapse.
> >
> > Once we determine what mTHP sizes fits best in that PMD range a collapse
> > is attempted. A minimum collapse order of 2 is used as this is the lowest
> > order supported by anon memory as defined by THP_ORDERS_ALL_ANON.
> >
> > Currently madv_collapse is not supported and will only attempt PMD
> > collapse.
> >
> > We can also remove the check for is_khugepaged inside the PMD scan as
> > the collapse_max_ptes_none() function handles this logic now.
>
> It'd be nice to have kept the ASCII diagram here too :'( but this is fine,
>
> >
> > Signed-off-by: Nico Pache <npache@redhat.com>
>
> This all LGTM, and we can fix up any issues that arise later if anything
> does break. So:
>
> Reviewed-by: Lorenzo Stoakes <ljs@kernel.org>
Thanks for reviewing :)
>
> > ---
> > mm/khugepaged.c | 146 +++++++++++++++++++++++++++++++++++++++++++++---
> > 1 file changed, 138 insertions(+), 8 deletions(-)
> >
> > diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> > index ec886a031952..430047316f43 100644
> > --- a/mm/khugepaged.c
> > +++ b/mm/khugepaged.c
> > @@ -99,6 +99,8 @@ static DEFINE_READ_MOSTLY_HASHTABLE(mm_slots_hash, MM_SLOTS_HASH_BITS);
> >
> > static struct kmem_cache *mm_slot_cache __ro_after_init;
> >
> > +#define KHUGEPAGED_MIN_MTHP_ORDER 2
> > +
> > struct collapse_control {
> > bool is_khugepaged;
> >
> > @@ -110,6 +112,9 @@ struct collapse_control {
> >
> > /* nodemask for allocation fallback */
> > nodemask_t alloc_nmask;
> > +
> > + /* Each bit represents a single occupied (!none/zero) page. */
> > + DECLARE_BITMAP(mthp_present_ptes, MAX_PTRS_PER_PTE);
> > };
> >
> > /**
> > @@ -1440,20 +1445,130 @@ static enum scan_result collapse_huge_page(struct mm_struct *mm, unsigned long s
> > return result;
> > }
> >
> > +/* Return the highest naturally aligned order that fits at @offset within a PMD. */
> > +static unsigned int max_order_from_offset(unsigned int offset)
> > +{
> > + if (offset == 0)
> > + return HPAGE_PMD_ORDER;
> > +
> > + return min_t(unsigned int, __ffs(offset), HPAGE_PMD_ORDER);
> > +}
>
> Thanks this is better! I wonder if we can ever actually see an
> __ffs(offset) that's > HPAGE_PMD_ORDER but probably better safe than sorry
> here with the min_t.
I don't think so unless offset somehow exceeds 512 (it shouldn't), but
like you said, better safe than sorry.
>
> > +
> > +/*
> > + * mthp_collapse() consumes the bitmap that is generated during
> > + * collapse_scan_pmd() to determine what regions and mTHP orders fit best.
> > + *
> > + * Each bit in cc->mthp_present_ptes represents a single occupied (!none/zero)
> > + * page. We start at the PMD order and check if it is eligible for collapse;
> > + * if not, we check the left and right halves of the PTE page table we are
> > + * examining at a lower order.
> > + *
> > + * For each of these, we determine how many PTE entries are occupied in the
> > + * range of PTE entries we propose to collapse, then we compare this to a
> > + * threshold number of PTE entries which would need to be occupied for a
> > + * collapse to be permitted at that order (accounting for max_ptes_none).
> > + *
> > + * If a collapse is permitted, we attempt to collapse the PTE range into a
> > + * mTHP.
> > + */
> > +static enum scan_result mthp_collapse(struct mm_struct *mm,
> > + unsigned long address, int referenced, int unmapped,
> > + struct collapse_control *cc, unsigned long enabled_orders)
> > +{
> > + unsigned int nr_occupied_ptes, nr_ptes, max_ptes_none;
> > + enum scan_result last_result = SCAN_FAIL;
> > + int collapsed = 0;
> > + bool alloc_failed = false;
> > + unsigned long collapse_address;
> > + unsigned int offset = 0;
> > + unsigned int order = HPAGE_PMD_ORDER;
> > +
> > + while (offset < HPAGE_PMD_NR) {
> > + nr_ptes = 1UL << order;
> > +
> > + if (!test_bit(order, &enabled_orders))
> > + goto next_order;
> > +
> > + max_ptes_none = collapse_max_ptes_none(cc, NULL, order);
> > + nr_occupied_ptes = bitmap_weight_from(cc->mthp_present_ptes, offset,
> > + offset + nr_ptes);
> > +
> > + if (nr_occupied_ptes >= nr_ptes - max_ptes_none) {
> > + enum scan_result ret;
> > +
> > + collapse_address = address + offset * PAGE_SIZE;
> > + ret = collapse_huge_page(mm, collapse_address, referenced,
> > + unmapped, cc, order);
> > + switch (ret) {
> > + /* Cases where we continue to next collapse candidate */
> > + case SCAN_SUCCEED:
> > + collapsed += nr_ptes;
> > + fallthrough;
> > + case SCAN_PTE_MAPPED_HUGEPAGE:
> > + goto next_offset;
> > + /* Cases where lower orders might still succeed */
> > + case SCAN_ALLOC_HUGE_PAGE_FAIL:
> > + alloc_failed = true;
> > + last_result = ret;
> > + goto next_order;
> > + /* Cases where no further collapse is possible */
> > + case SCAN_PMD_MAPPED:
> > + fallthrough;
> > + default:
> > + last_result = ret;
> > + goto done;
> > + }
> > + }
> > +
> > +next_order:
> > + /*
> > + * Continue with the next smaller order if there is still
> > + * any smaller order enabled. When at the smallest order
> > + * we must always move to the next offset.
> > + */
> > + if (order > KHUGEPAGED_MIN_MTHP_ORDER &&
> > + (enabled_orders & GENMASK(order - 1, 0))) {
>
> Honestly wasn't aware of GENMASK() before :)
I wasn't either! (thanks David ;) )
>
> > + order--;
> > + continue;
> > + }
> > +next_offset:
> > + /*
> > + * Advance past the region we just processed and determine the
> > + * highest order we can attempt next. Since huge pages must be
> > + * naturally aligned, the max order we can attempt next is
> > + * limited by the alignment of the new offset.
> > + * E.g. if we collapsed a order-2 mTHP at offset 0, offset
> > + * becomes 4 and __ffs(4) == 2, so the next attempt starts at
> > + * order 2.
> > + */
>
> Great comment thanks!
>
> > + offset += nr_ptes;
> > + order = max_order_from_offset(offset);
> > + }
> > +done:
> > + if (collapsed)
> > + return SCAN_SUCCEED;
> > + if (alloc_failed)
> > + return SCAN_ALLOC_HUGE_PAGE_FAIL;
> > + return last_result;
> > +}
> > +
> > static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
> > struct vm_area_struct *vma, unsigned long start_addr,
> > bool *lock_dropped, struct collapse_control *cc)
> > {
> > - const unsigned int max_ptes_none = collapse_max_ptes_none(cc, vma, HPAGE_PMD_ORDER);
> > const unsigned int max_ptes_shared = collapse_max_ptes_shared(cc, HPAGE_PMD_ORDER);
> > const unsigned int max_ptes_swap = collapse_max_ptes_swap(cc, HPAGE_PMD_ORDER);
> > + unsigned int max_ptes_none = collapse_max_ptes_none(cc, vma, HPAGE_PMD_ORDER);
> > + enum tva_type tva_flags = cc->is_khugepaged ? TVA_KHUGEPAGED : TVA_FORCED_COLLAPSE;
> > pmd_t *pmd;
> > - pte_t *pte, *_pte;
> > + pte_t *pte, *_pte, pteval;
> > + int i;
> > int none_or_zero = 0, shared = 0, referenced = 0;
> > enum scan_result result = SCAN_FAIL;
> > struct page *page = NULL;
> > struct folio *folio = NULL;
> > unsigned long addr;
> > + unsigned long enabled_orders;
> > spinlock_t *ptl;
> > int node = NUMA_NO_NODE, unmapped = 0;
> >
> > @@ -1465,8 +1580,19 @@ static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
> > goto out;
> > }
> >
> > + bitmap_zero(cc->mthp_present_ptes, MAX_PTRS_PER_PTE);
> > memset(cc->node_load, 0, sizeof(cc->node_load));
> > nodes_clear(cc->alloc_nmask);
> > +
> > + enabled_orders = collapse_possible_orders(vma, vma->vm_flags, tva_flags);
> > +
> > + /*
> > + * If PMD is the only enabled order, enforce max_ptes_none, otherwise
> > + * scan all pages to populate the bitmap for mTHP collapse.
> > + */
> > + if (enabled_orders != BIT(HPAGE_PMD_ORDER))
> > + max_ptes_none = KHUGEPAGED_MAX_PTES_LIMIT;
> > +
> > pte = pte_offset_map_lock(mm, pmd, start_addr, &ptl);
> > if (!pte) {
> > cc->progress++;
> > @@ -1474,11 +1600,13 @@ static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
> > goto out;
> > }
> >
> > - for (addr = start_addr, _pte = pte; _pte < pte + HPAGE_PMD_NR;
> > - _pte++, addr += PAGE_SIZE) {
> > + for (i = 0; i < HPAGE_PMD_NR; i++) {
> > + _pte = pte + i;
> > + addr = start_addr + i * PAGE_SIZE;
> > + pteval = ptep_get(_pte);
> > +
> > cc->progress++;
> >
> > - pte_t pteval = ptep_get(_pte);
> > if (pte_none_or_zero(pteval)) {
> > if (++none_or_zero > max_ptes_none) {
> > result = SCAN_EXCEED_NONE_PTE;
> > @@ -1558,6 +1686,8 @@ static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
> > }
> > }
> >
> > + /* Set bit for occupied pages */
> > + __set_bit(i, cc->mthp_present_ptes);
> > /*
> > * Record which node the original page is from and save this
> > * information to cc->node_load[].
> > @@ -1616,9 +1746,9 @@ static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
> > if (result == SCAN_SUCCEED) {
> > /* collapse_huge_page expects the lock to be dropped before calling */
> > mmap_read_unlock(mm);
> > - result = collapse_huge_page(mm, start_addr, referenced,
> > - unmapped, cc, HPAGE_PMD_ORDER);
> > - /* collapse_huge_page will return with the mmap_lock released */
> > + result = mthp_collapse(mm, start_addr, referenced,
> > + unmapped, cc, enabled_orders);
> > + /* mmap_lock was released above, set lock_dropped */
> > *lock_dropped = true;
> > }
> > out:
> > --
> > 2.54.0
> >
>
> Cheers, Lorenzo
>
^ permalink raw reply
* Re: [PATCH mm-unstable v19 11/14] mm/khugepaged: Introduce mTHP collapse support
From: Nico Pache @ 2026-06-09 9:06 UTC (permalink / raw)
To: David Hildenbrand (Arm), Lance Yang
Cc: linux-doc, linux-kernel, linux-mm, linux-trace-kernel, aarcange,
akpm, anshuman.khandual, apopple, baohua, baolin.wang, byungchul,
catalin.marinas, cl, corbet, dave.hansen, dev.jain, gourry,
hannes, hughd, jack, jackmanb, jannh, jglisse, joshua.hahnjy, kas,
liam, ljs, mathieu.desnoyers, matthew.brost, mhiramat, mhocko,
peterx, pfalcato, rakie.kim, raquini, rdunlap, richard.weiyang,
rientjes, rostedt, rppt, ryan.roberts, shivankg, sunnanyong,
surenb, thomas.hellstrom, tiwai, usamaarif642, vbabka,
vishal.moola, wangkefeng.wang, will, willy, yang, ying.huang, ziy,
zokeefe
In-Reply-To: <2553caae-9e0e-42a7-8b61-d1216f1e81fa@kernel.org>
On Mon, Jun 8, 2026 at 8:57 AM David Hildenbrand (Arm) <david@kernel.org> wrote:
>
> On 6/6/26 12:28, Lance Yang wrote:
> >
> > On Fri, Jun 05, 2026 at 10:14:18AM -0600, Nico Pache wrote:
> >> Enable khugepaged to collapse to mTHP orders. This patch implements the
> >> main scanning logic using a bitmap to track occupied pages and the
> >> algorithm to find optimal collapse sizes.
> >>
> >> Previous to this patch, PMD collapse had 3 main phases, a light weight
> >> scanning phase (mmap_read_lock) that determines a potential PMD
> >> collapse, an alloc phase (mmap unlocked), then finally heavier collapse
> >> phase (mmap_write_lock).
> >>
> >> To enabled mTHP collapse we make the following changes:
> >>
> >> During PMD scan phase, track occupied pages in a bitmap. When mTHP
> >> orders are enabled, we remove the restriction of max_ptes_none during the
> >> scan phase to avoid missing potential mTHP collapse candidates. Once we
> >> have scanned the full PMD range and updated the bitmap to track occupied
> >> pages, we use the bitmap to find the optimal mTHP size.
> >>
> >> Implement mthp_collapse() to walk forward through the bitmap and
> >> determine the best eligible order for each naturally-aligned region. The
> >> algorithm starts at the beginning of the PMD range and, for each offset,
> >> tries the highest order that fits the alignment. If the number of
> >> occupied PTEs in that region satisfies the max_ptes_none threshold for
> >> that order, a collapse is attempted. On failure, the order is
> >> decremented and the same offset is retried at the next smaller size. Once
> >> the smallest enabled order is exhausted (or a collapse succeeds), the
> >> offset advances past the region just processed, and the next attempt
> >> starts at the highest order permitted by the new offset's natural
> >> alignment.
> >>
> >> The algorithm works as follows:
> >> 1) set offset=0 and order=HPAGE_PMD_ORDER
> >> 2) if the order is not enabled, go to step (5)
> >> 3) count occupied PTEs in the (offset, order) range using
> >> bitmap_weight_from()
> >> 4) if the count satisfies the max_ptes_none threshold, attempt
> >> collapse; on success, advance to step (6)
> >> 5) if a smaller enabled order exists, decrement order and retry
> >> from step (2) at the same offset
> >> 6) advance offset past the current region and compute the next
> >> order from the new offset's natural alignment via __ffs(offset),
> >> capped at HPAGE_PMD_ORDER
> >> 7) repeat from step (2) until the full PMD range is covered
> >>
> >> mTHP collapses reject regions containing swapped out or shared pages.
> >> This is because adding new entries can lead to new none pages, and these
> >> may lead to constant promotion into a higher order mTHP. A similar
> >> issue can occur with "max_ptes_none > HPAGE_PMD_NR/2" due to a collapse
> >> introducing at least 2x the number of pages, and on a future scan will
> >> satisfy the promotion condition once again. This issue is prevented via
> >> the collapse_max_ptes_none() function which imposes the max_ptes_none
> >> restrictions above.
> >>
> >> We currently only support mTHP collapse for max_ptes_none values of 0
> >> and HPAGE_PMD_NR - 1. resulting in the following behavior:
> >>
> >> - max_ptes_none=0: Never introduce new empty pages during collapse
> >> - max_ptes_none=HPAGE_PMD_NR-1: Always try collapse to the highest
> >> available mTHP order
> >>
> >> Any other max_ptes_none value will emit a warning and default mTHP
> >> collapse to max_ptes_none=0. There should be no behavior change for PMD
> >> collapse.
> >>
> >> Once we determine what mTHP sizes fits best in that PMD range a collapse
> >> is attempted. A minimum collapse order of 2 is used as this is the lowest
> >> order supported by anon memory as defined by THP_ORDERS_ALL_ANON.
> >>
> >> Currently madv_collapse is not supported and will only attempt PMD
> >> collapse.
> >>
> >> We can also remove the check for is_khugepaged inside the PMD scan as
> >> the collapse_max_ptes_none() function handles this logic now.
> >>
> >> Signed-off-by: Nico Pache <npache@redhat.com>
> >> ---
> >> mm/khugepaged.c | 146 +++++++++++++++++++++++++++++++++++++++++++++---
> >> 1 file changed, 138 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> >> index ec886a031952..430047316f43 100644
> >> --- a/mm/khugepaged.c
> >> +++ b/mm/khugepaged.c
> >> @@ -99,6 +99,8 @@ static DEFINE_READ_MOSTLY_HASHTABLE(mm_slots_hash, MM_SLOTS_HASH_BITS);
> >>
> >> static struct kmem_cache *mm_slot_cache __ro_after_init;
> >>
> >> +#define KHUGEPAGED_MIN_MTHP_ORDER 2
> >> +
> >> struct collapse_control {
> >> bool is_khugepaged;
> >>
> >> @@ -110,6 +112,9 @@ struct collapse_control {
> >>
> >> /* nodemask for allocation fallback */
> >> nodemask_t alloc_nmask;
> >> +
> >> + /* Each bit represents a single occupied (!none/zero) page. */
> >> + DECLARE_BITMAP(mthp_present_ptes, MAX_PTRS_PER_PTE);
> >> };
> >>
> >> /**
> >> @@ -1440,20 +1445,130 @@ static enum scan_result collapse_huge_page(struct mm_struct *mm, unsigned long s
> >> return result;
> >> }
> >>
> >> +/* Return the highest naturally aligned order that fits at @offset within a PMD. */
> >> +static unsigned int max_order_from_offset(unsigned int offset)
> >> +{
> >> + if (offset == 0)
> >> + return HPAGE_PMD_ORDER;
> >> +
> >> + return min_t(unsigned int, __ffs(offset), HPAGE_PMD_ORDER);
> >> +}
> >> +
> >> +/*
> >> + * mthp_collapse() consumes the bitmap that is generated during
> >> + * collapse_scan_pmd() to determine what regions and mTHP orders fit best.
> >> + *
> >> + * Each bit in cc->mthp_present_ptes represents a single occupied (!none/zero)
> >> + * page. We start at the PMD order and check if it is eligible for collapse;
> >> + * if not, we check the left and right halves of the PTE page table we are
> >> + * examining at a lower order.
> >> + *
> >> + * For each of these, we determine how many PTE entries are occupied in the
> >> + * range of PTE entries we propose to collapse, then we compare this to a
> >> + * threshold number of PTE entries which would need to be occupied for a
> >> + * collapse to be permitted at that order (accounting for max_ptes_none).
> >> + *
> >> + * If a collapse is permitted, we attempt to collapse the PTE range into a
> >> + * mTHP.
> >> + */
> >> +static enum scan_result mthp_collapse(struct mm_struct *mm,
> >> + unsigned long address, int referenced, int unmapped,
> >> + struct collapse_control *cc, unsigned long enabled_orders)
> >> +{
> >> + unsigned int nr_occupied_ptes, nr_ptes, max_ptes_none;
> >> + enum scan_result last_result = SCAN_FAIL;
> >> + int collapsed = 0;
> >> + bool alloc_failed = false;
> >> + unsigned long collapse_address;
> >> + unsigned int offset = 0;
> >> + unsigned int order = HPAGE_PMD_ORDER;
> >> +
> >> + while (offset < HPAGE_PMD_NR) {
> >> + nr_ptes = 1UL << order;
> >> +
> >> + if (!test_bit(order, &enabled_orders))
> >> + goto next_order;
> >> +
> >> + max_ptes_none = collapse_max_ptes_none(cc, NULL, order);
> >> + nr_occupied_ptes = bitmap_weight_from(cc->mthp_present_ptes, offset,
> >> + offset + nr_ptes);
> >> +
> >> + if (nr_occupied_ptes >= nr_ptes - max_ptes_none) {
> >
> > Looks broken for swap PTEs in PMD collapse ...
> >
> > collapse_scan_pmd() allows them up to max_ptes_swap and record them in
> > unmapped, but they don't get a bit in mthp_present_ptes. And then
> > mthp_collapse() does the check above:
>
> Right. I assumed this is implicitly handled by the optimization in collapse_scan_pmd:
>
> if (enabled_orders != BIT(HPAGE_PMD_ORDER))
> max_ptes_none = KHUGEPAGED_MAX_PTES_LIMIT;
>
> But we perform the check a second time.
>
> >
> > nr_occupied_ptes >= nr_ptes - max_ptes_none
> >
> > So max_ptes_none=0 + 511 present PTEs + one allowed swap PTE won't even
> > call collapse_huge_page() for PMD order.
> >
> > Shouldn't we account for them in the PMD-order check? Something like:
> >
> > if (is_pmd_order(order))
> > nr_occupied_ptes += unmapped;
This solution seems good for a temporary fixup. but longterm we may
want something else. I'm still not sure how we plan on supporting
swapin without causing creep. So I'd be ok with adding a fix for
legacy PMD behavior until we know how to handle mTHP creep correctly.
> As an alternative, we could either 1) skip the check there for
> pmd order (as the check was already done); or 2) introduce+maintain
> a bitmap that tracks non-present PTEs.
>
> @@ -1475,7 +1477,9 @@ static enum scan_result mthp_collapse(struct mm_struct *mm,
> nr_occupied_ptes = bitmap_weight_from(cc->mthp_present_ptes, offset,
> offset + nr_ptes);
>
> - if (nr_occupied_ptes >= nr_ptes - max_ptes_none) {
> + /* Check was already done in the caller. */
> + if (is_pmd_order(order) ||
> + nr_occupied_ptes >= nr_ptes - max_ptes_none) {
> enum scan_result ret;
>
> collapse_address = address + offset * PAGE_SIZE;
>
> 2) would probably be cleanest long-term.
That would be best for future swapin support in mTHP, but I still
don't think it solves the creep issue. Perhaps we could combine the
two bitmaps to determine if it would make the future collapse eligible
again? Not sure but ill start thinking about it.
Should I send a fixup for this using Lance's solution? Or does Lance
want to send a patch out with the fixes tag?
>
> --
> Cheers,
>
> David
>
^ permalink raw reply
* Re: [PATCH v8 2/6] mm/memory-failure: surface unhandlable kernel pages as -ENOTRECOVERABLE
From: Lance Yang @ 2026-06-09 9:08 UTC (permalink / raw)
To: David Hildenbrand (Arm), Miaohe Lin, Breno Leitao
Cc: linux-mm, linux-kernel, linux-doc, linux-kselftest,
linux-trace-kernel, kernel-team, Andrew Morton, Lorenzo Stoakes,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Shuah Khan, Naoya Horiguchi, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Jonathan Corbet, Shuah Khan, Liam R. Howlett
In-Reply-To: <f2a4d5c8-3d7d-4fc3-8769-66e0c24866fb@kernel.org>
On 2026/6/9 15:09, David Hildenbrand (Arm) wrote:
> On 6/9/26 04:39, Miaohe Lin wrote:
>> On 2026/6/8 22:15, Breno Leitao wrote:
>>> On Fri, Jun 05, 2026 at 11:42:53AM +0200, David Hildenbrand (Arm) wrote:
>>>>
>>>> I mean, any such races can currently already happen one way or the other?
>>>>
>>>> Really, the only way to not get races is to tryget the (compound)page,
>>>> revalidate that the page is still part of the compound page.
>>>>
>>>> I'm not sure if that's really a good idea.
>>>>
>>>> But my memory is a bit vague in which scenarios we already hold a page reference
>>>> here to prevent any concurrent freeing?
>>>
>>> No, we don't hold one here in the case that matters.
>>>
>>> HWPoisonKernelOwned() runs at the very top of get_any_page(), before
>>> try_again: and before __get_hwpoison_page(). The first refcount taken in
>>> the whole path is the folio_try_get() inside __get_hwpoison_page(), which
>>> runs *after* the short-circuit.
>>>
>>> So get_any_page() itself never holds a reference at the check -- the only way
>>> one exists is if the caller passed MF_COUNT_INCREASED (count_increased ==
>>> true).
>>>
>>> So on the MCE/GHES path -- the one this panic option exists for -- no
>>> reference is held when HWPoisonKernelOwned() does its compound_head() +
>>> PageSlab()/PageTable()/PageLargeKmalloc() checks.
>>>
>>> Given that, I'd rather keep it racy and take no refcount than add a
>>> tryget + revalidate purely for this check. As I've said earleir, an operator
>>
>> Would it be acceptable to add a simple recheck? Something like below:
>>
>> retry:
>> head = compound_head(page);
>> PageSlab()/PageTable()/PageLargeKmalloc() checks
>> if (head != compound_head(page))
>> goto retry
>
> Sure. I guess it could still be racy in some weird scenarios where we
> free+allocate+free in-between.
+1, sounds reasonable to me. Still racy, but acceptable here I guess :D
^ permalink raw reply
* Re: [PATCH mm-unstable v19 11/14] mm/khugepaged: Introduce mTHP collapse support
From: David Hildenbrand (Arm) @ 2026-06-09 9:25 UTC (permalink / raw)
To: Nico Pache, Lance Yang
Cc: linux-doc, linux-kernel, linux-mm, linux-trace-kernel, aarcange,
akpm, anshuman.khandual, apopple, baohua, baolin.wang, byungchul,
catalin.marinas, cl, corbet, dave.hansen, dev.jain, gourry,
hannes, hughd, jack, jackmanb, jannh, jglisse, joshua.hahnjy, kas,
liam, ljs, mathieu.desnoyers, matthew.brost, mhiramat, mhocko,
peterx, pfalcato, rakie.kim, raquini, rdunlap, richard.weiyang,
rientjes, rostedt, rppt, ryan.roberts, shivankg, sunnanyong,
surenb, thomas.hellstrom, tiwai, usamaarif642, vbabka,
vishal.moola, wangkefeng.wang, will, willy, yang, ying.huang, ziy,
zokeefe
In-Reply-To: <CAA1CXcBY_2372eJru8VoCq90rUMxn7w23hHou68MmXRv48NRXg@mail.gmail.com>
On 6/9/26 11:06, Nico Pache wrote:
> On Mon, Jun 8, 2026 at 8:57 AM David Hildenbrand (Arm) <david@kernel.org> wrote:
>>
>> On 6/6/26 12:28, Lance Yang wrote:
>>>
>>>
>>> Looks broken for swap PTEs in PMD collapse ...
>>>
>>> collapse_scan_pmd() allows them up to max_ptes_swap and record them in
>>> unmapped, but they don't get a bit in mthp_present_ptes. And then
>>> mthp_collapse() does the check above:
>>
>> Right. I assumed this is implicitly handled by the optimization in collapse_scan_pmd:
>>
>> if (enabled_orders != BIT(HPAGE_PMD_ORDER))
>> max_ptes_none = KHUGEPAGED_MAX_PTES_LIMIT;
>>
>> But we perform the check a second time.
>>
>>>
>>> nr_occupied_ptes >= nr_ptes - max_ptes_none
>>>
>>> So max_ptes_none=0 + 511 present PTEs + one allowed swap PTE won't even
>>> call collapse_huge_page() for PMD order.
>>>
>>> Shouldn't we account for them in the PMD-order check? Something like:
>>>
>>> if (is_pmd_order(order))
>>> nr_occupied_ptes += unmapped;
>
> This solution seems good for a temporary fixup. but longterm we may
> want something else. I'm still not sure how we plan on supporting
> swapin without causing creep. So I'd be ok with adding a fix for
> legacy PMD behavior until we know how to handle mTHP creep correctly.
>
>> As an alternative, we could either 1) skip the check there for
>> pmd order (as the check was already done); or 2) introduce+maintain
>> a bitmap that tracks non-present PTEs.
>>
>> @@ -1475,7 +1477,9 @@ static enum scan_result mthp_collapse(struct mm_struct *mm,
>> nr_occupied_ptes = bitmap_weight_from(cc->mthp_present_ptes, offset,
>> offset + nr_ptes);
>>
>> - if (nr_occupied_ptes >= nr_ptes - max_ptes_none) {
>> + /* Check was already done in the caller. */
>> + if (is_pmd_order(order) ||
>> + nr_occupied_ptes >= nr_ptes - max_ptes_none) {
>> enum scan_result ret;
>>
>> collapse_address = address + offset * PAGE_SIZE;
>>
>> 2) would probably be cleanest long-term.
>
> That would be best for future swapin support in mTHP, but I still
> don't think it solves the creep issue.
It wouldn't, we'd simply maintain the state we collect + rely on in separate
bitmaps. On swapin, we'd have to update/refresh bitmaps I guess.
> Perhaps we could combine the
> two bitmaps to determine if it would make the future collapse eligible
> again? Not sure but ill start thinking about it.
>
> Should I send a fixup for this using Lance's solution? Or does Lance
> want to send a patch out with the fixes tag?
If Lance could send a fixup, explaining the situation, that would be nice.
--
Cheers,
David
^ permalink raw reply
* Re: [PATCH mm-unstable v19 11/14] mm/khugepaged: Introduce mTHP collapse support
From: Nico Pache @ 2026-06-09 9:32 UTC (permalink / raw)
To: David Hildenbrand (Arm)
Cc: Lance Yang, linux-doc, linux-kernel, linux-mm, linux-trace-kernel,
aarcange, akpm, anshuman.khandual, apopple, baohua, baolin.wang,
byungchul, catalin.marinas, cl, corbet, dave.hansen, dev.jain,
gourry, hannes, hughd, jack, jackmanb, jannh, jglisse,
joshua.hahnjy, kas, liam, ljs, mathieu.desnoyers, matthew.brost,
mhiramat, mhocko, peterx, pfalcato, rakie.kim, raquini, rdunlap,
richard.weiyang, rientjes, rostedt, rppt, ryan.roberts, shivankg,
sunnanyong, surenb, thomas.hellstrom, tiwai, usamaarif642, vbabka,
vishal.moola, wangkefeng.wang, will, willy, yang, ying.huang, ziy,
zokeefe
In-Reply-To: <b7fb4184-7a99-42c7-8ee2-4c7fa20827c4@kernel.org>
On Tue, Jun 9, 2026 at 3:26 AM David Hildenbrand (Arm) <david@kernel.org> wrote:
>
> On 6/9/26 11:06, Nico Pache wrote:
> > On Mon, Jun 8, 2026 at 8:57 AM David Hildenbrand (Arm) <david@kernel.org> wrote:
> >>
> >> On 6/6/26 12:28, Lance Yang wrote:
> >>>
> >>>
> >>> Looks broken for swap PTEs in PMD collapse ...
> >>>
> >>> collapse_scan_pmd() allows them up to max_ptes_swap and record them in
> >>> unmapped, but they don't get a bit in mthp_present_ptes. And then
> >>> mthp_collapse() does the check above:
> >>
> >> Right. I assumed this is implicitly handled by the optimization in collapse_scan_pmd:
> >>
> >> if (enabled_orders != BIT(HPAGE_PMD_ORDER))
> >> max_ptes_none = KHUGEPAGED_MAX_PTES_LIMIT;
> >>
> >> But we perform the check a second time.
> >>
> >>>
> >>> nr_occupied_ptes >= nr_ptes - max_ptes_none
> >>>
> >>> So max_ptes_none=0 + 511 present PTEs + one allowed swap PTE won't even
> >>> call collapse_huge_page() for PMD order.
> >>>
> >>> Shouldn't we account for them in the PMD-order check? Something like:
> >>>
> >>> if (is_pmd_order(order))
> >>> nr_occupied_ptes += unmapped;
> >
> > This solution seems good for a temporary fixup. but longterm we may
> > want something else. I'm still not sure how we plan on supporting
> > swapin without causing creep. So I'd be ok with adding a fix for
> > legacy PMD behavior until we know how to handle mTHP creep correctly.
> >
> >> As an alternative, we could either 1) skip the check there for
> >> pmd order (as the check was already done); or 2) introduce+maintain
> >> a bitmap that tracks non-present PTEs.
> >>
> >> @@ -1475,7 +1477,9 @@ static enum scan_result mthp_collapse(struct mm_struct *mm,
> >> nr_occupied_ptes = bitmap_weight_from(cc->mthp_present_ptes, offset,
> >> offset + nr_ptes);
> >>
> >> - if (nr_occupied_ptes >= nr_ptes - max_ptes_none) {
> >> + /* Check was already done in the caller. */
> >> + if (is_pmd_order(order) ||
> >> + nr_occupied_ptes >= nr_ptes - max_ptes_none) {
> >> enum scan_result ret;
> >>
> >> collapse_address = address + offset * PAGE_SIZE;
> >>
> >> 2) would probably be cleanest long-term.
> >
> > That would be best for future swapin support in mTHP, but I still
> > don't think it solves the creep issue.
>
> It wouldn't, we'd simply maintain the state we collect + rely on in separate
> bitmaps. On swapin, we'd have to update/refresh bitmaps I guess.
Yeah, I'm saying for the future, it obviously solves this issue here
as well, but if we have positional tracking of the swapout, shared,
and none PTEs, I think we can use this to determine whether the
collapse would lead to creep. If we detect creep would happen it may
be best to automatically collapse to the N+1 (or greater) candidate.
Just thinking outloud here.
>
> > Perhaps we could combine the
> > two bitmaps to determine if it would make the future collapse eligible
> > again? Not sure but ill start thinking about it.
> >
> > Should I send a fixup for this using Lance's solution? Or does Lance
> > want to send a patch out with the fixes tag?
>
> If Lance could send a fixup, explaining the situation, that would be nice.
OK, I'd appreciate that :)
Cheers,
-- Nico
>
> --
> Cheers,
>
> David
>
^ permalink raw reply
* Re: [PATCH v3] rethook: Remove the running task check in rethook_find_ret_addr()
From: Petr Mladek @ 2026-06-09 9:43 UTC (permalink / raw)
To: Tengda Wu
Cc: Masami Hiramatsu, Peter Zijlstra, Steven Rostedt,
Mathieu Desnoyers, Alexei Starovoitov, linux-trace-kernel,
linux-kernel, live-patching
In-Reply-To: <20260609084953.901576-1-wutengda@huaweicloud.com>
Added live-patching mailing list.
On Tue 2026-06-09 16:49:53, Tengda Wu wrote:
> The current check in rethook_find_ret_addr() prevents obtaining a return
> address when the target task is marked as running. However, this condition
> is both insufficient for correctness and unnecessary for its intended
> purpose.
>
> The check is inherently racy: a task can begin running on another CPU
> immediately after task_is_running() returns false, potentially leading to
> concurrent modification of rethook data structures while the iteration is
> in progress.
>
> Rather than trying to fix this unreliable check deep in the unwinding
> path, simply remove it. The iteration is already safe from crashes because
> unwind_next_frame() holds RCU and rethook_node structures are RCU-freed;
> even if the iteration goes off the rails and returns invalid information,
> it will not crash. Callers that require consistency must provide a safe
> context themselves.
>
> Fixes: 54ecbe6f1ed5 ("rethook: Add a generic return hook")
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>
> ---
> v3: Improve commit message: clarify safety semantics and document that RCU guarantees no crash.
> v2: https://lore.kernel.org/all/20260609005728.458962-1-wutengda@huaweicloud.com/
> v1: https://lore.kernel.org/all/20260525132253.1889726-1-wutengda@huaweicloud.com/
>
> --- a/kernel/trace/rethook.c
> +++ b/kernel/trace/rethook.c
> @@ -250,9 +250,6 @@ unsigned long rethook_find_ret_addr(struct task_struct *tsk, unsigned long frame
> if (WARN_ON_ONCE(!cur))
> return 0;
>
> - if (tsk != current && task_is_running(tsk))
> - return 0;
> -
The description of the function should be updated as well. It still
mentions:
* The @tsk must be 'current' or a task which is not running.
Instead it should explain that it safe to call the function even
on another running tasks but the returned address is not reliable
then.
> do {
> ret = __rethook_find_ret_addr(tsk, cur);
> if (!ret)
I am still a bit concerned about the motivation.
Tengda mentioned at
https://lore.kernel.org/all/679a1c8f-1e4d-4ae5-83e1-d0068e6de1a6@huaweicloud.com/
that they tried to verify livepatching:
<paste>
Background: We are verifying the support of live patches for functions that
have a kretprobe. The specific verification method is as follows:
We construct a function foo() that calls bar():
void bar(void)
{
for (;;) {
schedule();
}
}
void foo(void)
{
bar();
}
A kretprobe is attached to bar():
echo 'r:rp1 bar' > /sys/kernel/tracing/kprobe_events
echo 1 > /sys/kernel/tracing/events/kprobes/rp1/enable
Then foo() is triggered. The expected behavior is that bar() will call
schedule() and yield the CPU.
After that, the live patch is activated to attempt replacing the implementation
of foo(). The expectation is that this should succeed.
However, in reality, because the task that called schedule() is still in the
RUNNING state, the condition task_is_running(tsk) inside rethook_find_ret_addr()
is not satisfied, causing the function to return early. This, in turn,
prevents stack_trace_save_tsk_reliable() from determining the stack as
reliable, leading to a failure in activating the live patch.
**Not sure if this is correct:**
We believe that after a task voluntarily calls schedule(), when the stack
is expected to be reliable, it is a safe time to activate a live patch.
Additionally, a similar tsk->on_cpu check can be found elsewhere in the
kernel (See task_on_another_cpu() in arch/x86/include/asm/unwind.h).
Therefore, we propose changing the task_is_running(tsk) condition to
tsk->on_cpu.
</paste>
More background:
----------------
The test is artificial because it keeps the RUNNING state before
calling schedule, see
https://lore.kernel.org/all/20260608093449.GH4149641@noisy.programming.kicks-ass.net/
My questions:
Does this patch allows to livepatch the above mentioned test code?
Is the livepatching safe?
Does it help in another scenarios?
My opinion:
The livepatching might be safe only when the process is migrating
itself. I mean that it might be safe even when it is RUNNING as long
at it is _current_.
I agree that we do not need to enforce this in rethook_find_ret_addr()
if the function is used also in other scenarios, for example, by
ftrace/BTF for taking snapshots of other processes.
But we need to make sure that the backtrace is reliable when
livepatching (migrating) the task.
Best Regards,
Petr
^ permalink raw reply
* Re: [PATCH 0/2] arm64: ftrace: support DIRECT_CALLS without CALL_OPS
From: Puranjay Mohan @ 2026-06-09 9:50 UTC (permalink / raw)
To: Jose Fernandez (Anthropic)
Cc: Steven Rostedt, Masami Hiramatsu, Mark Rutland, Catalin Marinas,
Will Deacon, Nathan Chancellor, Nick Desaulniers, Bill Wendling,
Justin Stitt, linux-kernel, linux-trace-kernel, linux-arm-kernel,
llvm, bpf, Florent Revest, Xu Kuohai
In-Reply-To: <20260609-arm64-ftrace-direct-calls-v1-0-4a46f266697f@linux.dev>
On Tue, Jun 9, 2026 at 6:19 AM Jose Fernandez (Anthropic)
<jose.fernandez@linux.dev> wrote:
>
> On arm64, HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS is currently selected
> only when DYNAMIC_FTRACE_WITH_CALL_OPS is available. CALL_OPS, in
> turn, is mutually exclusive with kCFI: the pre-function NOPs it needs
> would change the offset of the pre-function type hash (see
> baaf553d3bc3 ("arm64: Implement HAVE_DYNAMIC_FTRACE_WITH_CALL_OPS")),
> and the compiler support needed to reconcile the two does not exist
> yet.
>
> The result is that a CONFIG_CFI=y arm64 kernel has no
> ftrace direct calls at all, so register_fentry() fails with -ENOTSUPP
> and no BPF trampoline can attach: fentry/fexit, fmod_ret and BPF LSM
> programs are all unavailable. Deployments that want both kCFI
> hardening and BPF-based security monitoring currently have to give
> one of them up. systemd's bpf-restrict-fs feature hits this today:
> https://lore.kernel.org/all/20250610232418.GA3544567@ax162/
>
> CALL_OPS is an optimization for direct calls, not a dependency.
> In-BL-range trampolines are reached by a direct branch without
> consulting the ops pointer, and out-of-range trampolines already
> fall back to ftrace_caller, where the DIRECT_CALLS machinery
> (call_direct_funcs() storing the trampoline in ftrace_regs, the
> ftrace_caller tail-call) is gated on DIRECT_CALLS alone. s390 and
> loongarch ship HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS this way,
> without having CALL_OPS at all.
>
> Patch 1 prepares ftrace_modify_call() to build without CALL_OPS by
> widening its #ifdef and using the existing ftrace_rec_update_ops()
> wrapper (no functional change for current configurations). Patch 2
> drops the CALL_OPS requirement from the DIRECT_CALLS select.
>
> Configurations that keep CALL_OPS (clang !CFI, and GCC without
> CC_OPTIMIZE_FOR_SIZE) are unchanged. We verified this: in an arm64
> clang build, every object file is byte-identical before and after
> the series except ftrace.o itself, and its disassembly is identical.
> CFI builds (and GCC -Os builds) gain working direct calls, with
> out-of-range attachments taking the ftrace_caller dispatch path
> instead of the per-callsite fast path.
>
> We tested on a 6.18.y-based kernel and on this base with clang
> kCFI builds (CONFIG_CFI=y, enforcing) under qemu (TCG, and KVM on an
> arm64 host) and on GB200-based arm64 hardware: fentry/fexit, fmod_ret
> and BPF LSM programs load, attach and execute; the ftrace-direct
> sample modules (including both modify samples, exercising
> ftrace_modify_call()) run cleanly; no CFI violations observed. The
> fentry_test, fexit_test, fentry_fexit, fexit_sleep, fexit_stress,
> modify_return, tracing_struct, lsm and trampoline_count selftests and
> the ftrace direct-call selftests (test.d/direct) pass on the new
> configuration with results identical to a CALL_OPS kernel built from
> the same tree, and a broader test_progs sweep showed no differences
> attributable to this series. Without the series, all of the above
> fail at attach time with -ENOTSUPP.
>
> riscv has the same gap (its DIRECT_CALLS select also requires
> CALL_OPS, and its CALL_OPS is likewise !CFI); if this approach is
> acceptable for arm64 we can follow up there.
>
It looks correct to me and should work.
Reviewed-by: Puranjay Mohan <puranjay@kernel.org>
^ permalink raw reply
* Re: [PATCH v3 0/6] bootconfig: embed kernel.* cmdline at build time
From: Breno Leitao @ 2026-06-09 10:16 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Andrew Morton, Nathan Chancellor, paulmck, Nicolas Schier,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, linux-kernel, linux-trace-kernel, linux-kbuild,
bpf, kernel-team
In-Reply-To: <20260609104611.a0a5def510944d12cd0b6dfc@kernel.org>
On Tue, Jun 09, 2026 at 10:46:11AM +0900, Masami Hiramatsu wrote:
> On Mon, 08 Jun 2026 09:23:57 -0700
> Breno Leitao <leitao@debian.org> wrote:
>
> > The userspace pieces (xbc_snprint_cmdline() in lib/, tools/bootconfig -C)
> > already landed; this series wires the rendered cmdline into the kernel.
> >
> > Motivation: today the embedded bootconfig is parsed at runtime, after
> > parse_early_param() has already run, so early_param() handlers can't
> > see embedded values. Folding the kernel.* subtree into the cmdline at
> > build time gives a CONFIG_CMDLINE-equivalent for embedded-bootconfig
> > users without forcing them to maintain two cmdline sources.
> >
> > Behaviorally, the "kernel" subtree is rendered to a flat string at
> > build time and stashed in .init.rodata. setup_arch() prepends it to
> > boot_command_line before parse_early_param() runs. Overflow is a soft
> > error: the helper logs and leaves boot_command_line untouched rather
> > than panicking, so an oversized embedded bconf cannot brick a boot.
> >
>
> Sashiko still leaves some comments.
> https://sashiko.dev/#/patchset/20260608-bootconfig_using_tools-v3-0-4ddd079a0696%40debian.org
Ack. There are two "low" issues and one high. I will fix all of them in
v4.
> BTW, can you also update the document (Documentation/admin-guide/bootconfig.rst)
> about what is the expected behavior of this feature (kconfigs, examples)?
Sure, good idea. I will add a new patch documenting this new feature.
Thanks for the review,
--breno
^ permalink raw reply
* Re: [PATCH v8 2/6] mm/memory-failure: surface unhandlable kernel pages as -ENOTRECOVERABLE
From: Breno Leitao @ 2026-06-09 10:21 UTC (permalink / raw)
To: Lance Yang
Cc: David Hildenbrand (Arm), Miaohe Lin, linux-mm, linux-kernel,
linux-doc, linux-kselftest, linux-trace-kernel, kernel-team,
Andrew Morton, Lorenzo Stoakes, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Shuah Khan, Naoya Horiguchi,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Jonathan Corbet, Shuah Khan, Liam R. Howlett
In-Reply-To: <f21d7c12-e6c7-49b0-8d83-c26946d0d4ee@linux.dev>
On Tue, Jun 09, 2026 at 05:08:14PM +0800, Lance Yang wrote:
>
>
> On 2026/6/9 15:09, David Hildenbrand (Arm) wrote:
> > On 6/9/26 04:39, Miaohe Lin wrote:
> > > On 2026/6/8 22:15, Breno Leitao wrote:
> > > > On Fri, Jun 05, 2026 at 11:42:53AM +0200, David Hildenbrand (Arm) wrote:
> > > > >
> > > > > I mean, any such races can currently already happen one way or the other?
> > > > >
> > > > > Really, the only way to not get races is to tryget the (compound)page,
> > > > > revalidate that the page is still part of the compound page.
> > > > >
> > > > > I'm not sure if that's really a good idea.
> > > > >
> > > > > But my memory is a bit vague in which scenarios we already hold a page reference
> > > > > here to prevent any concurrent freeing?
> > > >
> > > > No, we don't hold one here in the case that matters.
> > > >
> > > > HWPoisonKernelOwned() runs at the very top of get_any_page(), before
> > > > try_again: and before __get_hwpoison_page(). The first refcount taken in
> > > > the whole path is the folio_try_get() inside __get_hwpoison_page(), which
> > > > runs *after* the short-circuit.
> > > >
> > > > So get_any_page() itself never holds a reference at the check -- the only way
> > > > one exists is if the caller passed MF_COUNT_INCREASED (count_increased ==
> > > > true).
> > > >
> > > > So on the MCE/GHES path -- the one this panic option exists for -- no
> > > > reference is held when HWPoisonKernelOwned() does its compound_head() +
> > > > PageSlab()/PageTable()/PageLargeKmalloc() checks.
> > > >
> > > > Given that, I'd rather keep it racy and take no refcount than add a
> > > > tryget + revalidate purely for this check. As I've said earleir, an operator
> > >
> > > Would it be acceptable to add a simple recheck? Something like below:
> > >
> > > retry:
> > > head = compound_head(page);
> > > PageSlab()/PageTable()/PageLargeKmalloc() checks
> > > if (head != compound_head(page))
> > > goto retry
> >
> > Sure. I guess it could still be racy in some weird scenarios where we
> > free+allocate+free in-between.
>
> +1, sounds reasonable to me. Still racy, but acceptable here I guess :D
Ack. I will post v9 shortly with this plus a couple of selftest fixes
Sashiko flagged.
^ permalink raw reply
* [PATCH v4 0/7] bootconfig: embed kernel.* cmdline at build time
From: Breno Leitao @ 2026-06-09 10:28 UTC (permalink / raw)
To: Masami Hiramatsu, Andrew Morton, Nathan Chancellor, paulmck,
Nicolas Schier
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, linux-kernel, linux-trace-kernel, linux-kbuild,
bpf, Breno Leitao, kernel-team
The userspace pieces (xbc_snprint_cmdline() in lib/, tools/bootconfig -C)
already landed; this series wires the rendered cmdline into the kernel.
Motivation: today the embedded bootconfig is parsed at runtime, after
parse_early_param() has already run, so early_param() handlers can't
see embedded values. Folding the kernel.* subtree into the cmdline at
build time gives a CONFIG_CMDLINE-equivalent for embedded-bootconfig
users without forcing them to maintain two cmdline sources.
Behaviorally, the "kernel" subtree is rendered to a flat string at
build time and stashed in .init.rodata. setup_arch() prepends it to
boot_command_line before parse_early_param() runs. Overflow is a soft
error: the helper logs and leaves boot_command_line untouched rather
than panicking, so an oversized embedded bconf cannot brick a boot.
Signed-off-by: Breno Leitao <leitao@debian.org>
---
Changes in v4:
- Patch 3 (build pipeline): clear CROSS_COMPILE= in the kernel-side
tools/bootconfig sub-make. Without it, an LLVM=1 cross build
inherits CROSS_COMPILE and tools/scripts/Makefile.include injects
--target=/--sysroot= into the host clang, producing a target
binary that fails to exec.
- Patch 3 (build pipeline): place embedded-cmdline.S in its own
.init.rodata.embed_cmdline subsection ("a") so ld.lld does not
see a section-type mismatch against lib/bootconfig-data.S's
writable .init.rodata ("aw"). The linker's *(.init.rodata
.init.rodata.*) glob still folds it into the init image.
- Patch 6 (x86/setup): also accept the bootconfig=<anything> form
via cmdline_find_option(), matching the runtime parse_args() loop.
Without it, bootconfig=0/=off would skip the early prepend but
still trigger the late runtime apply -- a split-brain state.
- New patch 7: document CONFIG_BOOT_CONFIG_EMBED_CMDLINE in
Documentation/admin-guide/bootconfig.rst (semantics, opt-in,
precedence, overflow behavior, example).
- Link to v3: https://lore.kernel.org/r/20260608-bootconfig_using_tools-v3-0-4ddd079a0696@debian.org
Changes in v3:
- Patch 3: Move HOSTCC override to the kernel-side rule; tool keeps
$(CC) for standalone/cross builds.
- Patch 6: Drop the false fail-safe wording; document the
BOOT_CONFIG_FORCE=y default interaction.
- Link to v2:
https://lore.kernel.org/r/20260605-bootconfig_using_tools-v2-0-d309f544b5f7@debian.org
Changes in v2 (addressing review of v1):
- Split out a standalone fix for the NULL-pointer arithmetic in
xbc_snprint_cmdline() so the build-time render cannot trip host
UBSan/FORTIFY_SOURCE.
- Rework the leaf-root handling: instead of returning early, skip @root
inside the loop so a root carrying both a value and subkeys
(kernel = x together with kernel.foo = bar) still renders its
descendant keys.
- Build tools/bootconfig with $(HOSTCC) so cross-compiled (ARCH=...)
builds render the cmdline on the build host instead of failing with
"Exec format error".
- Mark the embedded cmdline section read-only (drop the "w" flag from
.init.rodata).
- Add a make-clean hook so tools/bootconfig artifacts are removed by
make clean.
- Gate the x86 prepend on "bootconfig" being present on the command
line (or CONFIG_BOOT_CONFIG_FORCE), matching the init.* opt-in
semantics documented in bootconfig.rst and preserving fail-safe
recovery: dropping "bootconfig" from the bootloader cmdline now also
disables the embedded kernel.* keys.
- Link to v1: https://patch.msgid.link/20260527-bootconfig_using_tools-v1-0-b6906a86e7d5@debian.org
---
Breno Leitao (7):
bootconfig: fix NULL-pointer arithmetic in xbc_snprint_cmdline()
bootconfig: render descendant keys when xbc_snprint_cmdline() root has a value
bootconfig: render embedded bootconfig as a kernel cmdline at build time
bootconfig: clean build-time tools/bootconfig from make clean
bootconfig: add xbc_prepend_embedded_cmdline() helper
Documentation: bootconfig: document build-time cmdline rendering
x86/setup: prepend embedded bootconfig cmdline before parse_early_param
Documentation/admin-guide/bootconfig.rst | 46 +++++++++++++
MAINTAINERS | 1 +
Makefile | 28 +++++++-
arch/x86/Kconfig | 1 +
arch/x86/kernel/setup.c | 27 ++++++++
include/linux/bootconfig.h | 9 +++
init/Kconfig | 36 ++++++++++
init/main.c | 25 ++++++-
lib/Makefile | 16 +++++
lib/bootconfig.c | 112 +++++++++++++++++++++++++++++--
lib/embedded-cmdline.S | 16 +++++
tools/bootconfig/Makefile | 4 +-
12 files changed, 308 insertions(+), 13 deletions(-)
---
base-commit: a87737435cfa134f9cdcc696ba3080759d04cf72
change-id: 20260508-bootconfig_using_tools-cfa7aa9d6a5a
Best regards,
--
Breno Leitao <leitao@debian.org>
^ permalink raw reply
* [PATCH v4 1/7] bootconfig: fix NULL-pointer arithmetic in xbc_snprint_cmdline()
From: Breno Leitao @ 2026-06-09 10:28 UTC (permalink / raw)
To: Masami Hiramatsu, Andrew Morton, Nathan Chancellor, paulmck,
Nicolas Schier
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, linux-kernel, linux-trace-kernel, linux-kbuild,
bpf, Breno Leitao, kernel-team
In-Reply-To: <20260609-bootconfig_using_tools-v4-0-73c463f03a97@debian.org>
xbc_snprint_cmdline() is meant to be called twice: first with
buf=NULL, size=0 to probe the rendered length, then with a real
buffer to fill it (the standard snprintf() two-pass pattern). The
probe call makes the function compute "buf + size" (NULL + 0) and,
on every iteration, advance "buf += ret" from that NULL base and
pass the result back into snprintf().
Pointer arithmetic on a NULL pointer is undefined behavior. It is
harmless in the in-kernel callers today, but the follow-up patches
run this same code in the userspace tools/bootconfig parser at kernel
build time, where host UBSan / FORTIFY_SOURCE abort the build.
Track a running written length (size_t) instead of mutating @buf, and
only form "buf + len" when @buf is non-NULL. snprintf(NULL, 0, ...)
is itself well defined and returns the would-be length, so the
two-pass "probe then fill" usage returns identical byte counts.
Signed-off-by: Breno Leitao <leitao@debian.org>
---
lib/bootconfig.c | 23 ++++++++++++++++-------
1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/lib/bootconfig.c b/lib/bootconfig.c
index f445b7703fdd..2ed9ee3dc81c 100644
--- a/lib/bootconfig.c
+++ b/lib/bootconfig.c
@@ -427,10 +427,18 @@ static char xbc_namebuf[XBC_KEYLEN_MAX] __initdata;
int __init xbc_snprint_cmdline(char *buf, size_t size, struct xbc_node *root)
{
struct xbc_node *knode, *vnode;
- char *end = buf + size;
const char *val, *q;
+ size_t len = 0;
int ret;
+ /*
+ * Track the running written length rather than advancing @buf, so we
+ * never form "buf + size" or "buf += ret" while @buf is NULL (the
+ * size-probe call passes buf=NULL, size=0). NULL pointer arithmetic
+ * is undefined behavior and trips host UBSan / FORTIFY_SOURCE when
+ * this renderer runs at kernel build time. snprintf(NULL, 0, ...)
+ * itself is well defined and returns the would-be length.
+ */
xbc_node_for_each_key_value(root, knode, val) {
ret = xbc_node_compose_key_after(root, knode,
xbc_namebuf, XBC_KEYLEN_MAX);
@@ -439,10 +447,11 @@ int __init xbc_snprint_cmdline(char *buf, size_t size, struct xbc_node *root)
vnode = xbc_node_get_child(knode);
if (!vnode) {
- ret = snprintf(buf, rest(buf, end), "%s ", xbc_namebuf);
+ ret = snprintf(buf ? buf + len : NULL, rest(len, size),
+ "%s ", xbc_namebuf);
if (ret < 0)
return ret;
- buf += ret;
+ len += ret;
continue;
}
xbc_array_for_each_value(vnode, val) {
@@ -452,15 +461,15 @@ int __init xbc_snprint_cmdline(char *buf, size_t size, struct xbc_node *root)
* whitespace.
*/
q = strpbrk(val, " \t\r\n") ? "\"" : "";
- ret = snprintf(buf, rest(buf, end), "%s=%s%s%s ",
- xbc_namebuf, q, val, q);
+ ret = snprintf(buf ? buf + len : NULL, rest(len, size),
+ "%s=%s%s%s ", xbc_namebuf, q, val, q);
if (ret < 0)
return ret;
- buf += ret;
+ len += ret;
}
}
- return buf - (end - size);
+ return len;
}
#undef rest
--
2.53.0-Meta
^ permalink raw reply related
* [PATCH v4 2/7] bootconfig: render descendant keys when xbc_snprint_cmdline() root has a value
From: Breno Leitao @ 2026-06-09 10:28 UTC (permalink / raw)
To: Masami Hiramatsu, Andrew Morton, Nathan Chancellor, paulmck,
Nicolas Schier
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, linux-kernel, linux-trace-kernel, linux-kbuild,
bpf, Breno Leitao, kernel-team
In-Reply-To: <20260609-bootconfig_using_tools-v4-0-73c463f03a97@debian.org>
xbc_node_for_each_key_value() walks to the first leaf under @root, and
when @root is itself a leaf it yields @root. That happens not only for
an empty "kernel {}" subtree, but also when @root carries both a value
and subkeys, e.g.
kernel = x
kernel.foo = bar
Here @root ("kernel") is a leaf because its first child is the value
node "x", so the iterator returns @root first. Feeding @root back into
xbc_node_compose_key_after(root, root) returns -EINVAL, which the only
in-kernel caller papers over with a "len <= 0" check -- but the
follow-up tools/bootconfig -C user propagates the error and turns such
a bootconfig into a build failure. Worse, short-circuiting the whole
call on a leaf @root would silently drop the valid "kernel.foo = bar"
descendant that this patch should render.
Skip @root inside the loop instead of bailing out: the value-only entry
is dropped (it is rendered through the "kernel" cmdline path, not here),
while real descendant keys are still emitted. An entirely empty subtree
now renders nothing and returns 0 rather than -EINVAL, matching the
"nothing to render is not an error" semantics expected by the new
build-time caller.
Signed-off-by: Breno Leitao <leitao@debian.org>
---
lib/bootconfig.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/lib/bootconfig.c b/lib/bootconfig.c
index 2ed9ee3dc81c..926094d97397 100644
--- a/lib/bootconfig.c
+++ b/lib/bootconfig.c
@@ -440,6 +440,17 @@ int __init xbc_snprint_cmdline(char *buf, size_t size, struct xbc_node *root)
* itself is well defined and returns the would-be length.
*/
xbc_node_for_each_key_value(root, knode, val) {
+ /*
+ * An empty or value-only @root (e.g. "kernel {}" or
+ * "kernel = x", possibly alongside "kernel.foo = bar")
+ * yields @root itself here. Skip it: composing a key for it
+ * would fail with -EINVAL, yet any real descendant keys must
+ * still be rendered. An entirely empty subtree then renders
+ * nothing and returns 0 rather than an error.
+ */
+ if (knode == root)
+ continue;
+
ret = xbc_node_compose_key_after(root, knode,
xbc_namebuf, XBC_KEYLEN_MAX);
if (ret < 0)
--
2.53.0-Meta
^ permalink raw reply related
* [PATCH v4 3/7] bootconfig: render embedded bootconfig as a kernel cmdline at build time
From: Breno Leitao @ 2026-06-09 10:28 UTC (permalink / raw)
To: Masami Hiramatsu, Andrew Morton, Nathan Chancellor, paulmck,
Nicolas Schier
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, linux-kernel, linux-trace-kernel, linux-kbuild,
bpf, Breno Leitao, kernel-team
In-Reply-To: <20260609-bootconfig_using_tools-v4-0-73c463f03a97@debian.org>
Add the build-time pipeline that renders the "kernel" subtree of
CONFIG_BOOT_CONFIG_EMBED_FILE into a flat cmdline string and stashes
it in .init.rodata as embedded_kernel_cmdline[]. A follow-up patch
adds the runtime helper that prepends this string to boot_command_line
during early architecture setup so parse_early_param() sees the values.
The build wires up:
tools/bootconfig -C kernel - userspace tool already shared with
lib/bootconfig.c, used here in -C mode
to render a bootconfig file to a cmdline
lib/embedded-cmdline.S - .incbin's the rendered text plus a NUL
(listed under the EXTRA BOOT CONFIG
MAINTAINERS entry)
lib/Makefile rule - runs tools/bootconfig at build time
Makefile prepare dep - ensures tools/bootconfig is built first,
same pattern as tools/objtool and
tools/bpf/resolve_btfids
Drop the test target from tools/bootconfig/Makefile's default 'all'
recipe so that hooking the binary into the kernel build does not run
test-bootconfig.sh on every prepare. The tests stay available as
'make -C tools/bootconfig test', matching the convention of
tools/objtool and tools/bpf/resolve_btfids whose 'all' targets only
build the binary.
Require BOOT_CONFIG_EMBED_FILE to be non-empty before the new option
can be enabled, otherwise tools/bootconfig -C runs against an empty
file and prints a parse error on every kernel build.
The feature gates on CONFIG_ARCH_SUPPORTS_CMDLINE_FROM_BOOTCONFIG, a
silent symbol arches select once they've wired the prepend call into
setup_arch(). No arch selects it in this patch, so the user-visible
CONFIG_BOOT_CONFIG_EMBED_CMDLINE is not yet enableable; when an arch
later opts in, the runtime behavior is added by the follow-up patches.
tools/bootconfig also installs on target systems, so its own Makefile
keeps $(CC) and stays cross-buildable as a standalone tool. The kernel
build, which runs the tool on the build host during prepare, instead
forces CC=$(HOSTCC) from a dedicated tools/bootconfig rule and clears
CROSS_COMPILE= in the sub-make. Without that clear, an LLVM=1 cross
build would inherit CROSS_COMPILE and tools/scripts/Makefile.include
would inject --target=/--sysroot= flags into the host clang invocation,
producing a target binary that fails to exec ("Exec format error").
embedded-cmdline.S places the rendered string in its own .init.rodata
subsection (.init.rodata.embed_cmdline) with the "a" (allocatable,
read-only) flag and %progbits. lib/bootconfig-data.S already places
the embedded bootconfig blob in .init.rodata with the "aw" flag
(xbc_init() rewrites separators in place, so that data must be
writable). Using a distinct subsection name avoids the ld.lld section-
type mismatch that would otherwise arise from mixing "a" and "aw"
under the same name; the linker's "*(.init.rodata .init.rodata.*)"
glob still folds both into the init image and frees them after boot.
A follow-up patch wires the build-time tools/bootconfig into the
top-level clean target.
Signed-off-by: Breno Leitao <leitao@debian.org>
---
MAINTAINERS | 1 +
Makefile | 15 +++++++++++++++
init/Kconfig | 36 ++++++++++++++++++++++++++++++++++++
lib/Makefile | 16 ++++++++++++++++
lib/embedded-cmdline.S | 16 ++++++++++++++++
tools/bootconfig/Makefile | 2 +-
6 files changed, 85 insertions(+), 1 deletion(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 57656ec0e9d5..953231df1911 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9844,6 +9844,7 @@ F: fs/proc/bootconfig.c
F: include/linux/bootconfig.h
F: lib/bootconfig-data.S
F: lib/bootconfig.c
+F: lib/embedded-cmdline.S
F: tools/bootconfig/*
F: tools/bootconfig/scripts/*
diff --git a/Makefile b/Makefile
index bf196c6df5b9..a7abb3f9a626 100644
--- a/Makefile
+++ b/Makefile
@@ -1545,6 +1545,21 @@ prepare: tools/bpf/resolve_btfids
endif
endif
+# tools/bootconfig renders the embedded bootconfig into a cmdline at build time.
+ifdef CONFIG_BOOT_CONFIG_EMBED_CMDLINE
+prepare: tools/bootconfig
+endif
+
+# tools/bootconfig is run on the build host during prepare, so force a host
+# binary here; its own Makefile keeps $(CC) for standalone and cross builds.
+# CROSS_COMPILE= is cleared so tools/scripts/Makefile.include does not inject
+# the target's --target=/--sysroot= flags into the host clang invocation under
+# LLVM=1 cross builds (which would produce a target binary that fails to exec).
+tools/bootconfig: FORCE
+ $(Q)mkdir -p $(objtree)/tools
+ $(Q)$(MAKE) O=$(abspath $(objtree)) subdir=tools -C $(srctree)/tools/ \
+ bootconfig CC=$(HOSTCC) CROSS_COMPILE=
+
# The tools build system is not a part of Kbuild and tends to introduce
# its own unique issues. If you need to integrate a new tool into Kbuild,
# please consider locating that tool outside the tools/ tree and using the
diff --git a/init/Kconfig b/init/Kconfig
index 5230d4879b1c..743f54397190 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1566,6 +1566,42 @@ config BOOT_CONFIG_EMBED_FILE
This bootconfig will be used if there is no initrd or no other
bootconfig in the initrd.
+config ARCH_SUPPORTS_CMDLINE_FROM_BOOTCONFIG
+ bool
+ help
+ Silent symbol; no C code reads it directly. Architectures
+ select it once their setup_arch() calls
+ xbc_prepend_embedded_cmdline() before parse_early_param().
+ Its only role is to gate the user-visible
+ BOOT_CONFIG_EMBED_CMDLINE option per-arch, the same
+ ARCH_SUPPORTS_* idiom used by ARCH_SUPPORTS_CFI, etc.
+
+config BOOT_CONFIG_EMBED_CMDLINE
+ bool "Render embedded bootconfig as kernel cmdline at build time"
+ depends on BOOT_CONFIG_EMBED
+ depends on BOOT_CONFIG_EMBED_FILE != ""
+ depends on ARCH_SUPPORTS_CMDLINE_FROM_BOOTCONFIG
+ default n
+ help
+ Render the "kernel" subtree of the embedded bootconfig file into a
+ flat cmdline string at kernel build time and prepend it to
+ boot_command_line during early architecture setup. This makes
+ early_param() handlers (e.g. mem=, earlycon=, loglevel=) see the
+ values supplied via the embedded bootconfig.
+
+ The runtime bootconfig parser is unaffected, so tree-structured
+ consumers such as ftrace boot-time tracing keep working.
+
+ Note: when an initrd also carries a bootconfig, its "kernel"
+ subtree is still parsed at runtime, but the embedded "kernel"
+ keys remain in boot_command_line for parse_early_param() and
+ end up later than the initrd keys in saved_command_line, so
+ parse_args() last-wins favors the embedded values. If you need
+ initrd to override embedded kernel.* keys, leave this option
+ off.
+
+ If unsure, say N.
+
config CMDLINE_LOG_WRAP_IDEAL_LEN
int "Length to try to wrap the cmdline when logged at boot"
default 1021
diff --git a/lib/Makefile b/lib/Makefile
index 7f75cc6edf94..4ace86a5cb6d 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -273,6 +273,22 @@ filechk_defbconf = cat $(or $(real-prereqs), /dev/null)
$(obj)/default.bconf: $(CONFIG_BOOT_CONFIG_EMBED_FILE) FORCE
$(call filechk,defbconf)
+obj-$(CONFIG_BOOT_CONFIG_EMBED_CMDLINE) += embedded-cmdline.o
+$(obj)/embedded-cmdline.o: $(obj)/embedded_cmdline.bin
+
+# Render the bootconfig "kernel" subtree to a flat cmdline string using
+# the userspace tools/bootconfig parser (-C mode). The runtime prepend
+# helper enforces COMMAND_LINE_SIZE at boot, so no build-time size
+# check is performed here (COMMAND_LINE_SIZE is an arch header
+# constant, not a Kconfig value).
+quiet_cmd_render_cmdline = BCONF2C $@
+ cmd_render_cmdline = \
+ $(objtree)/tools/bootconfig/bootconfig -C $< > $@
+
+targets += embedded_cmdline.bin
+$(obj)/embedded_cmdline.bin: $(obj)/default.bconf $(objtree)/tools/bootconfig/bootconfig FORCE
+ $(call if_changed,render_cmdline)
+
obj-$(CONFIG_RBTREE_TEST) += rbtree_test.o
obj-$(CONFIG_INTERVAL_TREE_TEST) += interval_tree_test.o
diff --git a/lib/embedded-cmdline.S b/lib/embedded-cmdline.S
new file mode 100644
index 000000000000..bda81b4a42be
--- /dev/null
+++ b/lib/embedded-cmdline.S
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Embed the build-time-rendered bootconfig "kernel" subtree as a flat
+ * cmdline string. setup_arch() prepends this to boot_command_line on
+ * architectures that select ARCH_SUPPORTS_CMDLINE_FROM_BOOTCONFIG.
+ *
+ * Copyright (c) 2026 Meta Platforms, Inc. and affiliates
+ * Copyright (c) 2026 Breno Leitao <leitao@debian.org>
+ */
+ .section .init.rodata.embed_cmdline, "a", %progbits
+ .global embedded_kernel_cmdline
+embedded_kernel_cmdline:
+ .incbin "lib/embedded_cmdline.bin"
+ .byte 0
+ .global embedded_kernel_cmdline_end
+embedded_kernel_cmdline_end:
diff --git a/tools/bootconfig/Makefile b/tools/bootconfig/Makefile
index 90eb47c9d8de..4e82fd9553cd 100644
--- a/tools/bootconfig/Makefile
+++ b/tools/bootconfig/Makefile
@@ -15,7 +15,7 @@ override CFLAGS += -Wall -g -I$(CURDIR)/include
ALL_TARGETS := bootconfig
ALL_PROGRAMS := $(patsubst %,$(OUTPUT)%,$(ALL_TARGETS))
-all: $(ALL_PROGRAMS) test
+all: $(ALL_PROGRAMS)
$(OUTPUT)bootconfig: main.c include/linux/bootconfig.h $(LIBSRC)
$(CC) $(filter %.c,$^) $(CFLAGS) $(LDFLAGS) -o $@
--
2.53.0-Meta
^ permalink raw reply related
* [PATCH v4 4/7] bootconfig: clean build-time tools/bootconfig from make clean
From: Breno Leitao @ 2026-06-09 10:28 UTC (permalink / raw)
To: Masami Hiramatsu, Andrew Morton, Nathan Chancellor, paulmck,
Nicolas Schier
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, linux-kernel, linux-trace-kernel, linux-kbuild,
bpf, Breno Leitao, kernel-team
In-Reply-To: <20260609-bootconfig_using_tools-v4-0-73c463f03a97@debian.org>
The previous patch builds tools/bootconfig during 'make prepare' to
render the embedded bootconfig cmdline, but nothing removes it on
'make clean', leaving the compiled tool and its objects behind.
Wire a bootconfig_clean hook into the top-level clean target so the
compiled tool and its objects are removed by make clean, matching the
prepare-wired tools/objtool and tools/bpf/resolve_btfids.
The hook runs tools/bootconfig's Makefile via $(MAKE), which the kernel
build invokes with -rR (MAKEFLAGS += -rR). -rR drops the built-in $(RM)
variable, so the existing "$(RM) -f ..." clean recipe would expand to a
bare "-f ..." and fail. Spell the recipe with a literal "rm -f" so it
keeps working both standalone and when invoked from Kbuild.
Signed-off-by: Breno Leitao <leitao@debian.org>
---
Makefile | 13 ++++++++++++-
tools/bootconfig/Makefile | 2 +-
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile
index a7abb3f9a626..a6e13fa1c1dc 100644
--- a/Makefile
+++ b/Makefile
@@ -1586,6 +1586,17 @@ ifneq ($(wildcard $(objtool_O)),)
$(Q)$(MAKE) -sC $(abs_srctree)/tools/objtool O=$(objtool_O) srctree=$(abs_srctree) $(patsubst objtool_%,%,$@)
endif
+PHONY += bootconfig_clean
+
+bootconfig_O = $(abspath $(objtree))/tools/bootconfig
+
+# tools/bootconfig is only built (via the prepare hook above) when
+# CONFIG_BOOT_CONFIG_EMBED_CMDLINE is set; skip its clean otherwise.
+bootconfig_clean:
+ifneq ($(wildcard $(bootconfig_O)),)
+ $(Q)$(MAKE) -sC $(srctree)/tools/bootconfig O=$(bootconfig_O) clean
+endif
+
tools/: FORCE
$(Q)mkdir -p $(objtree)/tools
$(Q)$(MAKE) O=$(abspath $(objtree)) subdir=tools -C $(srctree)/tools/
@@ -1756,7 +1767,7 @@ vmlinuxclean:
$(Q)$(CONFIG_SHELL) $(srctree)/scripts/link-vmlinux.sh clean
$(Q)$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) clean)
-clean: archclean vmlinuxclean resolve_btfids_clean objtool_clean
+clean: archclean vmlinuxclean resolve_btfids_clean objtool_clean bootconfig_clean
# mrproper - Delete all generated files, including .config
#
diff --git a/tools/bootconfig/Makefile b/tools/bootconfig/Makefile
index 4e82fd9553cd..3cb8066d5141 100644
--- a/tools/bootconfig/Makefile
+++ b/tools/bootconfig/Makefile
@@ -27,4 +27,4 @@ install: $(ALL_PROGRAMS)
install $(OUTPUT)bootconfig $(DESTDIR)$(bindir)
clean:
- $(RM) -f $(OUTPUT)*.o $(ALL_PROGRAMS)
+ rm -f $(OUTPUT)*.o $(ALL_PROGRAMS)
--
2.53.0-Meta
^ permalink raw reply related
* [PATCH v4 5/7] bootconfig: add xbc_prepend_embedded_cmdline() helper
From: Breno Leitao @ 2026-06-09 10:28 UTC (permalink / raw)
To: Masami Hiramatsu, Andrew Morton, Nathan Chancellor, paulmck,
Nicolas Schier
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, linux-kernel, linux-trace-kernel, linux-kbuild,
bpf, Breno Leitao, kernel-team
In-Reply-To: <20260609-bootconfig_using_tools-v4-0-73c463f03a97@debian.org>
Add a helper that prepends the build-time-rendered embedded bootconfig
"kernel" subtree (embedded_kernel_cmdline[] from embedded-cmdline.S) to
a cmdline buffer with a separating space. Architectures call this from
setup_arch() before parse_early_param() so early_param() handlers
(mem=, earlycon=, loglevel=, ...) see values supplied via the embedded
bootconfig.
The in-place prepend (shift the existing string right, then drop the
embedded string in front) is factored into a small str_prepend() helper.
On overflow the helper logs an error and leaves the cmdline untouched
rather than panicking. Booting without the embedded values is better
than refusing to boot, and the error tells the user why their embedded
keys are missing.
The helper records whether it actually prepended, exposed via
xbc_embedded_cmdline_applied(). setup_boot_config() uses this to decide
whether the runtime "kernel" render would duplicate keys already folded
into boot_command_line.
When CONFIG_BOOT_CONFIG_EMBED_CMDLINE=n, the public declaration in
<linux/bootconfig.h> resolves to a no-op stub so callers compile
unchanged.
Signed-off-by: Breno Leitao <leitao@debian.org>
---
include/linux/bootconfig.h | 9 ++++++
lib/bootconfig.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 87 insertions(+)
diff --git a/include/linux/bootconfig.h b/include/linux/bootconfig.h
index 1c7f3b74ffcf..c186137f87ac 100644
--- a/include/linux/bootconfig.h
+++ b/include/linux/bootconfig.h
@@ -308,4 +308,13 @@ static inline const char *xbc_get_embedded_bootconfig(size_t *size)
}
#endif
+/* Build-time-rendered bootconfig cmdline prepended in setup_arch() */
+#ifdef CONFIG_BOOT_CONFIG_EMBED_CMDLINE
+void __init xbc_prepend_embedded_cmdline(char *dst, size_t size);
+bool __init xbc_embedded_cmdline_applied(void);
+#else
+static inline void xbc_prepend_embedded_cmdline(char *dst, size_t size) { }
+static inline bool xbc_embedded_cmdline_applied(void) { return false; }
+#endif
+
#endif
diff --git a/lib/bootconfig.c b/lib/bootconfig.c
index 926094d97397..f66be0b2dc24 100644
--- a/lib/bootconfig.c
+++ b/lib/bootconfig.c
@@ -19,6 +19,7 @@
#include <linux/errno.h>
#include <linux/cache.h>
#include <linux/compiler.h>
+#include <linux/printk.h>
#include <linux/sprintf.h>
#include <linux/memblock.h>
#include <linux/string.h>
@@ -34,6 +35,83 @@ const char * __init xbc_get_embedded_bootconfig(size_t *size)
return (*size) ? embedded_bootconfig_data : NULL;
}
#endif
+
+#ifdef CONFIG_BOOT_CONFIG_EMBED_CMDLINE
+/* embedded_kernel_cmdline is defined in embedded-cmdline.S */
+extern __visible const char embedded_kernel_cmdline[];
+extern __visible const char embedded_kernel_cmdline_end[];
+
+/* Set once the embedded cmdline has actually been prepended. */
+static bool xbc_cmdline_applied __initdata;
+
+/*
+ * str_prepend() - Prepend @src in front of the string in @dst, in place
+ * @dst: NUL-terminated destination buffer, currently @dst_len bytes long
+ * @dst_len: length of the current @dst string (excluding its NUL)
+ * @src: bytes to prepend (not NUL-terminated)
+ * @src_len: number of bytes from @src to prepend
+ *
+ * The caller must guarantee @dst has room for src_len + dst_len + 1 bytes.
+ * Moving dst_len + 1 bytes carries @dst's NUL terminator too, so an empty
+ * @dst needs no special case.
+ */
+static void __init str_prepend(char *dst, size_t dst_len,
+ const char *src, size_t src_len)
+{
+ memmove(dst + src_len, dst, dst_len + 1);
+ memcpy(dst, src, src_len);
+}
+
+/**
+ * xbc_prepend_embedded_cmdline() - Prepend embedded bootconfig cmdline
+ * @dst: cmdline buffer to prepend into (must already contain a NUL byte)
+ * @size: total capacity of @dst in bytes
+ *
+ * Prepend the build-time-rendered "kernel" subtree of the embedded
+ * bootconfig to @dst. The rendered string already ends with a single
+ * space (the xbc_snprint_cmdline() invariant), which serves as the
+ * separator between the embedded keys and any existing content of @dst.
+ * On overflow, log an error and leave @dst untouched rather than
+ * silently truncating: booting without the embedded values is better
+ * than refusing to boot, and the error message tells the user why
+ * their embedded keys are missing.
+ *
+ * Intended to be called from setup_arch() before parse_early_param() so
+ * that early_param() handlers see the embedded values.
+ */
+void __init xbc_prepend_embedded_cmdline(char *dst, size_t size)
+{
+ size_t embed_len = embedded_kernel_cmdline_end - embedded_kernel_cmdline;
+ size_t dst_len;
+
+ if (!size || embed_len <= 1) /* trailing NUL only */
+ return;
+ embed_len--; /* exclude trailing NUL byte */
+
+ dst_len = strnlen(dst, size);
+ if (embed_len + dst_len + 1 > size) {
+ pr_err("embedded bootconfig cmdline (%zu bytes) does not fit in COMMAND_LINE_SIZE with %zu bytes already used; ignoring embedded values\n",
+ embed_len, dst_len);
+ return;
+ }
+
+ str_prepend(dst, dst_len, embedded_kernel_cmdline, embed_len);
+ xbc_cmdline_applied = true;
+}
+
+/**
+ * xbc_embedded_cmdline_applied() - Did the embedded cmdline get prepended?
+ *
+ * Return true if xbc_prepend_embedded_cmdline() actually prepended the
+ * embedded "kernel" subtree. setup_boot_config() uses this to avoid
+ * rendering the same keys a second time.
+ */
+bool __init xbc_embedded_cmdline_applied(void)
+{
+ return xbc_cmdline_applied;
+}
+#endif
+
#endif
/*
--
2.53.0-Meta
^ permalink raw reply related
* [PATCH v4 6/7] Documentation: bootconfig: document build-time cmdline rendering
From: Breno Leitao @ 2026-06-09 10:28 UTC (permalink / raw)
To: Masami Hiramatsu, Andrew Morton, Nathan Chancellor, paulmck,
Nicolas Schier
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, linux-kernel, linux-trace-kernel, linux-kbuild,
bpf, Breno Leitao, kernel-team
In-Reply-To: <20260609-bootconfig_using_tools-v4-0-73c463f03a97@debian.org>
Add a section describing CONFIG_BOOT_CONFIG_EMBED_CMDLINE: what it
does (renders the embedded "kernel" subtree to a flat cmdline at
build time so early_param() handlers see the values), what it
requires (BOOT_CONFIG_EMBED, a non-empty BOOT_CONFIG_EMBED_FILE,
and ARCH_SUPPORTS_CMDLINE_FROM_BOOTCONFIG -- currently x86 only),
the bootconfig opt-in semantics, the initrd-vs-embedded precedence,
and the soft-error overflow behavior.
Signed-off-by: Breno Leitao <leitao@debian.org>
---
Documentation/admin-guide/bootconfig.rst | 46 ++++++++++++++++++++++++++++++++
1 file changed, 46 insertions(+)
diff --git a/Documentation/admin-guide/bootconfig.rst b/Documentation/admin-guide/bootconfig.rst
index f712758472d5..f371e5cdc974 100644
--- a/Documentation/admin-guide/bootconfig.rst
+++ b/Documentation/admin-guide/bootconfig.rst
@@ -234,6 +234,52 @@ Kconfig option selected.
Note that even if you set this option, you can override the embedded
bootconfig by another bootconfig which attached to the initrd.
+Rendering Embedded kernel.* Keys at Build Time
+----------------------------------------------
+
+By default, the embedded bootconfig (``CONFIG_BOOT_CONFIG_EMBED=y``) is
+parsed at runtime, after ``parse_early_param()`` has already run. Early
+parameter handlers (``mem=``, ``earlycon=``, ``loglevel=``, ...) therefore
+cannot see values supplied via the embedded ``kernel`` subtree.
+
+``CONFIG_BOOT_CONFIG_EMBED_CMDLINE`` resolves this by rendering the
+``kernel`` subtree of ``CONFIG_BOOT_CONFIG_EMBED_FILE`` into a flat cmdline
+string at kernel build time (via ``tools/bootconfig -C``) and prepending
+it to ``boot_command_line`` during early architecture setup, so the keys
+are visible to ``parse_early_param()``.
+
+The option requires ``CONFIG_BOOT_CONFIG_EMBED=y``, a non-empty
+``CONFIG_BOOT_CONFIG_EMBED_FILE``, and an architecture that selects
+``CONFIG_ARCH_SUPPORTS_CMDLINE_FROM_BOOTCONFIG``. Currently only x86
+selects it; on other architectures the embedded bootconfig still works,
+but only through the late runtime parser.
+
+The same ``bootconfig`` opt-in applies as elsewhere: the rendered keys
+are prepended only when ``bootconfig`` (in any form) appears on the
+kernel command line, or when ``CONFIG_BOOT_CONFIG_FORCE`` is set, which
+defaults to ``y`` when ``CONFIG_BOOT_CONFIG_EMBED`` is set.
+
+For example, given::
+
+ kernel {
+ loglevel = 7
+ mem = 4G
+ }
+
+the kernel boots as if ``loglevel=7 mem=4G`` had been prepended to the
+bootloader command line, with the values visible to early-parsed
+handlers. Comma-separated values are still expanded into multiple
+cmdline entries per the bootconfig array convention -- the embedded
+``kernel.earlycon = "uart8250,io,0x3f8"`` must be quoted to land as a
+single ``earlycon=`` entry, exactly as for the runtime parser.
+
+If an initrd carries its own bootconfig, the runtime parser still
+processes it; ``parse_args()`` last-wins means the initrd's ``kernel``
+keys override the build-time-rendered ones. If the rendered string
+would not fit in ``COMMAND_LINE_SIZE`` together with the existing
+command line, the prepend is skipped and an error is logged, so an
+oversized embedded bootconfig cannot brick a boot.
+
Kernel parameters via Boot Config
=================================
--
2.53.0-Meta
^ permalink raw reply related
* [PATCH v4 7/7] x86/setup: prepend embedded bootconfig cmdline before parse_early_param
From: Breno Leitao @ 2026-06-09 10:28 UTC (permalink / raw)
To: Masami Hiramatsu, Andrew Morton, Nathan Chancellor, paulmck,
Nicolas Schier
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, linux-kernel, linux-trace-kernel, linux-kbuild,
bpf, Breno Leitao, kernel-team
In-Reply-To: <20260609-bootconfig_using_tools-v4-0-73c463f03a97@debian.org>
Call xbc_prepend_embedded_cmdline() in setup_arch() right after the
CONFIG_CMDLINE merge and before strscpy(command_line, ...) so the
build-time-rendered embedded bootconfig "kernel" subtree is part of
boot_command_line by the time parse_early_param() runs. early_param()
handlers (mem=, earlycon=, loglevel=, ...) now see values supplied via
CONFIG_BOOT_CONFIG_EMBED_FILE without parsing bootconfig at runtime.
Gate the prepend on the same opt-in the runtime parser uses: prepend
when "bootconfig" is present on the command line, or when
CONFIG_BOOT_CONFIG_FORCE is set. setup_boot_config()'s parse_args()
loop treats any presence of the "bootconfig" key as opt-in regardless
of value, so check both cmdline_find_option_bool() (matches the bare
key) and cmdline_find_option() (matches "bootconfig=<anything>").
Without the latter check, "bootconfig=0" would skip the early prepend
yet still trigger the late runtime apply, leaving the embedded keys
invisible to early_param() but applied to saved_command_line.
Signed-off-by: Breno Leitao <leitao@debian.org>
---
arch/x86/Kconfig | 1 +
arch/x86/kernel/setup.c | 27 +++++++++++++++++++++++++++
init/main.c | 25 ++++++++++++++++++++++---
3 files changed, 50 insertions(+), 3 deletions(-)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 0de23e647197..8ab11199c16d 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -127,6 +127,7 @@ config X86
select ARCH_SUPPORTS_NUMA_BALANCING if X86_64
select ARCH_SUPPORTS_KMAP_LOCAL_FORCE_MAP if NR_CPUS <= 4096
select ARCH_SUPPORTS_CFI if X86_64
+ select ARCH_SUPPORTS_CMDLINE_FROM_BOOTCONFIG
select ARCH_USES_CFI_TRAPS if X86_64 && CFI
select ARCH_SUPPORTS_LTO_CLANG
select ARCH_SUPPORTS_LTO_CLANG_THIN
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index 46882ce79c3a..d69ba84c203f 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -6,6 +6,7 @@
* parts of early kernel initialization.
*/
#include <linux/acpi.h>
+#include <linux/bootconfig.h>
#include <linux/console.h>
#include <linux/cpu.h>
#include <linux/crash_dump.h>
@@ -36,6 +37,7 @@
#include <asm/bios_ebda.h>
#include <asm/bugs.h>
#include <asm/cacheinfo.h>
+#include <asm/cmdline.h>
#include <asm/coco.h>
#include <asm/cpu.h>
#include <asm/efi.h>
@@ -924,6 +926,31 @@ void __init setup_arch(char **cmdline_p)
builtin_cmdline_added = true;
#endif
+ /*
+ * Match the runtime bootconfig parser's opt-in: only fold the
+ * embedded kernel.* keys into the cmdline when "bootconfig" is
+ * present on the command line, or CONFIG_BOOT_CONFIG_FORCE is set.
+ * setup_boot_config()'s parse_args() loop treats any presence of
+ * the "bootconfig" key as opt-in (bare, =0, =1, ...), so check both
+ * forms here: cmdline_find_option_bool() matches the bare key,
+ * cmdline_find_option() matches "bootconfig=<anything>". Without
+ * the second check, "bootconfig=0" would skip the early prepend
+ * but still trigger the late runtime apply -- a split-brain state.
+ * CONFIG_BOOT_CONFIG_FORCE defaults to y when BOOT_CONFIG_EMBED is
+ * set, so on the default config the embedded keys are applied
+ * unconditionally.
+ */
+ {
+ char buf[8];
+
+ if (IS_ENABLED(CONFIG_BOOT_CONFIG_FORCE) ||
+ cmdline_find_option_bool(boot_command_line, "bootconfig") ||
+ cmdline_find_option(boot_command_line, "bootconfig",
+ buf, sizeof(buf)) >= 0)
+ xbc_prepend_embedded_cmdline(boot_command_line,
+ COMMAND_LINE_SIZE);
+ }
+
strscpy(command_line, boot_command_line, COMMAND_LINE_SIZE);
*cmdline_p = command_line;
diff --git a/init/main.c b/init/main.c
index e363232b428b..2ecb6aa536dd 100644
--- a/init/main.c
+++ b/init/main.c
@@ -378,12 +378,15 @@ static void __init setup_boot_config(void)
int pos, ret;
size_t size;
char *err;
+ bool from_embedded = false;
/* Cut out the bootconfig data even if we have no bootconfig option */
data = get_boot_config_from_initrd(&size);
/* If there is no bootconfig in initrd, try embedded one. */
- if (!data)
+ if (!data) {
data = xbc_get_embedded_bootconfig(&size);
+ from_embedded = true;
+ }
strscpy(tmp_cmdline, boot_command_line, COMMAND_LINE_SIZE);
err = parse_args("bootconfig", tmp_cmdline, NULL, 0, 0, 0, NULL,
@@ -421,8 +424,24 @@ static void __init setup_boot_config(void)
} else {
xbc_get_info(&ret, NULL);
pr_info("Load bootconfig: %ld bytes %d nodes\n", (long)size, ret);
- /* keys starting with "kernel." are passed via cmdline */
- extra_command_line = xbc_make_cmdline("kernel");
+ /*
+ * keys starting with "kernel." are passed via cmdline. When
+ * this bootconfig came from the embedded source and
+ * setup_arch() already prepended the rendered "kernel" subtree
+ * to boot_command_line, rendering again here would duplicate
+ * the keys in saved_command_line and make accumulating handlers
+ * (console=, earlycon=, ...) re-register the same value. Skip
+ * only when the prepend really happened.
+ *
+ * On arches that do not select ARCH_SUPPORTS_CMDLINE_FROM_BOOTCONFIG,
+ * CONFIG_BOOT_CONFIG_EMBED_CMDLINE is unselectable and
+ * xbc_embedded_cmdline_applied() collapses to a stub returning
+ * false, so this path still runs and the embedded "kernel"
+ * keys reach the cmdline via the runtime parser exactly as
+ * before this series.
+ */
+ if (!from_embedded || !xbc_embedded_cmdline_applied())
+ extra_command_line = xbc_make_cmdline("kernel");
/* Also, "init." keys are init arguments */
extra_init_args = xbc_make_cmdline("init");
}
--
2.53.0-Meta
^ permalink raw reply related
* Re: [PATCH mm-unstable v19 11/14] mm/khugepaged: Introduce mTHP collapse support
From: Lance Yang @ 2026-06-09 10:36 UTC (permalink / raw)
To: Nico Pache, David Hildenbrand (Arm)
Cc: linux-doc, linux-kernel, linux-mm, linux-trace-kernel, aarcange,
akpm, anshuman.khandual, apopple, baohua, baolin.wang, byungchul,
catalin.marinas, cl, corbet, dave.hansen, dev.jain, gourry,
hannes, hughd, jack, jackmanb, jannh, jglisse, joshua.hahnjy, kas,
liam, ljs, mathieu.desnoyers, matthew.brost, mhiramat, mhocko,
peterx, pfalcato, rakie.kim, raquini, rdunlap, richard.weiyang,
rientjes, rostedt, rppt, ryan.roberts, shivankg, sunnanyong,
surenb, thomas.hellstrom, tiwai, usamaarif642, vbabka,
vishal.moola, wangkefeng.wang, will, willy, yang, ying.huang, ziy,
zokeefe
In-Reply-To: <CAA1CXcAhw8V+_dYcrqmtZ9ht4Pqz5PPB8EOcDrVCp4DA4y7pLg@mail.gmail.com>
On 2026/6/9 17:32, Nico Pache wrote:
> On Tue, Jun 9, 2026 at 3:26 AM David Hildenbrand (Arm) <david@kernel.org> wrote:
>>
>> On 6/9/26 11:06, Nico Pache wrote:
>>> On Mon, Jun 8, 2026 at 8:57 AM David Hildenbrand (Arm) <david@kernel.org> wrote:
>>>>
>>>> On 6/6/26 12:28, Lance Yang wrote:
>>>>>
>>>>>
>>>>> Looks broken for swap PTEs in PMD collapse ...
>>>>>
>>>>> collapse_scan_pmd() allows them up to max_ptes_swap and record them in
>>>>> unmapped, but they don't get a bit in mthp_present_ptes. And then
>>>>> mthp_collapse() does the check above:
>>>>
>>>> Right. I assumed this is implicitly handled by the optimization in collapse_scan_pmd:
>>>>
>>>> if (enabled_orders != BIT(HPAGE_PMD_ORDER))
>>>> max_ptes_none = KHUGEPAGED_MAX_PTES_LIMIT;
>>>>
>>>> But we perform the check a second time.
>>>>
>>>>>
>>>>> nr_occupied_ptes >= nr_ptes - max_ptes_none
>>>>>
>>>>> So max_ptes_none=0 + 511 present PTEs + one allowed swap PTE won't even
>>>>> call collapse_huge_page() for PMD order.
>>>>>
>>>>> Shouldn't we account for them in the PMD-order check? Something like:
>>>>>
>>>>> if (is_pmd_order(order))
>>>>> nr_occupied_ptes += unmapped;
>>>
>>> This solution seems good for a temporary fixup. but longterm we may
>>> want something else. I'm still not sure how we plan on supporting
>>> swapin without causing creep. So I'd be ok with adding a fix for
>>> legacy PMD behavior until we know how to handle mTHP creep correctly.
>>>
>>>> As an alternative, we could either 1) skip the check there for
>>>> pmd order (as the check was already done); or 2) introduce+maintain
>>>> a bitmap that tracks non-present PTEs.
>>>>
>>>> @@ -1475,7 +1477,9 @@ static enum scan_result mthp_collapse(struct mm_struct *mm,
>>>> nr_occupied_ptes = bitmap_weight_from(cc->mthp_present_ptes, offset,
>>>> offset + nr_ptes);
>>>>
>>>> - if (nr_occupied_ptes >= nr_ptes - max_ptes_none) {
>>>> + /* Check was already done in the caller. */
>>>> + if (is_pmd_order(order) ||
>>>> + nr_occupied_ptes >= nr_ptes - max_ptes_none) {
>>>> enum scan_result ret;
>>>>
>>>> collapse_address = address + offset * PAGE_SIZE;
>>>>
>>>> 2) would probably be cleanest long-term.
>>>
>>> That would be best for future swapin support in mTHP, but I still
>>> don't think it solves the creep issue.
>>
>> It wouldn't, we'd simply maintain the state we collect + rely on in separate
>> bitmaps. On swapin, we'd have to update/refresh bitmaps I guess.
>
> Yeah, I'm saying for the future, it obviously solves this issue here
> as well, but if we have positional tracking of the swapout, shared,
> and none PTEs, I think we can use this to determine whether the
> collapse would lead to creep. If we detect creep would happen it may
> be best to automatically collapse to the N+1 (or greater) candidate.
> Just thinking outloud here.
>
>>
>>> Perhaps we could combine the
>>> two bitmaps to determine if it would make the future collapse eligible
>>> again? Not sure but ill start thinking about it.
>>>
>>> Should I send a fixup for this using Lance's solution? Or does Lance
>>> want to send a patch out with the fixes tag?
>>
>> If Lance could send a fixup, explaining the situation, that would be nice.
Sure, happy to send a fixup :P
Should I send it as a fixup to be folded into this patch, or as a
separate patch with a Fixes: tag?
Will get one out soon :)
> OK, I'd appreciate that :)
Cheers!
^ permalink raw reply
* Re: [PATCH mm-unstable v19 11/14] mm/khugepaged: Introduce mTHP collapse support
From: Nico Pache @ 2026-06-09 10:50 UTC (permalink / raw)
To: Lance Yang
Cc: David Hildenbrand (Arm), linux-doc, linux-kernel, linux-mm,
linux-trace-kernel, aarcange, akpm, anshuman.khandual, apopple,
baohua, baolin.wang, byungchul, catalin.marinas, cl, corbet,
dave.hansen, dev.jain, gourry, hannes, hughd, jack, jackmanb,
jannh, jglisse, joshua.hahnjy, kas, liam, ljs, mathieu.desnoyers,
matthew.brost, mhiramat, mhocko, peterx, pfalcato, rakie.kim,
raquini, rdunlap, richard.weiyang, rientjes, rostedt, rppt,
ryan.roberts, shivankg, sunnanyong, surenb, thomas.hellstrom,
tiwai, usamaarif642, vbabka, vishal.moola, wangkefeng.wang, will,
willy, yang, ying.huang, ziy, zokeefe
In-Reply-To: <7e36f7f0-b4d5-41c9-b399-9e0079907d33@linux.dev>
On Tue, Jun 9, 2026 at 4:37 AM Lance Yang <lance.yang@linux.dev> wrote:
>
>
>
> On 2026/6/9 17:32, Nico Pache wrote:
> > On Tue, Jun 9, 2026 at 3:26 AM David Hildenbrand (Arm) <david@kernel.org> wrote:
> >>
> >> On 6/9/26 11:06, Nico Pache wrote:
> >>> On Mon, Jun 8, 2026 at 8:57 AM David Hildenbrand (Arm) <david@kernel.org> wrote:
> >>>>
> >>>> On 6/6/26 12:28, Lance Yang wrote:
> >>>>>
> >>>>>
> >>>>> Looks broken for swap PTEs in PMD collapse ...
> >>>>>
> >>>>> collapse_scan_pmd() allows them up to max_ptes_swap and record them in
> >>>>> unmapped, but they don't get a bit in mthp_present_ptes. And then
> >>>>> mthp_collapse() does the check above:
> >>>>
> >>>> Right. I assumed this is implicitly handled by the optimization in collapse_scan_pmd:
> >>>>
> >>>> if (enabled_orders != BIT(HPAGE_PMD_ORDER))
> >>>> max_ptes_none = KHUGEPAGED_MAX_PTES_LIMIT;
> >>>>
> >>>> But we perform the check a second time.
> >>>>
> >>>>>
> >>>>> nr_occupied_ptes >= nr_ptes - max_ptes_none
> >>>>>
> >>>>> So max_ptes_none=0 + 511 present PTEs + one allowed swap PTE won't even
> >>>>> call collapse_huge_page() for PMD order.
> >>>>>
> >>>>> Shouldn't we account for them in the PMD-order check? Something like:
> >>>>>
> >>>>> if (is_pmd_order(order))
> >>>>> nr_occupied_ptes += unmapped;
> >>>
> >>> This solution seems good for a temporary fixup. but longterm we may
> >>> want something else. I'm still not sure how we plan on supporting
> >>> swapin without causing creep. So I'd be ok with adding a fix for
> >>> legacy PMD behavior until we know how to handle mTHP creep correctly.
> >>>
> >>>> As an alternative, we could either 1) skip the check there for
> >>>> pmd order (as the check was already done); or 2) introduce+maintain
> >>>> a bitmap that tracks non-present PTEs.
> >>>>
> >>>> @@ -1475,7 +1477,9 @@ static enum scan_result mthp_collapse(struct mm_struct *mm,
> >>>> nr_occupied_ptes = bitmap_weight_from(cc->mthp_present_ptes, offset,
> >>>> offset + nr_ptes);
> >>>>
> >>>> - if (nr_occupied_ptes >= nr_ptes - max_ptes_none) {
> >>>> + /* Check was already done in the caller. */
> >>>> + if (is_pmd_order(order) ||
> >>>> + nr_occupied_ptes >= nr_ptes - max_ptes_none) {
> >>>> enum scan_result ret;
> >>>>
> >>>> collapse_address = address + offset * PAGE_SIZE;
> >>>>
> >>>> 2) would probably be cleanest long-term.
> >>>
> >>> That would be best for future swapin support in mTHP, but I still
> >>> don't think it solves the creep issue.
> >>
> >> It wouldn't, we'd simply maintain the state we collect + rely on in separate
> >> bitmaps. On swapin, we'd have to update/refresh bitmaps I guess.
> >
> > Yeah, I'm saying for the future, it obviously solves this issue here
> > as well, but if we have positional tracking of the swapout, shared,
> > and none PTEs, I think we can use this to determine whether the
> > collapse would lead to creep. If we detect creep would happen it may
> > be best to automatically collapse to the N+1 (or greater) candidate.
> > Just thinking outloud here.
> >
> >>
> >>> Perhaps we could combine the
> >>> two bitmaps to determine if it would make the future collapse eligible
> >>> again? Not sure but ill start thinking about it.
> >>>
> >>> Should I send a fixup for this using Lance's solution? Or does Lance
> >>> want to send a patch out with the fixes tag?
> >>
> >> If Lance could send a fixup, explaining the situation, that would be nice.
>
> Sure, happy to send a fixup :P
>
> Should I send it as a fixup to be folded into this patch, or as a
> separate patch with a Fixes: tag?
Id assume a seperate patch so you can keep credit for the discovery :)
Thank you for all the review you provided on this series, its been
really helpful!
-- Nico
>
> Will get one out soon :)
>
> > OK, I'd appreciate that :)
>
> Cheers!
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox