linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Nico Pache <npache@redhat.com>
To: Lance Yang <lance.yang@linux.dev>
Cc: "Garg, Shivank" <shivankg@amd.com>,
	linux-mm@kvack.org,  Lorenzo Stoakes <lorenzo.stoakes@oracle.com>,
	Ryan Roberts <ryan.roberts@arm.com>,
	 linux-kernel@vger.kernel.org, Zi Yan <ziy@nvidia.com>,
	Dev Jain <dev.jain@arm.com>,
	 Baolin Wang <baolin.wang@linux.alibaba.com>,
	Jann Horn <jannh@google.com>,
	 David Hildenbrand <david@redhat.com>,
	"Liam R. Howlett" <Liam.Howlett@oracle.com>,
	 Barry Song <baohua@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	zokeefe@google.com,  Vlastimil Babka <vbabka@suse.cz>
Subject: Re: madvise(MADV_COLLAPSE) fails with EINVAL on dirty file-backed text pages
Date: Thu, 6 Nov 2025 06:03:24 -0700	[thread overview]
Message-ID: <CAA1CXcBktafjRFyD8JD-akVutzS-jQEFFC=kJtO3ewLg0Kk0zw@mail.gmail.com> (raw)
In-Reply-To: <1783f8fc-6b9b-422e-999e-2a6f58d90807@linux.dev>

On Thu, Nov 6, 2025 at 5:55 AM Lance Yang <lance.yang@linux.dev> wrote:
>
>
>
> On 2025/11/6 20:16, Garg, Shivank wrote:
> > Hi All,
>
> Hi Shivank,
>
> Good catch and a really clear analysis - thanks!
+1!
> >
> > I've been investigating an issue with madvise(MADV_COLLAPSE) for TEXT pages
> > when CONFIG_READ_ONLY_THP_FOR_FS=y is enabled, and would like to discuss the
> > current behavior and improvements.
> >
> > Problem:
> > When attempting to collapse read-only file-backed TEXT sections into THPs
> > using madvise(MADV_COLLAPSE), the operation fails with EINVAL if the pages
> > are marked dirty.
> > madvise(aligned_start, aligned_size, MADV_COLLAPSE) -> returns -1 and errno = -22
> >
> > Subsequent calls to madvise(MADV_COLLAPSE) succeed because the first madvise
> > attempt triggers filemap_flush() which initiates async writeback of the dirty folios.
> >
> > Root Cause:
> > The failure occurs in mm/khugepaged.c:collapse_file():
> > } else if (folio_test_dirty(folio)) {
> >      /*
> >       * khugepaged only works on read-only fd,
> >       * so this page is dirty because it hasn't
> >       * been flushed since first write. There
> >       * won't be new dirty pages.
> >       *
> >       * Trigger async flush here and hope the
> >       * writeback is done when khugepaged
> >       * revisits this page.
> >       */
> >      xas_unlock_irq(&xas);
> >      filemap_flush(mapping);
> >      result = SCAN_FAIL;
> >      goto xa_unlocked;
> > }
> >
> > Why the text pages are dirty?
> > It initially seemed unusual for a read-only text section to be marked as dirty, but
> > this was actually confirmed by /proc/pid/smaps.
> >
> > 55bc90200000-55bc91200000 r-xp 00400000 07:00 133                        /mnt/xfs-mnt/large_binary_thp
> > Size:              16384 kB
> > KernelPageSize:        4 kB
> > MMUPageSize:           4 kB
> > Rss:                 256 kB
> > Pss:                 256 kB
> > Pss_Dirty:           256 kB
> > Shared_Clean:          0 kB
> > Shared_Dirty:          0 kB
> > Private_Clean:         0 kB
> > Private_Dirty:       256 kB
> >
> > /proc/pid/smaps (before calling MADV_COLLAPSE) showing Private_Dirty pages in r-xp mappings.
> > This may be due to dynamic linker and relocations that occurred during program loading.
> >
> > Reproduction using XFS/EXT4:
> >
> > 1. Compile a test binary with madvise(MADV_COLLAPSE), ensuring the load TEXT segment is
> >     2MB-aligned and sized to a multiple of 2MB.
> >    Type           Offset   VirtAddr           PhysAddr           FileSiz  MemSiz   Flg Align
> > LOAD           0x400000 0x0000000000400000 0x0000000000400000 0x1000000 0x1000000 R E 0x200000
> >
> > 2. Create and mount the XFS/EXT4 fs:
> >     dd if=/dev/zero of=/tmp/xfs-test.img bs=1M count=1024
> >     losetup -f --show /tmp/xfs-test.img  # output: /dev/loop0
> >     mkfs.xfs -f /dev/loop0
> >     mkdir -p /mnt/xfs-mnt
> >     mount /dev/loop0 /mnt/xfs-mnt
> > 3. Copy the binaries to /mnt/xfs-mnt and execute.
> > 4. Returns -EINVAL on first run, then run successfully on subsequent run. (100% reproducible)
> > 5. To reproduce again; reboot/kexec and repeat from step 2.
> >
> > Workaround:
> > 1. Manually flush dirty pages before calling madvise(MADV_COLLAPSE):
> >       int fd = open("/proc/self/exe", O_RDONLY);
> >       if (fd >= 0) {
> >               fsync(fd);
> >               close(fd);
> >       }
> >       // Now madvise(MADV_COLLAPSE) succeeds
> > 2. Alternatively, retrying madvise_collapse on EINVAL failure also work.
> >
> > Problems with Current Behavior:
> > 1. Confusing Error Code: The syscall returns EINVAL which typically indicates invalid arguments
> >     rather than a transient condition that could succeed on retry.
> >
> > 2. Non-Transparent Handling: Users are unaware they need to flush dirty pages manually. Current
> >     madvise_collapse assumes the caller is khugepaged (as per code snippet comment) which will revisit
> >     the page. However, when called via madvise(MADV_COLLAPSE), the userspace program typically don't
> >     retry, making the async flush ineffective. Should we differentiate between madvise and khugepaged
> >     behavior for MADV_COLLAPSE?
> >
> > Would appreciate thoughts on the best approach to address this issue.
>
> Just throwing out a couple of ideas ...
>
> We could just switch the return code to EAGAIN in the MADV_COLLAPSE
> path. At least that
> gives the right hint that retrying is an option ;)

Hey! I agree with Lance here, it seems the solution would be to return
something other than SCAN_FAIL in collapse_file(), then in
madvise_collapse_errno() catch this error and return EAGAIN. We could
use SCAN_PAGE_COUNT which will cause a EAGAIN, or we could create a
new result enum.

Cheers,
-- Nico
>
> Or, what if we just handle it inside the syscall? When we hit a dirty
> page, we wait for
> the writeback to finish and then try again right away. The call might be
> a little slower,
> but MADV_COLLAPSE is best effort, right? That seems worth the trouble ...
>
> Cheers,
> Lance
>



  reply	other threads:[~2025-11-06 13:03 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-06 12:16 Garg, Shivank
2025-11-06 12:55 ` Lance Yang
2025-11-06 13:03   ` Nico Pache [this message]
2025-11-06 16:32 ` Ryan Roberts
2025-11-06 16:55   ` Liam R. Howlett
2025-11-06 17:17     ` Lorenzo Stoakes
2025-11-06 21:05       ` David Hildenbrand (Red Hat)
2025-11-07  8:51         ` Garg, Shivank
2025-11-07  9:12           ` David Hildenbrand (Red Hat)
2025-11-07 10:09             ` Lance Yang
2025-11-07 10:10             ` Lorenzo Stoakes
2025-11-07 12:46               ` Garg, Shivank
2025-11-07 10:09         ` Lorenzo Stoakes
2025-11-07 12:50           ` Lorenzo Stoakes
2025-11-06 20:32 ` Yang Shi
2025-11-07  9:44   ` Garg, Shivank

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAA1CXcBktafjRFyD8JD-akVutzS-jQEFFC=kJtO3ewLg0Kk0zw@mail.gmail.com' \
    --to=npache@redhat.com \
    --cc=Liam.Howlett@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=baohua@kernel.org \
    --cc=baolin.wang@linux.alibaba.com \
    --cc=david@redhat.com \
    --cc=dev.jain@arm.com \
    --cc=jannh@google.com \
    --cc=lance.yang@linux.dev \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=lorenzo.stoakes@oracle.com \
    --cc=ryan.roberts@arm.com \
    --cc=shivankg@amd.com \
    --cc=vbabka@suse.cz \
    --cc=ziy@nvidia.com \
    --cc=zokeefe@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox