* Re: [PATCH 1/2] mm/khugepaged: do synchronous writeback for MADV_COLLAPSE
From: Matthew Wilcox @ 2025-11-10 14:28 UTC (permalink / raw)
To: Garg, Shivank
Cc: Lorenzo Stoakes, Andrew Morton, David Hildenbrand, Zi Yan,
Baolin Wang, Liam R . Howlett, Nico Pache, Ryan Roberts, Dev Jain,
Barry Song, Lance Yang, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Zach O'Keefe, linux-mm, linux-kernel,
linux-trace-kernel, Branden Moore
In-Reply-To: <39c8a4ed-53b5-48e3-baea-f67cc1e8be4f@amd.com>
On Mon, Nov 10, 2025 at 07:50:17PM +0530, Garg, Shivank wrote:
> The issue is copying those binary to a freshly mounted filesystem.
> The page cache folios remain dirty until background writeback completes.
>
> Reproduces 100% for me: fresh XFS/EXT4 mount -> copy binary -> execute -> MADV_COLLAPSE fails.
Yes, but this is an uncommon thing to do. Really, it's the kind of
thing you do when you're testing something (like, whether ext4 supports
large folios, and whether that yields a performance improvement).
It's more reasonable to change userspace than the kernel to solve this
problem you're having.
^ permalink raw reply
* Re: [PATCH 1/2] mm/khugepaged: do synchronous writeback for MADV_COLLAPSE
From: Lorenzo Stoakes @ 2025-11-10 14:23 UTC (permalink / raw)
To: Garg, Shivank
Cc: Matthew Wilcox, Andrew Morton, David Hildenbrand, Zi Yan,
Baolin Wang, Liam R . Howlett, Nico Pache, Ryan Roberts, Dev Jain,
Barry Song, Lance Yang, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Zach O'Keefe, linux-mm, linux-kernel,
linux-trace-kernel, Branden Moore
In-Reply-To: <39c8a4ed-53b5-48e3-baea-f67cc1e8be4f@amd.com>
On Mon, Nov 10, 2025 at 07:50:17PM +0530, Garg, Shivank wrote:
>
>
> On 11/10/2025 7:22 PM, Lorenzo Stoakes wrote:
> > On Mon, Nov 10, 2025 at 01:47:10PM +0000, Matthew Wilcox wrote:
> >> On Mon, Nov 10, 2025 at 11:32:53AM +0000, Shivank Garg wrote:
> >>> When MADV_COLLAPSE is called on file-backed mappings (e.g., executable
> >>> text sections), the pages may still be dirty from recent writes. The
> >>
> >> That explanation derails my brain entirely. Text isn't writable! How
> >> can the pages be dirty and file-backed text?
> >
> > Because they just compiled it and it's not been written back to disk yet :)
>
> Hi,
>
> It's not about compilation. My binaries are already compiled.
>
> The issue is copying those binary to a freshly mounted filesystem.
> The page cache folios remain dirty until background writeback completes.
>
> Reproduces 100% for me: fresh XFS/EXT4 mount -> copy binary -> execute -> MADV_COLLAPSE fails.
>
> Detail logs: https://lore.kernel.org/all/0b84865c-5b23-4be6-9902-af9d5e63c182@amd.com
It amounts to the same thing unless I'm missing something? You've done something
to write out the file and then immediately execute before writeback completes.
>
> Thanks,
> Shivank
Thanks, Lorenzo
^ permalink raw reply
* Re: [PATCH 1/2] mm/khugepaged: do synchronous writeback for MADV_COLLAPSE
From: Garg, Shivank @ 2025-11-10 14:20 UTC (permalink / raw)
To: Lorenzo Stoakes, Matthew Wilcox
Cc: Andrew Morton, David Hildenbrand, Zi Yan, Baolin Wang,
Liam R . Howlett, Nico Pache, Ryan Roberts, Dev Jain, Barry Song,
Lance Yang, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Zach O'Keefe, linux-mm, linux-kernel, linux-trace-kernel,
Branden Moore
In-Reply-To: <b7313a45-c36e-4390-a0b8-46f412474f86@lucifer.local>
On 11/10/2025 7:22 PM, Lorenzo Stoakes wrote:
> On Mon, Nov 10, 2025 at 01:47:10PM +0000, Matthew Wilcox wrote:
>> On Mon, Nov 10, 2025 at 11:32:53AM +0000, Shivank Garg wrote:
>>> When MADV_COLLAPSE is called on file-backed mappings (e.g., executable
>>> text sections), the pages may still be dirty from recent writes. The
>>
>> That explanation derails my brain entirely. Text isn't writable! How
>> can the pages be dirty and file-backed text?
>
> Because they just compiled it and it's not been written back to disk yet :)
Hi,
It's not about compilation. My binaries are already compiled.
The issue is copying those binary to a freshly mounted filesystem.
The page cache folios remain dirty until background writeback completes.
Reproduces 100% for me: fresh XFS/EXT4 mount -> copy binary -> execute -> MADV_COLLAPSE fails.
Detail logs: https://lore.kernel.org/all/0b84865c-5b23-4be6-9902-af9d5e63c182@amd.com
Thanks,
Shivank
^ permalink raw reply
* Re: [PATCH 1/2] mm/khugepaged: do synchronous writeback for MADV_COLLAPSE
From: Matthew Wilcox @ 2025-11-10 14:17 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Shivank Garg, Andrew Morton, David Hildenbrand, Zi Yan,
Baolin Wang, Liam R . Howlett, Nico Pache, Ryan Roberts, Dev Jain,
Barry Song, Lance Yang, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Zach O'Keefe, linux-mm, linux-kernel,
linux-trace-kernel, Branden Moore
In-Reply-To: <b7313a45-c36e-4390-a0b8-46f412474f86@lucifer.local>
On Mon, Nov 10, 2025 at 01:52:46PM +0000, Lorenzo Stoakes wrote:
> On Mon, Nov 10, 2025 at 01:47:10PM +0000, Matthew Wilcox wrote:
> > On Mon, Nov 10, 2025 at 11:32:53AM +0000, Shivank Garg wrote:
> > > When MADV_COLLAPSE is called on file-backed mappings (e.g., executable
> > > text sections), the pages may still be dirty from recent writes. The
> >
> > That explanation derails my brain entirely. Text isn't writable! How
> > can the pages be dirty and file-backed text?
>
> Because they just compiled it and it's not been written back to disk yet :)
So we can avoid these patches simply by changing userspace to call
fsync()? Cool!
^ permalink raw reply
* Re: [PATCH 1/2] mm/khugepaged: do synchronous writeback for MADV_COLLAPSE
From: Lorenzo Stoakes @ 2025-11-10 14:05 UTC (permalink / raw)
To: Matthew Wilcox
Cc: David Hildenbrand (Red Hat), Shivank Garg, Andrew Morton, Zi Yan,
Baolin Wang, Liam R . Howlett, Nico Pache, Ryan Roberts, Dev Jain,
Barry Song, Lance Yang, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Zach O'Keefe, linux-mm, linux-kernel,
linux-trace-kernel, Branden Moore
In-Reply-To: <aRHwv3XzI4XHcLPP@casper.infradead.org>
On Mon, Nov 10, 2025 at 02:03:43PM +0000, Matthew Wilcox wrote:
> On Mon, Nov 10, 2025 at 03:00:32PM +0100, David Hildenbrand (Red Hat) wrote:
> > On 10.11.25 14:47, Matthew Wilcox wrote:
> > > On Mon, Nov 10, 2025 at 11:32:53AM +0000, Shivank Garg wrote:
> > > > When MADV_COLLAPSE is called on file-backed mappings (e.g., executable
> > > > text sections), the pages may still be dirty from recent writes. The
> > >
> > > That explanation derails my brain entirely. Text isn't writable! How
> > > can the pages be dirty and file-backed text?
> >
> > Files are writable :)
>
> Well, if you're root ...
>
> -rwxr-xr-x 1 root root 158632 Jun 4 11:14 /bin/ls
>
gcc foo.c -o foo && ./foo
Is more the issue I think
^ permalink raw reply
* Re: [PATCH 1/2] mm/khugepaged: do synchronous writeback for MADV_COLLAPSE
From: Matthew Wilcox @ 2025-11-10 14:03 UTC (permalink / raw)
To: David Hildenbrand (Red Hat)
Cc: Shivank Garg, Andrew Morton, Lorenzo Stoakes, Zi Yan, Baolin Wang,
Liam R . Howlett, Nico Pache, Ryan Roberts, Dev Jain, Barry Song,
Lance Yang, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Zach O'Keefe, linux-mm, linux-kernel, linux-trace-kernel,
Branden Moore
In-Reply-To: <9b622374-778a-4459-aca9-4eb406a58516@gmail.com>
On Mon, Nov 10, 2025 at 03:00:32PM +0100, David Hildenbrand (Red Hat) wrote:
> On 10.11.25 14:47, Matthew Wilcox wrote:
> > On Mon, Nov 10, 2025 at 11:32:53AM +0000, Shivank Garg wrote:
> > > When MADV_COLLAPSE is called on file-backed mappings (e.g., executable
> > > text sections), the pages may still be dirty from recent writes. The
> >
> > That explanation derails my brain entirely. Text isn't writable! How
> > can the pages be dirty and file-backed text?
>
> Files are writable :)
Well, if you're root ...
-rwxr-xr-x 1 root root 158632 Jun 4 11:14 /bin/ls
^ permalink raw reply
* Re: [PATCH 1/2] mm/khugepaged: do synchronous writeback for MADV_COLLAPSE
From: David Hildenbrand (Red Hat) @ 2025-11-10 14:00 UTC (permalink / raw)
To: Matthew Wilcox, Shivank Garg
Cc: Andrew Morton, Lorenzo Stoakes, Zi Yan, Baolin Wang,
Liam R . Howlett, Nico Pache, Ryan Roberts, Dev Jain, Barry Song,
Lance Yang, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Zach O'Keefe, linux-mm, linux-kernel, linux-trace-kernel,
Branden Moore
In-Reply-To: <aRHs3pA2kOr_uD3k@casper.infradead.org>
On 10.11.25 14:47, Matthew Wilcox wrote:
> On Mon, Nov 10, 2025 at 11:32:53AM +0000, Shivank Garg wrote:
>> When MADV_COLLAPSE is called on file-backed mappings (e.g., executable
>> text sections), the pages may still be dirty from recent writes. The
>
> That explanation derails my brain entirely. Text isn't writable! How
> can the pages be dirty and file-backed text?
Files are writable :)
^ permalink raw reply
* Re: [PATCH 1/2] mm/khugepaged: do synchronous writeback for MADV_COLLAPSE
From: Lorenzo Stoakes @ 2025-11-10 13:36 UTC (permalink / raw)
To: Dev Jain
Cc: Shivank Garg, Andrew Morton, David Hildenbrand, Zi Yan,
Baolin Wang, Liam R . Howlett, Nico Pache, Ryan Roberts,
Barry Song, Lance Yang, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Zach O'Keefe, linux-mm, linux-kernel,
linux-trace-kernel, Branden Moore
In-Reply-To: <3f5f8a48-fa3b-4985-95e1-dd0ac21b5dcc@arm.com>
On Mon, Nov 10, 2025 at 06:54:57PM +0530, Dev Jain wrote:
>
> On 10/11/25 5:31 pm, Lorenzo Stoakes wrote:
> >
> > ofc you are addressing this with the !cc->is_khugepaged, but feels like we'd be
> > better off just doing it in madvise_collapse().
>
> I suppose the common case is that writeback is not needed, and I don't know what
> is the overhead of filemap_write_and_wait_range() in that case, but your
Low.
> suggestion will force that unconditional overhead and skip all the early exits
> possible in hpage_collapse_scan_file()?
Which are?
PMD-mapped folio at start of range, scan abort, non-LRU, spurious ref count
I don't think this matters.
And we're trading for putting _yet more_ logic that belongs elsewhere in the
wrong place.
I mean I'd actually be pretty good with us putting it literally in madvise.c,
but since we defer the collapse to khugepaged.c then madvise_collapse().
>
> > I wonder also if doing I/O without getting the mmap lock again and revalidating
> > is wise, as the state of things might have changed significantly.
> >
> > So maybe need a hugepage_vma_revalidate() as well?
>
> The file collapse path (apart from collapse_pte_mapped_thp) is solely concerned
> about doing the collapse in the page cache, for which information about the mm or
> the vma is not needed, that is why no locking is required. The get_file() in
The user has asked specifically to collapse pages in a VMA's range. Yes you can
go check the mapping of a pinned file which you're keeping pinned during this
operation (wise? Not sure it is).
This would be the first time in this operation we are doing a _synchronous_ I/O
operation where we sleep holding a pin.
So no, I think we really do need to revalidate.
'Collapse some random file we no longer map at this address' is probably not
great semantics, also of course, we are revalidating at each PMD anyway.
Maybe even do a addr -= HPAGE_PMD_SIZE; continue + take it from the top?
Maybe David has thoughts...
> khugepaged_scan_mm_slot() seems to be serving the same function as maybe_unlock_mmap_for_io().
> So I don't think this is needed?
We're talking about the MADV_COLLAPSE case so don't understand this? I may be
missing something here (happens a lot ;)!
>
> >
> >
> > > +
> > > result = alloc_charge_folio(&new_folio, mm, cc);
> > > if (result != SCAN_SUCCEED)
> > > goto out;
> > >
> > > base-commit: e9a6fb0bcdd7609be6969112f3fbfcce3b1d4a7c
> > > --
> > > 2.43.0
> > >
> > Thanks, Lorenzo
Cheers, Lorenzo
^ permalink raw reply
* Re: [PATCH 1/2] mm/khugepaged: do synchronous writeback for MADV_COLLAPSE
From: Lorenzo Stoakes @ 2025-11-10 13:52 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Shivank Garg, Andrew Morton, David Hildenbrand, Zi Yan,
Baolin Wang, Liam R . Howlett, Nico Pache, Ryan Roberts, Dev Jain,
Barry Song, Lance Yang, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Zach O'Keefe, linux-mm, linux-kernel,
linux-trace-kernel, Branden Moore
In-Reply-To: <aRHs3pA2kOr_uD3k@casper.infradead.org>
On Mon, Nov 10, 2025 at 01:47:10PM +0000, Matthew Wilcox wrote:
> On Mon, Nov 10, 2025 at 11:32:53AM +0000, Shivank Garg wrote:
> > When MADV_COLLAPSE is called on file-backed mappings (e.g., executable
> > text sections), the pages may still be dirty from recent writes. The
>
> That explanation derails my brain entirely. Text isn't writable! How
> can the pages be dirty and file-backed text?
Because they just compiled it and it's not been written back to disk yet :)
^ permalink raw reply
* Re: [PATCH 1/2] mm/khugepaged: do synchronous writeback for MADV_COLLAPSE
From: Matthew Wilcox @ 2025-11-10 13:47 UTC (permalink / raw)
To: Shivank Garg
Cc: Andrew Morton, David Hildenbrand, Lorenzo Stoakes, Zi Yan,
Baolin Wang, Liam R . Howlett, Nico Pache, Ryan Roberts, Dev Jain,
Barry Song, Lance Yang, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Zach O'Keefe, linux-mm, linux-kernel,
linux-trace-kernel, Branden Moore
In-Reply-To: <20251110113254.77822-1-shivankg@amd.com>
On Mon, Nov 10, 2025 at 11:32:53AM +0000, Shivank Garg wrote:
> When MADV_COLLAPSE is called on file-backed mappings (e.g., executable
> text sections), the pages may still be dirty from recent writes. The
That explanation derails my brain entirely. Text isn't writable! How
can the pages be dirty and file-backed text?
^ permalink raw reply
* Re: [PATCH 2/2] mm/khugepaged: return EAGAIN for transient dirty pages in MADV_COLLAPSE
From: Dev Jain @ 2025-11-10 13:46 UTC (permalink / raw)
To: Shivank Garg, Andrew Morton, David Hildenbrand, Lorenzo Stoakes
Cc: Zi Yan, Baolin Wang, Liam R . Howlett, Nico Pache, Ryan Roberts,
Barry Song, Lance Yang, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Zach O'Keefe, linux-mm, linux-kernel,
linux-trace-kernel
In-Reply-To: <20251110113254.77822-3-shivankg@amd.com>
On 10/11/25 5:02 pm, Shivank Garg wrote:
> When MADV_COLLAPSE encounters dirty file-backed pages, it currently
> returns -EINVAL, this is misleading as EINVAL suggests invalid arguments,
> whereas dirty pages are a transient condition that may resolve on retry.
>
> Introduce SCAN_PAGE_DIRTY and map it to -EAGAIN. For khugepaged, this
> is harmless as it will revisit the range after async writeback completes.
Taking a cursory look at enum scan_result, I am sure there are other error
codes as well which redirect to -EINVAL when they actually should to something
else...
>
> Signed-off-by: Shivank Garg <shivankg@amd.com>
> ---
> include/trace/events/huge_memory.h | 3 ++-
> mm/khugepaged.c | 4 +++-
> 2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/include/trace/events/huge_memory.h b/include/trace/events/huge_memory.h
> index dd94d14a2427..9014a9bbe64c 100644
> --- a/include/trace/events/huge_memory.h
> +++ b/include/trace/events/huge_memory.h
> @@ -38,7 +38,8 @@
> EM( SCAN_PAGE_HAS_PRIVATE, "page_has_private") \
> EM( SCAN_STORE_FAILED, "store_failed") \
> EM( SCAN_COPY_MC, "copy_poisoned_page") \
> - EMe(SCAN_PAGE_FILLED, "page_filled")
> + EM(SCAN_PAGE_FILLED, "page_filled") \
> + EMe(SCAN_PAGE_DIRTY, "page_dirty")
>
> #undef EM
> #undef EMe
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index d08ed6eb9ce1..7df329c9c87d 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -60,6 +60,7 @@ enum scan_result {
> SCAN_STORE_FAILED,
> SCAN_COPY_MC,
> SCAN_PAGE_FILLED,
> + SCAN_PAGE_DIRTY,
> };
>
> #define CREATE_TRACE_POINTS
> @@ -1967,7 +1968,7 @@ static int collapse_file(struct mm_struct *mm, unsigned long addr,
> */
> xas_unlock_irq(&xas);
> filemap_flush(mapping);
> - result = SCAN_FAIL;
> + result = SCAN_PAGE_DIRTY;
> goto xa_unlocked;
> } else if (folio_test_writeback(folio)) {
> xas_unlock_irq(&xas);
Now that you are at it, it would make sense to redirect this folio_test_writeback()
branch also to -EAGAIN, and call the common error code SCAN_PAGE_UNDER_WRITEBACK.
> @@ -2747,6 +2748,7 @@ static int madvise_collapse_errno(enum scan_result r)
> case SCAN_PAGE_LRU:
> case SCAN_DEL_PAGE_LRU:
> case SCAN_PAGE_FILLED:
> + case SCAN_PAGE_DIRTY:
> return -EAGAIN;
> /*
> * Other: Trying again likely not to succeed / error intrinsic to
^ permalink raw reply
* Re: [PATCH 6/6] kallsyms: Prevent module removal when printing module name and buildid
From: Petr Mladek @ 2025-11-10 13:26 UTC (permalink / raw)
To: Aaron Tomlin
Cc: Petr Pavlu, Steven Rostedt, Alexei Starovoitov, Andrew Morton,
Kees Cook, Daniel Borkmann, John Fastabend, Masami Hiramatsu,
Mark Rutland, Luis Chamberlain, Daniel Gomez, Sami Tolvanen,
linux-kernel, bpf, linux-modules, linux-trace-kernel
In-Reply-To: <kubk2a4ydmja45dfnwxkkhpdbov27m6errnenc6eljbgdmidzl@is24eqefukit>
On Fri 2025-11-07 19:36:35, Aaron Tomlin wrote:
> On Wed, Nov 05, 2025 at 03:23:18PM +0100, Petr Mladek wrote:
> > kallsyms_lookup_buildid() copies the symbol name into the given buffer
> > so that it can be safely read anytime later. But it just copies pointers
> > to mod->name and mod->build_id which might get reused after the related
> > struct module gets removed.
> >
> > The lifetime of struct module is synchronized using RCU. Take the rcu
> > read lock for the entire __sprint_symbol().
> >
> > Signed-off-by: Petr Mladek <pmladek@suse.com>
> > ---
> > kernel/kallsyms.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
> > index ff7017337535..1fda06b6638c 100644
> > --- a/kernel/kallsyms.c
> > +++ b/kernel/kallsyms.c
> > @@ -468,6 +468,9 @@ static int __sprint_symbol(char *buffer, unsigned long address,
> > unsigned long offset, size;
> > int len;
> >
> > + /* Prevent module removal until modname and modbuildid are printed */
> > + guard(rcu)();
> > +
> > address += symbol_offset;
> > len = kallsyms_lookup_buildid(address, &size, &offset, &modname, &buildid,
> > buffer);
> > --
> > 2.51.1
> >
> >
>
> Hi Petr,
>
> If I am not mistaken, this is handled safely within the context of
> module_address_lookup() since f01369239293e ("module: Use RCU in
> find_kallsyms_symbol()."), no?
The above mention commit fixed an API which is looking only for
the symbol name. It seems to be used, for example, in kprobe
or ftrace code.
This patch is fixing another API which is used in vsprintf() for
printing backtraces. It looks for more information: symbol name,
module name, and buildid. It needs its own RCU read protection.
Best Regards,
Petr
^ permalink raw reply
* Re: [PATCH 1/2] mm/khugepaged: do synchronous writeback for MADV_COLLAPSE
From: Dev Jain @ 2025-11-10 13:24 UTC (permalink / raw)
To: Lorenzo Stoakes, Shivank Garg
Cc: Andrew Morton, David Hildenbrand, Zi Yan, Baolin Wang,
Liam R . Howlett, Nico Pache, Ryan Roberts, Barry Song,
Lance Yang, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Zach O'Keefe, linux-mm, linux-kernel, linux-trace-kernel,
Branden Moore
In-Reply-To: <f21b37bb-7a2b-412b-be76-e8968b4c375d@lucifer.local>
On 10/11/25 5:31 pm, Lorenzo Stoakes wrote:
> On Mon, Nov 10, 2025 at 11:32:53AM +0000, Shivank Garg wrote:
>> When MADV_COLLAPSE is called on file-backed mappings (e.g., executable
>> text sections), the pages may still be dirty from recent writes. The
>> current code triggers an async flush via filemap_flush() and returns
>> SCAN_FAIL, requiring userspace to retry the operation.
>>
>> This is problematic for userspace that wants to collapse text pages into
>> THPs to reduce ITLB pressure. The first madvise() call always fails with
>> EINVAL, and only subsequent calls succeed after writeback completes.
>>
>> For direct MADV_COLLAPSE calls (!cc->is_khugepaged), perform a synchronous
>> writeback using filemap_write_and_wait_range() before scanning the folios.
>> This ensures that folios are clean on the first attempt.
>>
>> Reported-by: Branden Moore <Branden.Moore@amd.com>
>> Closes: https://lore.kernel.org/all/4e26fe5e-7374-467c-a333-9dd48f85d7cc@amd.com
>> Fixes: 34488399fa08 ("mm/madvise: add file and shmem support to MADV_COLLAPSE")
>> Suggested-by: David Hildenbrand <david@redhat.com>
>> Signed-off-by: Shivank Garg <shivankg@amd.com>
>> ---
>> Applies cleanly on:
>> 6.18-rc5
>> mm-stable:e9a6fb0bc
> Please base on mm-unstable. mm-stable is usually out of date until very close to
> merge window.
>
>>
>> mm/khugepaged.c | 17 +++++++++++++++++
>> 1 file changed, 17 insertions(+)
>>
>> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
>> index abe54f0043c7..d08ed6eb9ce1 100644
>> --- a/mm/khugepaged.c
>> +++ b/mm/khugepaged.c
>> @@ -21,6 +21,7 @@
>> #include <linux/shmem_fs.h>
>> #include <linux/dax.h>
>> #include <linux/ksm.h>
>> +#include <linux/backing-dev.h>
>>
>> #include <asm/tlb.h>
>> #include <asm/pgalloc.h>
>> @@ -1845,6 +1846,7 @@ static int collapse_file(struct mm_struct *mm, unsigned long addr,
>> struct page *dst;
>> struct folio *folio, *tmp, *new_folio;
>> pgoff_t index = 0, end = start + HPAGE_PMD_NR;
>> + loff_t range_start, range_end;
>> LIST_HEAD(pagelist);
>> XA_STATE_ORDER(xas, &mapping->i_pages, start, HPAGE_PMD_ORDER);
>> int nr_none = 0, result = SCAN_SUCCEED;
>> @@ -1853,6 +1855,21 @@ static int collapse_file(struct mm_struct *mm, unsigned long addr,
>> VM_BUG_ON(!IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) && !is_shmem);
>> VM_BUG_ON(start & (HPAGE_PMD_NR - 1));
>>
>> + /*
>> + * For MADV_COLLAPSE on regular files, do a synchronous writeback
>> + * to ensure dirty folios are flushed before we attempt collapse.
>> + * This is a best-effort approach to avoid failing on the first
>> + * attempt when freshly-written executable text is still dirty.
>> + */
>> + if (!is_shmem && cc && !cc->is_khugepaged && mapping_can_writeback(mapping)) {
>> + range_start = (loff_t)start << PAGE_SHIFT;
>> + range_end = ((loff_t)end << PAGE_SHIFT) - 1;
>> + if (filemap_write_and_wait_range(mapping, range_start, range_end)) {
>> + result = SCAN_FAIL;
>> + goto out;
>> + }
>> + }
> I feel this is the wrong level of abstraction.
>
> We explicitly invoke this oth from khugepaged and madvise(..., MADV_COLLAPSE):
>
>
> khugepaged_scan_mm_slot() / madvise_collapse()
> -> hpage_collapse_scan_file()
> -> collapse_file()
>
> ofc you are addressing this with the !cc->is_khugepaged, but feels like we'd be
> better off just doing it in madvise_collapse().
I suppose the common case is that writeback is not needed, and I don't know what
is the overhead of filemap_write_and_wait_range() in that case, but your
suggestion will force that unconditional overhead and skip all the early exits
possible in hpage_collapse_scan_file()?
> I wonder also if doing I/O without getting the mmap lock again and revalidating
> is wise, as the state of things might have changed significantly.
>
> So maybe need a hugepage_vma_revalidate() as well?
The file collapse path (apart from collapse_pte_mapped_thp) is solely concerned
about doing the collapse in the page cache, for which information about the mm or
the vma is not needed, that is why no locking is required. The get_file() in
khugepaged_scan_mm_slot() seems to be serving the same function as maybe_unlock_mmap_for_io().
So I don't think this is needed?
>
>
>> +
>> result = alloc_charge_folio(&new_folio, mm, cc);
>> if (result != SCAN_SUCCEED)
>> goto out;
>>
>> base-commit: e9a6fb0bcdd7609be6969112f3fbfcce3b1d4a7c
>> --
>> 2.43.0
>>
> Thanks, Lorenzo
^ permalink raw reply
* Re: [PATCH 1/2] mm/khugepaged: do synchronous writeback for MADV_COLLAPSE
From: Lorenzo Stoakes @ 2025-11-10 13:22 UTC (permalink / raw)
To: Garg, Shivank
Cc: Andrew Morton, David Hildenbrand, Zi Yan, Baolin Wang,
Liam R . Howlett, Nico Pache, Ryan Roberts, Dev Jain, Barry Song,
Lance Yang, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Zach O'Keefe, linux-mm, linux-kernel, linux-trace-kernel,
Branden Moore
In-Reply-To: <3b10d11a-3c75-4c37-8810-e238113ce75d@amd.com>
On Mon, Nov 10, 2025 at 06:37:58PM +0530, Garg, Shivank wrote:
>
>
> On 11/10/2025 5:31 PM, Lorenzo Stoakes wrote:
> > On Mon, Nov 10, 2025 at 11:32:53AM +0000, Shivank Garg wrote:
> >> When MADV_COLLAPSE is called on file-backed mappings (e.g., executable
>
> >> ---
> >> Applies cleanly on:
> >> 6.18-rc5
> >> mm-stable:e9a6fb0bc
> >
> > Please base on mm-unstable. mm-stable is usually out of date until very close to
> > merge window.
>
> I'm observing issues when testing with kselftest on mm-unstable and mm-new branches that prevent
> proper testing for my patches:
>
> On mm-unstable (without my patches):
>
> # # running ./transhuge-stress -d 20
> # # --------------------------------
> # # TAP version 13
> # # 1..1
> # # transhuge-stress: allocate 220271 transhuge pages, using 440543 MiB virtual memory and 1720 MiB of ram
>
>
> [ 367.225667] RIP: 0010:swap_cache_get_folio+0x2d/0xc0
> [ 367.230635] Code: 00 00 48 89 f9 49 89 f9 48 89 fe 48 c1 e1 06 49 c1 e9 3a 48 c1 e9 0f 48 c1 e1 05 4a 8b 04 cd c0 2e 5b 99 48 8b 78 60 48 01 cf <48> 8b 47 08 48 85 c0 74 20 48 89 f2 81 e2 ff 01 00 00 48 8d 04 d0
> [ 367.249378] RSP: 0000:ffffcde32943fba8 EFLAGS: 00010282
> [ 367.254605] RAX: ffff8bd1668fdc00 RBX: 00007ffc15df5000 RCX: 00003fffffffffe0
> [ 367.261736] RDX: ffffffff995cb530 RSI: 0003ffffffffffff RDI: ffffcbd1560dffe0
> [ 367.268862] RBP: 0003ffffffffffff R08: ffffcde32943fc47 R09: 0000000000000000
> [ 367.275994] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
> [ 367.283129] R13: 0000000000000000 R14: ffff8bd1668fdc00 R15: 0000000000100cca
> [ 367.290260] FS: 00007ff600af5b80(0000) GS:ffff8c4e9ec7e000(0000) knlGS:0000000000000000
> [ 367.298344] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 367.304083] CR2: ffffcbd1560dffe8 CR3: 00000001280e9001 CR4: 0000000000770ef0
> [ 367.311216] PKRU: 55555554
> [ 367.313929] Call Trace:
> [ 367.316375] <TASK>
> [ 367.318479] __read_swap_cache_async+0x8e/0x1b0
> [ 367.323014] swap_vma_readahead+0x23d/0x430
> [ 367.327198] swapin_readahead+0xb0/0xc0
> [ 367.331039] do_swap_page+0x5bc/0x1260
> [ 367.334789] ? rseq_ip_fixup+0x6f/0x190
> [ 367.338631] ? __pfx_default_wake_function+0x10/0x10
> [ 367.343596] __handle_mm_fault+0x49a/0x760
> [ 367.347696] handle_mm_fault+0x188/0x300
> [ 367.351620] do_user_addr_fault+0x15b/0x6c0
> [ 367.355807] exc_page_fault+0x60/0x100
> [ 367.359562] asm_exc_page_fault+0x22/0x30
> [ 367.363574] RIP: 0033:0x7ff60091ba99
> [ 367.367153] Code: f7 d8 64 89 02 b8 ff ff ff ff eb bd e8 40 c4 01 00 f3 0f 1e fa 80 3d b5 f5 0e 00 00 74 13 31 c0 0f 05 48 3d 00 f0 ff ff 77 4f <c3> 66 0f 1f 44 00 00 55 48 89 e5 48 83 ec 20 48 89 55 e8 48 89 75
> [ 367.385897] RSP: 002b:00007ffc15df1118 EFLAGS: 00010203
> [ 367.391124] RAX: 0000000000000001 RBX: 000055941fb672a0 RCX: 00007ff60091ba91
> [ 367.398256] RDX: 0000000000000001 RSI: 000055941fb813e0 RDI: 0000000000000000
> [ 367.405387] RBP: 00007ffc15df21e0 R08: 0000000000000000 R09: 0000000000000007
> [ 367.412513] R10: 000055941fb97cb0 R11: 0000000000000246 R12: 000055941fb813e0
> [ 367.419646] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
> [ 367.426781] </TASK>
> [ 367.428970] Modules linked in: xfrm_user xfrm_algo xt_addrtype xt_CHECKSUM xt_MASQUERADE xt_conntrack ipt_REJECT nf_reject_ipv4 nft_compat nft_chain_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 nf_tables overlay bridge stp llc cfg80211 rfkill binfmt_misc ipmi_ssif amd_atl intel_rapl_msr intel_rapl_common wmi_bmof amd64_edac edac_mce_amd mgag200 rapl drm_client_lib i2c_algo_bit drm_shmem_helper drm_kms_helper acpi_cpufreq i2c_piix4 ptdma k10temp i2c_smbus wmi acpi_power_meter ipmi_si acpi_ipmi ipmi_devintf ipmi_msghandler sg dm_multipath drm fuse dm_mod nfnetlink ext4 crc16 mbcache jbd2 raid10 raid456 async_raid6_recov async_memcpy async_pq async_xor xor async_tx raid6_pq raid1 raid0 kvm_amd sd_mod ahci nvme libahci kvm libata nvme_core tg3 ccp megaraid_sas irqbypass
> [ 367.497528] CR2: ffffcbd1560dffe8
> [ 367.500846] ---[ end trace 0000000000000000 ]---
Yikes, oopsies!
I'll try running tests locally on threadripper, but ran tests against yours
previously and seemed fine, strange. Maybe fixed since but let me try, maybe
because swap is not enabled locally for me?
Likely this actually...
>
>
>
> -----------------
> On mm-new (without my patches):
>
> [ 394.144770] get_swap_device: Bad swap offset entry 3ffffffffffff
>
> dmesg | grep "get_swap_device: Bad swap offset entry" | wc -l
> 359
>
>
> Additionally, kexec triggers an oops and crash during swapoff:
>
>
> Deactivating swap 704.854238] BUG: unable to handle page fault for address: ffffcbe2de8dffe8
> [ 704.861524] #PF: supervisor read access in kernel mode
> ;39mswap.img.swa[ 704.866666] #PF: error_code(0x0000) - not-present page
> [ 704.873253] PGD 0 P4D 0
> p - /swap.im[ 704.875790] Oops: Oops: 0000 [#1] SMP NOPTI
> g...
> [ 704.881354] CPU: 122 UID: 0 PID: 107680 Comm: swapoff Kdump: loaded Not tainted 6.18.0-rc5+ #11 NONE
> [ 704.891283] Hardware name: Dell Inc. PowerEdge R6525/024PW1, BIOS 2.16.2 07/09/2024
> [ 704.898930] RIP: 0010:swap_cache_get_folio+0x2d/0xc0
> [ 704.903907] Code: 00 00 48 89 f9 49 89 f9 48 89 fe 48 c1 e1 06 49 c1 e9 3a 48 c1 e9 0f 48 c1 e1 05 4a 8b 04 cd c0 2e 7b 95 48 8b 78 60 48 01 cf <48> 8b 47 08 48 85 c0 74 20 48 89 f2 81 e2 ff 01 00 00 48 8d 04 d0
> [ 704.922720] RSP: 0018:ffffcf1227b1fc08 EFLAGS: 00010282
> [ 704.928035] RAX: ffff8be2cefb3c00 RBX: 0000555c65a5c000 RCX: 00003fffffffffe0
> [ 704.928036] RDX: 0003ffffffffffff RSI: 0003ffffffffffff RDI: ffffcbe2de8dffe0
> [ 704.928037] RBP: 0000000000000000 R08: ffff8be2de8e0520 R09: 0000000000000000
> Unmount[ 704.928038] R10: 000000000000ffff R11: ffffcf12236f4000 R12: ffff8be2d5b8d968
> [ 704.928039] R13: 0003ffffffffffff R14: fffff3eec85eb000 R15: 0000555c65a51000
> [ 704.928039] FS: 00007f41fcab3800(0000) GS:ffff8c602b6fe000(0000) knlGS:0000000000000000
> [ 704.928040] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 704.928041] CR2: ffffcbe2de8dffe8 CR3: 00000074981af004 CR4: 0000000000770ef0
> [ 704.928041] PKRU: 55555554
> [ 704.928042] Call Trace:
> [ 704.928043] <TASK>
> [ 704.928044] unuse_pte_range+0x10b/0x290
> [ 704.928047] unuse_pud_range.isra.0+0x149/0x190
> [ 704.928048] unuse_vma+0x1a6/0x220
> [ 704.928050] unuse_mm+0x9b/0x110
> [ 704.928052] try_to_unuse+0xc5/0x260
> [ 704.928053] __do_sys_swapoff+0x244/0x670
> ing boo[ 705.016662] do_syscall_64+0x67/0xc50
> [ 705.016667] ? do_user_addr_fault+0x15b/0x6c0
> t.mount - /b[ 705.026100] ? exc_page_fault+0x60/0x100
> [ 705.031498] ? irqentry_exit_to_user_mode+0x20/0xe0
> oot...
> [ 705.036377] entry_SYSCALL_64_after_hwframe+0x76/0x7e
> [ 705.042200] RIP: 0033:0x7f41fc9271bb
> [ 705.045780] Code: 0f 1e fa 48 83 fe 01 48 8b 15 59 bc 0d 00 19 c0 83 e0 f0 83 c0 26 64 89 02 b8 ff ff ff ff c3 f3 0f 1e fa b8 a8 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 2d bc 0d 00 f7 d8 64 89 01 48
> [ 705.064807] RSP: 002b:00007ffd14b5b6e8 EFLAGS: 00000202 ORIG_RAX: 00000000000000a8
> [ 705.064809] RAX: ffffffffffffffda RBX: 00007ffd14b5cf30 RCX: 00007f41fc9271bb
> [ 705.064810] RDX: 0000000000000001 RSI: 0000000000000c00 RDI: 000055d48f533a40
> [ 705.064810] RBP: 00007ffd14b5b750 R08: 00007f41fca03b20 R09: 0000000000000000
> [ 705.064811] R10: 0000000000000001 R11: 0000000000000202 R12: 0000000000000000
> [ 705.064811] R13: 0000000000000000 R14: 000055d4584f1479 R15: 000055d4584f2b20
> [ 705.064813] </TASK>
> [ 705.064814] Modules linked in: xfrm_user xfrm_algo xt_addrtype xt_CHECKSUM xt_MASQUERADE xt_conntrack ipt_REJECT nf_reject_ipv4 nft_compat nft_chain_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 nf_tables overlay bridge stp llc cfg80211 rfkill binfmt_misc ipmi_ssif amd_atl intel_rapl_msr intel_rapl_common wmi_bmof amd64_edac edac_mce_amd rapl mgag200 drm_client_lib i2c_algo_bit drm_shmem_helper drm_kms_helper acpi_cpufreq i2c_piix4 ptdma ipmi_si k10temp i2c_smbus acpi_power_meter wmi acpi_ipmi ipmi_msghandler sg dm_multipath fuse drm dm_mod nfnetlink ext4 crc16 mbcache jbd2 raid10 raid456 async_raid6_recov async_memcpy async_pq async_xor xor async_tx raid6_pq raid1 raid0 sd_mod kvm_amd ahci libahci kvm nvme tg3 libata ccp irqbypass nvme_core megaraid_sas [last unloaded: ipmi_devintf]
> [ 705.180420] CR2: ffffcbe2de8dffe8
> [ 705.183852] ---[ end trace 0000000000000000 ]---
>
>
> I haven't had cycles to dig into this yet and been swamped with other things.
Fully understand, I'm _very_ familiar with this situation :)
I need more cores... ;)
>
>
> >> + if (!is_shmem && cc && !cc->is_khugepaged && mapping_can_writeback(mapping)) {
> >> + range_start = (loff_t)start << PAGE_SHIFT;
> >> + range_end = ((loff_t)end << PAGE_SHIFT) - 1;
> >> + if (filemap_write_and_wait_range(mapping, range_start, range_end)) {
> >> + result = SCAN_FAIL;
> >> + goto out;
> >> + }
> >> + }
> >
> > I feel this is the wrong level of abstraction.
> >
> > We explicitly invoke this oth from khugepaged and madvise(..., MADV_COLLAPSE):
> >
> >
> > khugepaged_scan_mm_slot() / madvise_collapse()
> > -> hpage_collapse_scan_file()
> > -> collapse_file()
> >
> > ofc you are addressing this with the !cc->is_khugepaged, but feels like we'd be
> > better off just doing it in madvise_collapse().
> >
> > I wonder also if doing I/O without getting the mmap lock again and revalidating
> > is wise, as the state of things might have changed significantly.
> >
> > So maybe need a hugepage_vma_revalidate() as well?
>
> Thanks for the feedback. I'll incorporate these comments for v2.
Thanks!
>
> Thanks,
> Shivank
>
>
Cheers, Lorenzo
^ permalink raw reply
* Re: [PATCH v12 mm-new 05/15] khugepaged: generalize __collapse_huge_page_* for mTHP support
From: Nico Pache @ 2025-11-10 13:20 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: linux-kernel, linux-trace-kernel, linux-mm, linux-doc, david, ziy,
baolin.wang, Liam.Howlett, ryan.roberts, dev.jain, corbet,
rostedt, mhiramat, mathieu.desnoyers, akpm, baohua, willy, peterx,
wangkefeng.wang, usamaarif642, sunnanyong, vishal.moola,
thomas.hellstrom, yang, kas, aarcange, raquini, anshuman.khandual,
catalin.marinas, tiwai, will, dave.hansen, jack, cl, jglisse,
surenb, zokeefe, hannes, rientjes, mhocko, rdunlap, hughd,
richard.weiyang, lance.yang, vbabka, rppt, jannh, pfalcato
In-Reply-To: <c006138d-6d12-4f91-8a06-da279ae3795a@lucifer.local>
On Mon, Oct 27, 2025 at 10:02 AM Lorenzo Stoakes
<lorenzo.stoakes@oracle.com> wrote:
>
> On Wed, Oct 22, 2025 at 12:37:07PM -0600, Nico Pache wrote:
> > generalize the order of the __collapse_huge_page_* functions
> > to support future mTHP collapse.
> >
> > mTHP collapse will not honor the khugepaged_max_ptes_shared or
> > khugepaged_max_ptes_swap parameters, and will fail if it encounters a
> > shared or swapped entry.
> >
> > No functional changes in this patch.
> >
> > Reviewed-by: Baolin Wang <baolin.wang@linux.alibaba.com>
> > Acked-by: David Hildenbrand <david@redhat.com>
> > Co-developed-by: Dev Jain <dev.jain@arm.com>
> > Signed-off-by: Dev Jain <dev.jain@arm.com>
> > Signed-off-by: Nico Pache <npache@redhat.com>
>
> Thanks for addressing the v10 stuff (didn't check at v11).
>
> Overall LGTM, so:
>
> Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Thanks!
>
> Few minor nits below.
>
> > ---
> > mm/khugepaged.c | 78 ++++++++++++++++++++++++++++++-------------------
> > 1 file changed, 48 insertions(+), 30 deletions(-)
> >
> > diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> > index 36ee659acfbb..4ccebf5dda97 100644
> > --- a/mm/khugepaged.c
> > +++ b/mm/khugepaged.c
> > @@ -537,25 +537,25 @@ static void release_pte_pages(pte_t *pte, pte_t *_pte,
> > }
> >
> > static int __collapse_huge_page_isolate(struct vm_area_struct *vma,
> > - unsigned long start_addr,
> > - pte_t *pte,
> > - struct collapse_control *cc,
> > - struct list_head *compound_pagelist)
> > + unsigned long start_addr, pte_t *pte, struct collapse_control *cc,
> > + unsigned int order, struct list_head *compound_pagelist)
>
> This series isn't the right place for it, but god do we need helper structs in
> this code... :)
Well we have collapse_control! I can spend some time in a follow up
series to better leverage this struct.
>
> > {
> > struct page *page = NULL;
> > struct folio *folio = NULL;
> > unsigned long addr = start_addr;
> > pte_t *_pte;
> > int none_or_zero = 0, shared = 0, result = SCAN_FAIL, referenced = 0;
> > + const unsigned long nr_pages = 1UL << order;
> > + int max_ptes_none = khugepaged_max_ptes_none >> (HPAGE_PMD_ORDER - order);
>
> Nit, but we should const-ify this too.
This gets converted to collapse_max_ptes_none in the future.
>
> >
> > - for (_pte = pte; _pte < pte + HPAGE_PMD_NR;
> > + for (_pte = pte; _pte < pte + nr_pages;
> > _pte++, addr += PAGE_SIZE) {
> > pte_t pteval = ptep_get(_pte);
> > if (pte_none_or_zero(pteval)) {
> > ++none_or_zero;
> > if (!userfaultfd_armed(vma) &&
> > (!cc->is_khugepaged ||
> > - none_or_zero <= khugepaged_max_ptes_none)) {
> > + none_or_zero <= max_ptes_none)) {
> > continue;
> > } else {
> > result = SCAN_EXCEED_NONE_PTE;
> > @@ -583,8 +583,14 @@ static int __collapse_huge_page_isolate(struct vm_area_struct *vma,
> > /* See collapse_scan_pmd(). */
> > if (folio_maybe_mapped_shared(folio)) {
> > ++shared;
> > - if (cc->is_khugepaged &&
> > - shared > khugepaged_max_ptes_shared) {
> > + /*
> > + * TODO: Support shared pages without leading to further
> > + * mTHP collapses. Currently bringing in new pages via
> > + * shared may cause a future higher order collapse on a
> > + * rescan of the same range.
> > + */
>
> Yeah, I wish we could find a way to address this in some other way but given the
> mire of THP code putting this comment here for now is probably the only sensible
> way.
>
> > + if (order != HPAGE_PMD_ORDER || (cc->is_khugepaged &&
> > + shared > khugepaged_max_ptes_shared)) {
> > result = SCAN_EXCEED_SHARED_PTE;
> > count_vm_event(THP_SCAN_EXCEED_SHARED_PTE);
> > goto out;
> > @@ -677,18 +683,18 @@ static int __collapse_huge_page_isolate(struct vm_area_struct *vma,
> > }
> >
> > static void __collapse_huge_page_copy_succeeded(pte_t *pte,
> > - struct vm_area_struct *vma,
> > - unsigned long address,
> > - spinlock_t *ptl,
> > - struct list_head *compound_pagelist)
> > + struct vm_area_struct *vma, unsigned long address,
> > + spinlock_t *ptl, unsigned int order,
> > + struct list_head *compound_pagelist)
> > {
> > - unsigned long end = address + HPAGE_PMD_SIZE;
> > + unsigned long end = address + (PAGE_SIZE << order);
> > struct folio *src, *tmp;
> > pte_t pteval;
> > pte_t *_pte;
> > unsigned int nr_ptes;
> > + const unsigned long nr_pages = 1UL << order;
> >
> > - for (_pte = pte; _pte < pte + HPAGE_PMD_NR; _pte += nr_ptes,
> > + for (_pte = pte; _pte < pte + nr_pages; _pte += nr_ptes,
> > address += nr_ptes * PAGE_SIZE) {
> > nr_ptes = 1;
> > pteval = ptep_get(_pte);
> > @@ -741,13 +747,11 @@ static void __collapse_huge_page_copy_succeeded(pte_t *pte,
> > }
> >
> > static void __collapse_huge_page_copy_failed(pte_t *pte,
> > - pmd_t *pmd,
> > - pmd_t orig_pmd,
> > - struct vm_area_struct *vma,
> > - struct list_head *compound_pagelist)
> > + pmd_t *pmd, pmd_t orig_pmd, struct vm_area_struct *vma,
> > + unsigned int order, struct list_head *compound_pagelist)
> > {
> > spinlock_t *pmd_ptl;
> > -
> > + const unsigned long nr_pages = 1UL << order;
> > /*
> > * Re-establish the PMD to point to the original page table
> > * entry. Restoring PMD needs to be done prior to releasing
> > @@ -761,7 +765,7 @@ static void __collapse_huge_page_copy_failed(pte_t *pte,
> > * Release both raw and compound pages isolated
> > * in __collapse_huge_page_isolate.
> > */
> > - release_pte_pages(pte, pte + HPAGE_PMD_NR, compound_pagelist);
> > + release_pte_pages(pte, pte + nr_pages, compound_pagelist);
> > }
> >
> > /*
> > @@ -781,16 +785,16 @@ static void __collapse_huge_page_copy_failed(pte_t *pte,
> > */
> > static int __collapse_huge_page_copy(pte_t *pte, struct folio *folio,
> > pmd_t *pmd, pmd_t orig_pmd, struct vm_area_struct *vma,
> > - unsigned long address, spinlock_t *ptl,
> > + unsigned long address, spinlock_t *ptl, unsigned int order,
> > struct list_head *compound_pagelist)
> > {
> > unsigned int i;
> > int result = SCAN_SUCCEED;
> > -
> > + const unsigned long nr_pages = 1UL << order;
> > /*
> > * Copying pages' contents is subject to memory poison at any iteration.
> > */
> > - for (i = 0; i < HPAGE_PMD_NR; i++) {
> > + for (i = 0; i < nr_pages; i++) {
> > pte_t pteval = ptep_get(pte + i);
> > struct page *page = folio_page(folio, i);
> > unsigned long src_addr = address + i * PAGE_SIZE;
> > @@ -809,10 +813,10 @@ static int __collapse_huge_page_copy(pte_t *pte, struct folio *folio,
> >
> > if (likely(result == SCAN_SUCCEED))
> > __collapse_huge_page_copy_succeeded(pte, vma, address, ptl,
> > - compound_pagelist);
> > + order, compound_pagelist);
> > else
> > __collapse_huge_page_copy_failed(pte, pmd, orig_pmd, vma,
> > - compound_pagelist);
> > + order, compound_pagelist);
> >
> > return result;
> > }
> > @@ -985,13 +989,12 @@ static int check_pmd_still_valid(struct mm_struct *mm,
> > * Returns result: if not SCAN_SUCCEED, mmap_lock has been released.
> > */
> > static int __collapse_huge_page_swapin(struct mm_struct *mm,
> > - struct vm_area_struct *vma,
> > - unsigned long start_addr, pmd_t *pmd,
> > - int referenced)
> > + struct vm_area_struct *vma, unsigned long start_addr,
> > + pmd_t *pmd, int referenced, unsigned int order)
>
> Nit, super nit really, but since other __collapse_huge_page_*() functions have
> ..., order, param) as their last parameters, perhaps worth flipping referenced +
> order here?
>
> Not a big deal though.
>
> > {
> > int swapped_in = 0;
> > vm_fault_t ret = 0;
> > - unsigned long addr, end = start_addr + (HPAGE_PMD_NR * PAGE_SIZE);
> > + unsigned long addr, end = start_addr + (PAGE_SIZE << order);
> > int result;
> > pte_t *pte = NULL;
> > spinlock_t *ptl;
> > @@ -1022,6 +1025,19 @@ static int __collapse_huge_page_swapin(struct mm_struct *mm,
> > if (!is_swap_pte(vmf.orig_pte))
> > continue;
> >
> > + /*
> > + * TODO: Support swapin without leading to further mTHP
> > + * collapses. Currently bringing in new pages via swapin may
> > + * cause a future higher order collapse on a rescan of the same
> > + * range.
> > + */
>
> Same comment as above re: this, i.e. that it's a pity but probably unavoidable
> for now.
>
> > + if (order != HPAGE_PMD_ORDER) {
> > + pte_unmap(pte);
> > + mmap_read_unlock(mm);
> > + result = SCAN_EXCEED_SWAP_PTE;
> > + goto out;
> > + }
> > +
> > vmf.pte = pte;
> > vmf.ptl = ptl;
> > ret = do_swap_page(&vmf);
> > @@ -1142,7 +1158,7 @@ static int collapse_huge_page(struct mm_struct *mm, unsigned long address,
> > * that case. Continuing to collapse causes inconsistency.
> > */
> > result = __collapse_huge_page_swapin(mm, vma, address, pmd,
> > - referenced);
> > + referenced, HPAGE_PMD_ORDER);
> > if (result != SCAN_SUCCEED)
> > goto out_nolock;
> > }
> > @@ -1190,6 +1206,7 @@ static int collapse_huge_page(struct mm_struct *mm, unsigned long address,
> > pte = pte_offset_map_lock(mm, &_pmd, address, &pte_ptl);
> > if (pte) {
> > result = __collapse_huge_page_isolate(vma, address, pte, cc,
> > + HPAGE_PMD_ORDER,
> > &compound_pagelist);
> > spin_unlock(pte_ptl);
> > } else {
> > @@ -1220,6 +1237,7 @@ static int collapse_huge_page(struct mm_struct *mm, unsigned long address,
> >
> > result = __collapse_huge_page_copy(pte, folio, pmd, _pmd,
> > vma, address, pte_ptl,
> > + HPAGE_PMD_ORDER,
> > &compound_pagelist);
> > pte_unmap(pte);
> > if (unlikely(result != SCAN_SUCCEED))
> > --
> > 2.51.0
> >
>
^ permalink raw reply
* Re: [PATCH 1/2] mm/khugepaged: do synchronous writeback for MADV_COLLAPSE
From: Garg, Shivank @ 2025-11-10 13:07 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Andrew Morton, David Hildenbrand, Zi Yan, Baolin Wang,
Liam R . Howlett, Nico Pache, Ryan Roberts, Dev Jain, Barry Song,
Lance Yang, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Zach O'Keefe, linux-mm, linux-kernel, linux-trace-kernel,
Branden Moore
In-Reply-To: <f21b37bb-7a2b-412b-be76-e8968b4c375d@lucifer.local>
On 11/10/2025 5:31 PM, Lorenzo Stoakes wrote:
> On Mon, Nov 10, 2025 at 11:32:53AM +0000, Shivank Garg wrote:
>> When MADV_COLLAPSE is called on file-backed mappings (e.g., executable
>> ---
>> Applies cleanly on:
>> 6.18-rc5
>> mm-stable:e9a6fb0bc
>
> Please base on mm-unstable. mm-stable is usually out of date until very close to
> merge window.
I'm observing issues when testing with kselftest on mm-unstable and mm-new branches that prevent
proper testing for my patches:
On mm-unstable (without my patches):
# # running ./transhuge-stress -d 20
# # --------------------------------
# # TAP version 13
# # 1..1
# # transhuge-stress: allocate 220271 transhuge pages, using 440543 MiB virtual memory and 1720 MiB of ram
[ 367.225667] RIP: 0010:swap_cache_get_folio+0x2d/0xc0
[ 367.230635] Code: 00 00 48 89 f9 49 89 f9 48 89 fe 48 c1 e1 06 49 c1 e9 3a 48 c1 e9 0f 48 c1 e1 05 4a 8b 04 cd c0 2e 5b 99 48 8b 78 60 48 01 cf <48> 8b 47 08 48 85 c0 74 20 48 89 f2 81 e2 ff 01 00 00 48 8d 04 d0
[ 367.249378] RSP: 0000:ffffcde32943fba8 EFLAGS: 00010282
[ 367.254605] RAX: ffff8bd1668fdc00 RBX: 00007ffc15df5000 RCX: 00003fffffffffe0
[ 367.261736] RDX: ffffffff995cb530 RSI: 0003ffffffffffff RDI: ffffcbd1560dffe0
[ 367.268862] RBP: 0003ffffffffffff R08: ffffcde32943fc47 R09: 0000000000000000
[ 367.275994] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
[ 367.283129] R13: 0000000000000000 R14: ffff8bd1668fdc00 R15: 0000000000100cca
[ 367.290260] FS: 00007ff600af5b80(0000) GS:ffff8c4e9ec7e000(0000) knlGS:0000000000000000
[ 367.298344] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 367.304083] CR2: ffffcbd1560dffe8 CR3: 00000001280e9001 CR4: 0000000000770ef0
[ 367.311216] PKRU: 55555554
[ 367.313929] Call Trace:
[ 367.316375] <TASK>
[ 367.318479] __read_swap_cache_async+0x8e/0x1b0
[ 367.323014] swap_vma_readahead+0x23d/0x430
[ 367.327198] swapin_readahead+0xb0/0xc0
[ 367.331039] do_swap_page+0x5bc/0x1260
[ 367.334789] ? rseq_ip_fixup+0x6f/0x190
[ 367.338631] ? __pfx_default_wake_function+0x10/0x10
[ 367.343596] __handle_mm_fault+0x49a/0x760
[ 367.347696] handle_mm_fault+0x188/0x300
[ 367.351620] do_user_addr_fault+0x15b/0x6c0
[ 367.355807] exc_page_fault+0x60/0x100
[ 367.359562] asm_exc_page_fault+0x22/0x30
[ 367.363574] RIP: 0033:0x7ff60091ba99
[ 367.367153] Code: f7 d8 64 89 02 b8 ff ff ff ff eb bd e8 40 c4 01 00 f3 0f 1e fa 80 3d b5 f5 0e 00 00 74 13 31 c0 0f 05 48 3d 00 f0 ff ff 77 4f <c3> 66 0f 1f 44 00 00 55 48 89 e5 48 83 ec 20 48 89 55 e8 48 89 75
[ 367.385897] RSP: 002b:00007ffc15df1118 EFLAGS: 00010203
[ 367.391124] RAX: 0000000000000001 RBX: 000055941fb672a0 RCX: 00007ff60091ba91
[ 367.398256] RDX: 0000000000000001 RSI: 000055941fb813e0 RDI: 0000000000000000
[ 367.405387] RBP: 00007ffc15df21e0 R08: 0000000000000000 R09: 0000000000000007
[ 367.412513] R10: 000055941fb97cb0 R11: 0000000000000246 R12: 000055941fb813e0
[ 367.419646] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
[ 367.426781] </TASK>
[ 367.428970] Modules linked in: xfrm_user xfrm_algo xt_addrtype xt_CHECKSUM xt_MASQUERADE xt_conntrack ipt_REJECT nf_reject_ipv4 nft_compat nft_chain_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 nf_tables overlay bridge stp llc cfg80211 rfkill binfmt_misc ipmi_ssif amd_atl intel_rapl_msr intel_rapl_common wmi_bmof amd64_edac edac_mce_amd mgag200 rapl drm_client_lib i2c_algo_bit drm_shmem_helper drm_kms_helper acpi_cpufreq i2c_piix4 ptdma k10temp i2c_smbus wmi acpi_power_meter ipmi_si acpi_ipmi ipmi_devintf ipmi_msghandler sg dm_multipath drm fuse dm_mod nfnetlink ext4 crc16 mbcache jbd2 raid10 raid456 async_raid6_recov async_memcpy async_pq async_xor xor async_tx raid6_pq raid1 raid0 kvm_amd sd_mod ahci nvme libahci kvm libata nvme_core tg3 ccp megaraid_sas irqbypass
[ 367.497528] CR2: ffffcbd1560dffe8
[ 367.500846] ---[ end trace 0000000000000000 ]---
-----------------
On mm-new (without my patches):
[ 394.144770] get_swap_device: Bad swap offset entry 3ffffffffffff
dmesg | grep "get_swap_device: Bad swap offset entry" | wc -l
359
Additionally, kexec triggers an oops and crash during swapoff:
Deactivating swap 704.854238] BUG: unable to handle page fault for address: ffffcbe2de8dffe8
[ 704.861524] #PF: supervisor read access in kernel mode
;39mswap.img.swa[ 704.866666] #PF: error_code(0x0000) - not-present page
[ 704.873253] PGD 0 P4D 0
p - /swap.im[ 704.875790] Oops: Oops: 0000 [#1] SMP NOPTI
g...
[ 704.881354] CPU: 122 UID: 0 PID: 107680 Comm: swapoff Kdump: loaded Not tainted 6.18.0-rc5+ #11 NONE
[ 704.891283] Hardware name: Dell Inc. PowerEdge R6525/024PW1, BIOS 2.16.2 07/09/2024
[ 704.898930] RIP: 0010:swap_cache_get_folio+0x2d/0xc0
[ 704.903907] Code: 00 00 48 89 f9 49 89 f9 48 89 fe 48 c1 e1 06 49 c1 e9 3a 48 c1 e9 0f 48 c1 e1 05 4a 8b 04 cd c0 2e 7b 95 48 8b 78 60 48 01 cf <48> 8b 47 08 48 85 c0 74 20 48 89 f2 81 e2 ff 01 00 00 48 8d 04 d0
[ 704.922720] RSP: 0018:ffffcf1227b1fc08 EFLAGS: 00010282
[ 704.928035] RAX: ffff8be2cefb3c00 RBX: 0000555c65a5c000 RCX: 00003fffffffffe0
[ 704.928036] RDX: 0003ffffffffffff RSI: 0003ffffffffffff RDI: ffffcbe2de8dffe0
[ 704.928037] RBP: 0000000000000000 R08: ffff8be2de8e0520 R09: 0000000000000000
Unmount[ 704.928038] R10: 000000000000ffff R11: ffffcf12236f4000 R12: ffff8be2d5b8d968
[ 704.928039] R13: 0003ffffffffffff R14: fffff3eec85eb000 R15: 0000555c65a51000
[ 704.928039] FS: 00007f41fcab3800(0000) GS:ffff8c602b6fe000(0000) knlGS:0000000000000000
[ 704.928040] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 704.928041] CR2: ffffcbe2de8dffe8 CR3: 00000074981af004 CR4: 0000000000770ef0
[ 704.928041] PKRU: 55555554
[ 704.928042] Call Trace:
[ 704.928043] <TASK>
[ 704.928044] unuse_pte_range+0x10b/0x290
[ 704.928047] unuse_pud_range.isra.0+0x149/0x190
[ 704.928048] unuse_vma+0x1a6/0x220
[ 704.928050] unuse_mm+0x9b/0x110
[ 704.928052] try_to_unuse+0xc5/0x260
[ 704.928053] __do_sys_swapoff+0x244/0x670
ing boo[ 705.016662] do_syscall_64+0x67/0xc50
[ 705.016667] ? do_user_addr_fault+0x15b/0x6c0
t.mount - /b[ 705.026100] ? exc_page_fault+0x60/0x100
[ 705.031498] ? irqentry_exit_to_user_mode+0x20/0xe0
oot...
[ 705.036377] entry_SYSCALL_64_after_hwframe+0x76/0x7e
[ 705.042200] RIP: 0033:0x7f41fc9271bb
[ 705.045780] Code: 0f 1e fa 48 83 fe 01 48 8b 15 59 bc 0d 00 19 c0 83 e0 f0 83 c0 26 64 89 02 b8 ff ff ff ff c3 f3 0f 1e fa b8 a8 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 2d bc 0d 00 f7 d8 64 89 01 48
[ 705.064807] RSP: 002b:00007ffd14b5b6e8 EFLAGS: 00000202 ORIG_RAX: 00000000000000a8
[ 705.064809] RAX: ffffffffffffffda RBX: 00007ffd14b5cf30 RCX: 00007f41fc9271bb
[ 705.064810] RDX: 0000000000000001 RSI: 0000000000000c00 RDI: 000055d48f533a40
[ 705.064810] RBP: 00007ffd14b5b750 R08: 00007f41fca03b20 R09: 0000000000000000
[ 705.064811] R10: 0000000000000001 R11: 0000000000000202 R12: 0000000000000000
[ 705.064811] R13: 0000000000000000 R14: 000055d4584f1479 R15: 000055d4584f2b20
[ 705.064813] </TASK>
[ 705.064814] Modules linked in: xfrm_user xfrm_algo xt_addrtype xt_CHECKSUM xt_MASQUERADE xt_conntrack ipt_REJECT nf_reject_ipv4 nft_compat nft_chain_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 nf_tables overlay bridge stp llc cfg80211 rfkill binfmt_misc ipmi_ssif amd_atl intel_rapl_msr intel_rapl_common wmi_bmof amd64_edac edac_mce_amd rapl mgag200 drm_client_lib i2c_algo_bit drm_shmem_helper drm_kms_helper acpi_cpufreq i2c_piix4 ptdma ipmi_si k10temp i2c_smbus acpi_power_meter wmi acpi_ipmi ipmi_msghandler sg dm_multipath fuse drm dm_mod nfnetlink ext4 crc16 mbcache jbd2 raid10 raid456 async_raid6_recov async_memcpy async_pq async_xor xor async_tx raid6_pq raid1 raid0 sd_mod kvm_amd ahci libahci kvm nvme tg3 libata ccp irqbypass nvme_core megaraid_sas [last unloaded: ipmi_devintf]
[ 705.180420] CR2: ffffcbe2de8dffe8
[ 705.183852] ---[ end trace 0000000000000000 ]---
I haven't had cycles to dig into this yet and been swamped with other things.
>> + if (!is_shmem && cc && !cc->is_khugepaged && mapping_can_writeback(mapping)) {
>> + range_start = (loff_t)start << PAGE_SHIFT;
>> + range_end = ((loff_t)end << PAGE_SHIFT) - 1;
>> + if (filemap_write_and_wait_range(mapping, range_start, range_end)) {
>> + result = SCAN_FAIL;
>> + goto out;
>> + }
>> + }
>
> I feel this is the wrong level of abstraction.
>
> We explicitly invoke this oth from khugepaged and madvise(..., MADV_COLLAPSE):
>
>
> khugepaged_scan_mm_slot() / madvise_collapse()
> -> hpage_collapse_scan_file()
> -> collapse_file()
>
> ofc you are addressing this with the !cc->is_khugepaged, but feels like we'd be
> better off just doing it in madvise_collapse().
>
> I wonder also if doing I/O without getting the mmap lock again and revalidating
> is wise, as the state of things might have changed significantly.
>
> So maybe need a hugepage_vma_revalidate() as well?
Thanks for the feedback. I'll incorporate these comments for v2.
Thanks,
Shivank
^ permalink raw reply
* [PATCH] ftrace: avoid redundant initialization in register_ftrace_direct
From: Menglong Dong @ 2025-11-10 12:18 UTC (permalink / raw)
To: rostedt, jolsa
Cc: mhiramat, mark.rutland, mathieu.desnoyers, jiang.biao,
linux-kernel, linux-trace-kernel, bpf
The FTRACE_OPS_FL_INITIALIZED flag is cleared in register_ftrace_direct,
which can make it initialized by ftrace_ops_init() even if it is already
initialized. It seems that there is no big deal here, but let's still fix
it.
Fixes: f64dd4627ec6 ("ftrace: Add multi direct register/unregister interface")
Signed-off-by: Menglong Dong <dongml2@chinatelecom.cn>
---
kernel/trace/ftrace.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 42bd2ba68a82..efb5ce32298f 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -6043,7 +6043,7 @@ int register_ftrace_direct(struct ftrace_ops *ops, unsigned long addr)
new_hash = NULL;
ops->func = call_direct_funcs;
- ops->flags = MULTI_FLAGS;
+ ops->flags |= MULTI_FLAGS;
ops->trampoline = FTRACE_REGS_ADDR;
ops->direct_call = addr;
--
2.51.2
^ permalink raw reply related
* Re: [PATCH 1/2] mm/khugepaged: do synchronous writeback for MADV_COLLAPSE
From: Lorenzo Stoakes @ 2025-11-10 12:01 UTC (permalink / raw)
To: Shivank Garg
Cc: Andrew Morton, David Hildenbrand, Zi Yan, Baolin Wang,
Liam R . Howlett, Nico Pache, Ryan Roberts, Dev Jain, Barry Song,
Lance Yang, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Zach O'Keefe, linux-mm, linux-kernel, linux-trace-kernel,
Branden Moore
In-Reply-To: <20251110113254.77822-1-shivankg@amd.com>
On Mon, Nov 10, 2025 at 11:32:53AM +0000, Shivank Garg wrote:
> When MADV_COLLAPSE is called on file-backed mappings (e.g., executable
> text sections), the pages may still be dirty from recent writes. The
> current code triggers an async flush via filemap_flush() and returns
> SCAN_FAIL, requiring userspace to retry the operation.
>
> This is problematic for userspace that wants to collapse text pages into
> THPs to reduce ITLB pressure. The first madvise() call always fails with
> EINVAL, and only subsequent calls succeed after writeback completes.
>
> For direct MADV_COLLAPSE calls (!cc->is_khugepaged), perform a synchronous
> writeback using filemap_write_and_wait_range() before scanning the folios.
> This ensures that folios are clean on the first attempt.
>
> Reported-by: Branden Moore <Branden.Moore@amd.com>
> Closes: https://lore.kernel.org/all/4e26fe5e-7374-467c-a333-9dd48f85d7cc@amd.com
> Fixes: 34488399fa08 ("mm/madvise: add file and shmem support to MADV_COLLAPSE")
> Suggested-by: David Hildenbrand <david@redhat.com>
> Signed-off-by: Shivank Garg <shivankg@amd.com>
> ---
> Applies cleanly on:
> 6.18-rc5
> mm-stable:e9a6fb0bc
Please base on mm-unstable. mm-stable is usually out of date until very close to
merge window.
>
>
> mm/khugepaged.c | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index abe54f0043c7..d08ed6eb9ce1 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -21,6 +21,7 @@
> #include <linux/shmem_fs.h>
> #include <linux/dax.h>
> #include <linux/ksm.h>
> +#include <linux/backing-dev.h>
>
> #include <asm/tlb.h>
> #include <asm/pgalloc.h>
> @@ -1845,6 +1846,7 @@ static int collapse_file(struct mm_struct *mm, unsigned long addr,
> struct page *dst;
> struct folio *folio, *tmp, *new_folio;
> pgoff_t index = 0, end = start + HPAGE_PMD_NR;
> + loff_t range_start, range_end;
> LIST_HEAD(pagelist);
> XA_STATE_ORDER(xas, &mapping->i_pages, start, HPAGE_PMD_ORDER);
> int nr_none = 0, result = SCAN_SUCCEED;
> @@ -1853,6 +1855,21 @@ static int collapse_file(struct mm_struct *mm, unsigned long addr,
> VM_BUG_ON(!IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) && !is_shmem);
> VM_BUG_ON(start & (HPAGE_PMD_NR - 1));
>
> + /*
> + * For MADV_COLLAPSE on regular files, do a synchronous writeback
> + * to ensure dirty folios are flushed before we attempt collapse.
> + * This is a best-effort approach to avoid failing on the first
> + * attempt when freshly-written executable text is still dirty.
> + */
> + if (!is_shmem && cc && !cc->is_khugepaged && mapping_can_writeback(mapping)) {
> + range_start = (loff_t)start << PAGE_SHIFT;
> + range_end = ((loff_t)end << PAGE_SHIFT) - 1;
> + if (filemap_write_and_wait_range(mapping, range_start, range_end)) {
> + result = SCAN_FAIL;
> + goto out;
> + }
> + }
I feel this is the wrong level of abstraction.
We explicitly invoke this oth from khugepaged and madvise(..., MADV_COLLAPSE):
khugepaged_scan_mm_slot() / madvise_collapse()
-> hpage_collapse_scan_file()
-> collapse_file()
ofc you are addressing this with the !cc->is_khugepaged, but feels like we'd be
better off just doing it in madvise_collapse().
I wonder also if doing I/O without getting the mmap lock again and revalidating
is wise, as the state of things might have changed significantly.
So maybe need a hugepage_vma_revalidate() as well?
> +
> result = alloc_charge_folio(&new_folio, mm, cc);
> if (result != SCAN_SUCCEED)
> goto out;
>
> base-commit: e9a6fb0bcdd7609be6969112f3fbfcce3b1d4a7c
> --
> 2.43.0
>
Thanks, Lorenzo
^ permalink raw reply
* Re: [PATCH 2/2] mm/khugepaged: return EAGAIN for transient dirty pages in MADV_COLLAPSE
From: Lorenzo Stoakes @ 2025-11-10 11:56 UTC (permalink / raw)
To: Shivank Garg
Cc: Andrew Morton, David Hildenbrand, Zi Yan, Baolin Wang,
Liam R . Howlett, Nico Pache, Ryan Roberts, Dev Jain, Barry Song,
Lance Yang, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Zach O'Keefe, linux-mm, linux-kernel, linux-trace-kernel
In-Reply-To: <20251110113254.77822-3-shivankg@amd.com>
Please, please, please send a cover letter when there's > 1 patch :)
This 2/2 replying to 1/2 is a pain (not your fault that perhaps you're not aware
of typical mm series style but FYI :P)
Also there is some tiny conflict on khugepaged.c in mm-new, but it's literally 1
#include so probably nothing to worry about.
On Mon, Nov 10, 2025 at 11:32:55AM +0000, Shivank Garg wrote:
> When MADV_COLLAPSE encounters dirty file-backed pages, it currently
> returns -EINVAL, this is misleading as EINVAL suggests invalid arguments,
> whereas dirty pages are a transient condition that may resolve on retry.
>
> Introduce SCAN_PAGE_DIRTY and map it to -EAGAIN. For khugepaged, this
> is harmless as it will revisit the range after async writeback completes.
>
> Signed-off-by: Shivank Garg <shivankg@amd.com>
With comments below addressed, LGTM so:
Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> ---
> include/trace/events/huge_memory.h | 3 ++-
> mm/khugepaged.c | 4 +++-
> 2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/include/trace/events/huge_memory.h b/include/trace/events/huge_memory.h
> index dd94d14a2427..9014a9bbe64c 100644
> --- a/include/trace/events/huge_memory.h
> +++ b/include/trace/events/huge_memory.h
> @@ -38,7 +38,8 @@
> EM( SCAN_PAGE_HAS_PRIVATE, "page_has_private") \
> EM( SCAN_STORE_FAILED, "store_failed") \
> EM( SCAN_COPY_MC, "copy_poisoned_page") \
> - EMe(SCAN_PAGE_FILLED, "page_filled")
> + EM(SCAN_PAGE_FILLED, "page_filled") \
> + EMe(SCAN_PAGE_DIRTY, "page_dirty")
>
> #undef EM
> #undef EMe
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index d08ed6eb9ce1..7df329c9c87d 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -60,6 +60,7 @@ enum scan_result {
> SCAN_STORE_FAILED,
> SCAN_COPY_MC,
> SCAN_PAGE_FILLED,
> + SCAN_PAGE_DIRTY,
it feels like a lot to add a scan result for this, but I mean... probably
actually valid.
> };
>
> #define CREATE_TRACE_POINTS
> @@ -1967,7 +1968,7 @@ static int collapse_file(struct mm_struct *mm, unsigned long addr,
> */
> xas_unlock_irq(&xas);
> filemap_flush(mapping);
> - result = SCAN_FAIL;
> + result = SCAN_PAGE_DIRTY;
> goto xa_unlocked;
Hmmm shmem dirty is going to be weird but we also have:
if (!is_shmem && (folio_test_dirty(folio) ||
folio_test_writeback(folio))) {
/*
* khugepaged only works on read-only fd, so this
* folio is dirty because it hasn't been flushed
* since first write.
*/
result = SCAN_FAIL;
goto out_unlock;
}
It's weird though, why would we have writeback, surely handled by swap, and
won't it be like anon, i.e. pretty well always dirty? This comment seems
copy/pasta wrong.
We do need to at least mention in commit message that shmem is explicitly
excluded.
> } else if (folio_test_writeback(folio)) {
> xas_unlock_irq(&xas);
> @@ -2747,6 +2748,7 @@ static int madvise_collapse_errno(enum scan_result r)
> case SCAN_PAGE_LRU:
> case SCAN_DEL_PAGE_LRU:
> case SCAN_PAGE_FILLED:
> + case SCAN_PAGE_DIRTY:
> return -EAGAIN;
> /*
> * Other: Trying again likely not to succeed / error intrinsic to
> --
> 2.43.0
>
^ permalink raw reply
* [PATCH 2/2] mm/khugepaged: return EAGAIN for transient dirty pages in MADV_COLLAPSE
From: Shivank Garg @ 2025-11-10 11:32 UTC (permalink / raw)
To: Andrew Morton, David Hildenbrand, Lorenzo Stoakes
Cc: Zi Yan, Baolin Wang, Liam R . Howlett, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Lance Yang, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, Zach O'Keefe, linux-mm,
linux-kernel, linux-trace-kernel, shivankg
In-Reply-To: <20251110113254.77822-1-shivankg@amd.com>
When MADV_COLLAPSE encounters dirty file-backed pages, it currently
returns -EINVAL, this is misleading as EINVAL suggests invalid arguments,
whereas dirty pages are a transient condition that may resolve on retry.
Introduce SCAN_PAGE_DIRTY and map it to -EAGAIN. For khugepaged, this
is harmless as it will revisit the range after async writeback completes.
Signed-off-by: Shivank Garg <shivankg@amd.com>
---
include/trace/events/huge_memory.h | 3 ++-
mm/khugepaged.c | 4 +++-
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/include/trace/events/huge_memory.h b/include/trace/events/huge_memory.h
index dd94d14a2427..9014a9bbe64c 100644
--- a/include/trace/events/huge_memory.h
+++ b/include/trace/events/huge_memory.h
@@ -38,7 +38,8 @@
EM( SCAN_PAGE_HAS_PRIVATE, "page_has_private") \
EM( SCAN_STORE_FAILED, "store_failed") \
EM( SCAN_COPY_MC, "copy_poisoned_page") \
- EMe(SCAN_PAGE_FILLED, "page_filled")
+ EM(SCAN_PAGE_FILLED, "page_filled") \
+ EMe(SCAN_PAGE_DIRTY, "page_dirty")
#undef EM
#undef EMe
diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index d08ed6eb9ce1..7df329c9c87d 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -60,6 +60,7 @@ enum scan_result {
SCAN_STORE_FAILED,
SCAN_COPY_MC,
SCAN_PAGE_FILLED,
+ SCAN_PAGE_DIRTY,
};
#define CREATE_TRACE_POINTS
@@ -1967,7 +1968,7 @@ static int collapse_file(struct mm_struct *mm, unsigned long addr,
*/
xas_unlock_irq(&xas);
filemap_flush(mapping);
- result = SCAN_FAIL;
+ result = SCAN_PAGE_DIRTY;
goto xa_unlocked;
} else if (folio_test_writeback(folio)) {
xas_unlock_irq(&xas);
@@ -2747,6 +2748,7 @@ static int madvise_collapse_errno(enum scan_result r)
case SCAN_PAGE_LRU:
case SCAN_DEL_PAGE_LRU:
case SCAN_PAGE_FILLED:
+ case SCAN_PAGE_DIRTY:
return -EAGAIN;
/*
* Other: Trying again likely not to succeed / error intrinsic to
--
2.43.0
^ permalink raw reply related
* [PATCH 1/2] mm/khugepaged: do synchronous writeback for MADV_COLLAPSE
From: Shivank Garg @ 2025-11-10 11:32 UTC (permalink / raw)
To: Andrew Morton, David Hildenbrand, Lorenzo Stoakes
Cc: Zi Yan, Baolin Wang, Liam R . Howlett, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Lance Yang, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, Zach O'Keefe, linux-mm,
linux-kernel, linux-trace-kernel, shivankg, Branden Moore
When MADV_COLLAPSE is called on file-backed mappings (e.g., executable
text sections), the pages may still be dirty from recent writes. The
current code triggers an async flush via filemap_flush() and returns
SCAN_FAIL, requiring userspace to retry the operation.
This is problematic for userspace that wants to collapse text pages into
THPs to reduce ITLB pressure. The first madvise() call always fails with
EINVAL, and only subsequent calls succeed after writeback completes.
For direct MADV_COLLAPSE calls (!cc->is_khugepaged), perform a synchronous
writeback using filemap_write_and_wait_range() before scanning the folios.
This ensures that folios are clean on the first attempt.
Reported-by: Branden Moore <Branden.Moore@amd.com>
Closes: https://lore.kernel.org/all/4e26fe5e-7374-467c-a333-9dd48f85d7cc@amd.com
Fixes: 34488399fa08 ("mm/madvise: add file and shmem support to MADV_COLLAPSE")
Suggested-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Shivank Garg <shivankg@amd.com>
---
Applies cleanly on:
6.18-rc5
mm-stable:e9a6fb0bc
mm/khugepaged.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index abe54f0043c7..d08ed6eb9ce1 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -21,6 +21,7 @@
#include <linux/shmem_fs.h>
#include <linux/dax.h>
#include <linux/ksm.h>
+#include <linux/backing-dev.h>
#include <asm/tlb.h>
#include <asm/pgalloc.h>
@@ -1845,6 +1846,7 @@ static int collapse_file(struct mm_struct *mm, unsigned long addr,
struct page *dst;
struct folio *folio, *tmp, *new_folio;
pgoff_t index = 0, end = start + HPAGE_PMD_NR;
+ loff_t range_start, range_end;
LIST_HEAD(pagelist);
XA_STATE_ORDER(xas, &mapping->i_pages, start, HPAGE_PMD_ORDER);
int nr_none = 0, result = SCAN_SUCCEED;
@@ -1853,6 +1855,21 @@ static int collapse_file(struct mm_struct *mm, unsigned long addr,
VM_BUG_ON(!IS_ENABLED(CONFIG_READ_ONLY_THP_FOR_FS) && !is_shmem);
VM_BUG_ON(start & (HPAGE_PMD_NR - 1));
+ /*
+ * For MADV_COLLAPSE on regular files, do a synchronous writeback
+ * to ensure dirty folios are flushed before we attempt collapse.
+ * This is a best-effort approach to avoid failing on the first
+ * attempt when freshly-written executable text is still dirty.
+ */
+ if (!is_shmem && cc && !cc->is_khugepaged && mapping_can_writeback(mapping)) {
+ range_start = (loff_t)start << PAGE_SHIFT;
+ range_end = ((loff_t)end << PAGE_SHIFT) - 1;
+ if (filemap_write_and_wait_range(mapping, range_start, range_end)) {
+ result = SCAN_FAIL;
+ goto out;
+ }
+ }
+
result = alloc_charge_folio(&new_folio, mm, cc);
if (result != SCAN_SUCCEED)
goto out;
base-commit: e9a6fb0bcdd7609be6969112f3fbfcce3b1d4a7c
--
2.43.0
^ permalink raw reply related
* Re: [PATCH 3/3] rv: Add explicit lockdep context for reactors
From: Nam Cao @ 2025-11-10 10:53 UTC (permalink / raw)
To: Thomas Weißschuh, Gabriele Monaco
Cc: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
linux-trace-kernel, linux-kernel
In-Reply-To: <20251015115714-e8e69ed8-a5e4-4139-8d6d-7f2487674e68@linutronix.de>
Thomas Weißschuh <thomas.weissschuh@linutronix.de> writes:
> On Tue, Oct 14, 2025 at 04:50:18PM +0200, Gabriele Monaco wrote:
>> I don't mean to really sacrifice accuracy, DA monitors are disabled after a
>> reaction. This comes from the assumption that the model becomes invalid, so
>> whatever comes after might be meaningless. Monitors restart as soon as we are
>> sure we reached the initial state.
>> In this case, it already doesn't make sense to monitor events triggered by
>> reactors.
I can get behind this idea. I would even suspend tracepoints entirely
during tracepoint handling. Currently there is nothing protecting LTL's
internal data if multiple CPUs touch it internally. It needs a (raw)
spin lock, but cannot use it because spin locks step on some
tracepoints.
Suspending tracepoints during tracepoint handling would make lots of
things accessible to RV.
>> LTL is a bit more complex, so it might make sense to continue monitoring just
>> after a reaction, but I'm not sure how useful that is.
>
> Ack.
>
> It is still possible to manually re-enable all monitors through sysfs, correct?
> That is needed for the kind of testing I have in mind.
>
> Do we still consider these hypothetical tracepoint loops a blocker for this
> patch series? In my opinion the usage of lockdep does not exacerbate the risk.
Gabriele, if you want to take these patches:
Acked-by: Nam Cao <namcao@linutronix.de>
as the lockdep's tracepoints are not used by RV at the moment, so it
should be fine. And the patch does help with RV development.
But we need to investigate further into suspending monitor or suspending
tracepoints. If that is not feasible, we can revert this patch in the
future.
Nam
^ permalink raw reply
* Re: [RFC PATCH v1 06/37] KVM: guest_memfd: Update kvm_gmem_populate() to use gmem attributes
From: Yan Zhao @ 2025-11-10 10:01 UTC (permalink / raw)
To: Ackerley Tng
Cc: cgroups, kvm, linux-doc, linux-fsdevel, linux-kernel,
linux-kselftest, linux-mm, linux-trace-kernel, x86, akpm,
binbin.wu, bp, brauner, chao.p.peng, chenhuacai, corbet,
dave.hansen, dave.hansen, david, dmatlack, erdemaktas, fan.du,
fvdl, haibo1.xu, hannes, hch, hpa, hughd, ira.weiny,
isaku.yamahata, jack, james.morse, jarkko, jgg, jgowans, jhubbard,
jroedel, jthoughton, jun.miao, kai.huang, keirf, kent.overstreet,
liam.merwick, maciej.wieczor-retman, mail, maobibo,
mathieu.desnoyers, maz, mhiramat, mhocko, mic, michael.roth,
mingo, mlevitsk, mpe, muchun.song, nikunj, nsaenz, oliver.upton,
palmer, pankaj.gupta, paul.walmsley, pbonzini, peterx, pgonda,
prsampat, pvorel, qperret, richard.weiyang, rick.p.edgecombe,
rientjes, rostedt, roypat, rppt, seanjc, shakeel.butt, shuah,
steven.price, steven.sistare, suzuki.poulose, tabba, tglx,
thomas.lendacky, vannapurve, vbabka, viro, vkuznets, wei.w.wang,
will, willy, wyihan, xiaoyao.li, yilun.xu, yuzenghui, zhiquan1.li
In-Reply-To: <a3795f7fb4f785ced19abe18c2f33aa478c4d202.1760731772.git.ackerleytng@google.com>
> #ifdef CONFIG_HAVE_KVM_ARCH_GMEM_POPULATE
> +static bool kvm_gmem_range_is_private(struct gmem_inode *gi, pgoff_t index,
> + size_t nr_pages, struct kvm *kvm, gfn_t gfn)
> +{
> + pgoff_t end = index + nr_pages - 1;
> + void *entry;
> +
> + if (vm_memory_attributes)
> + return kvm_range_has_vm_memory_attributes(kvm, gfn, gfn + nr_pages,
> + KVM_MEMORY_ATTRIBUTE_PRIVATE,
> + KVM_MEMORY_ATTRIBUTE_PRIVATE);
Can't compile kvm_range_has_vm_memory_attributes() if
CONFIG_KVM_VM_MEMORY_ATTRIBUTES is not set.
^ permalink raw reply
* Re: [PATCH 2/2] tracing: Use switch statement instead of ifs in set_tracer_flag()
From: Masami Hiramatsu @ 2025-11-10 5:48 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-kernel, linux-trace-kernel, Masami Hiramatsu, Mark Rutland,
Mathieu Desnoyers, Andrew Morton
In-Reply-To: <20251106003501.890298562@kernel.org>
On Wed, 05 Nov 2025 19:33:26 -0500
Steven Rostedt <rostedt@kernel.org> wrote:
> From: Steven Rostedt <rostedt@goodmis.org>
>
> The "mask" passed in to set_trace_flag() has a single bit set. The
> function then checks if the mask is equal to one of the option masks and
> performs the appropriate function associated to that option.
>
> Instead of having a bunch of "if ()" statement, use a "switch ()"
> statement instead to make it cleaner and a bit more optimal.
>
> No function changes.
>
Looks good to me.
Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
BTW, set_tracer_flag() seems to expect to modify only one bit.
If we can count the number of its in @mask and reject if it is
not 1, we can use bit-mask instead of the first switch()?
if (!mask || /* mask has no bit */
(mask & ~(1 << (ffs64(mask) - 1)))) /* mask has more than 2 bits */
return -EINVAL;
Thanks,
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> ---
> kernel/trace/trace.c | 38 +++++++++++++++++++++++---------------
> 1 file changed, 23 insertions(+), 15 deletions(-)
>
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 07bd10808277..8460bec9f263 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -5251,11 +5251,13 @@ int trace_keep_overwrite(struct tracer *tracer, u64 mask, int set)
>
> int set_tracer_flag(struct trace_array *tr, u64 mask, int enabled)
> {
> - if ((mask == TRACE_ITER(RECORD_TGID)) ||
> - (mask == TRACE_ITER(RECORD_CMD)) ||
> - (mask == TRACE_ITER(TRACE_PRINTK)) ||
> - (mask == TRACE_ITER(COPY_MARKER)))
> + switch (mask) {
> + case TRACE_ITER(RECORD_TGID):
> + case TRACE_ITER(RECORD_CMD):
> + case TRACE_ITER(TRACE_PRINTK):
> + case TRACE_ITER(COPY_MARKER):
> lockdep_assert_held(&event_mutex);
> + }
>
> /* do nothing if flag is already set */
> if (!!(tr->trace_flags & mask) == !!enabled)
> @@ -5266,7 +5268,8 @@ int set_tracer_flag(struct trace_array *tr, u64 mask, int enabled)
> if (tr->current_trace->flag_changed(tr, mask, !!enabled))
> return -EINVAL;
>
> - if (mask == TRACE_ITER(TRACE_PRINTK)) {
> + switch (mask) {
> + case TRACE_ITER(TRACE_PRINTK):
> if (enabled) {
> update_printk_trace(tr);
> } else {
> @@ -5283,9 +5286,9 @@ int set_tracer_flag(struct trace_array *tr, u64 mask, int enabled)
> if (printk_trace == tr)
> update_printk_trace(&global_trace);
> }
> - }
> + break;
>
> - if (mask == TRACE_ITER(COPY_MARKER)) {
> + case TRACE_ITER(COPY_MARKER):
> update_marker_trace(tr, enabled);
> /* update_marker_trace updates the tr->trace_flags */
> return 0;
> @@ -5296,10 +5299,12 @@ int set_tracer_flag(struct trace_array *tr, u64 mask, int enabled)
> else
> tr->trace_flags &= ~mask;
>
> - if (mask == TRACE_ITER(RECORD_CMD))
> + switch (mask) {
> + case TRACE_ITER(RECORD_CMD):
> trace_event_enable_cmd_record(enabled);
> + break;
>
> - if (mask == TRACE_ITER(RECORD_TGID)) {
> + case TRACE_ITER(RECORD_TGID):
>
> if (trace_alloc_tgid_map() < 0) {
> tr->trace_flags &= ~TRACE_ITER(RECORD_TGID);
> @@ -5307,24 +5312,27 @@ int set_tracer_flag(struct trace_array *tr, u64 mask, int enabled)
> }
>
> trace_event_enable_tgid_record(enabled);
> - }
> + break;
>
> - if (mask == TRACE_ITER(EVENT_FORK))
> + case TRACE_ITER(EVENT_FORK):
> trace_event_follow_fork(tr, enabled);
> + break;
>
> - if (mask == TRACE_ITER(FUNC_FORK))
> + case TRACE_ITER(FUNC_FORK):
> ftrace_pid_follow_fork(tr, enabled);
> + break;
>
> - if (mask == TRACE_ITER(OVERWRITE)) {
> + case TRACE_ITER(OVERWRITE):
> ring_buffer_change_overwrite(tr->array_buffer.buffer, enabled);
> #ifdef CONFIG_TRACER_MAX_TRACE
> ring_buffer_change_overwrite(tr->max_buffer.buffer, enabled);
> #endif
> - }
> + break;
>
> - if (mask == TRACE_ITER(PRINTK)) {
> + case TRACE_ITER(PRINTK):
> trace_printk_start_stop_comm(enabled);
> trace_printk_control(enabled);
> + break;
> }
>
> return 0;
> --
> 2.51.0
>
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply
* Re: [PATCH 1/2] tracing: Exit out immediately after update_marker_trace()
From: Masami Hiramatsu @ 2025-11-10 5:24 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-kernel, linux-trace-kernel, Masami Hiramatsu, Mark Rutland,
Mathieu Desnoyers, Andrew Morton
In-Reply-To: <20251106003501.726406870@kernel.org>
On Wed, 05 Nov 2025 19:33:25 -0500
Steven Rostedt <rostedt@kernel.org> wrote:
> From: Steven Rostedt <rostedt@goodmis.org>
>
> The call to update_marker_trace() in set_tracer_flag() performs the update
> to the tr->trace_flags. There's no reason to perform it again after it is
> called. Return immediately instead.
>
Looks good to me.
Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Thanks,
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> ---
> kernel/trace/trace.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index dea1566b3301..07bd10808277 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -5285,8 +5285,11 @@ int set_tracer_flag(struct trace_array *tr, u64 mask, int enabled)
> }
> }
>
> - if (mask == TRACE_ITER(COPY_MARKER))
> + if (mask == TRACE_ITER(COPY_MARKER)) {
> update_marker_trace(tr, enabled);
> + /* update_marker_trace updates the tr->trace_flags */
> + return 0;
> + }
>
> if (enabled)
> tr->trace_flags |= mask;
> --
> 2.51.0
>
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ 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