Re: [PATCH 0/5] mm: limit pagecache folio refcount

[David Hildenbrand (Arm)]

On 11/6/26 20:49, Josef Bacik wrote:
> On Thu, Nov 05, 2026 at 02:10:09PM +0100, David Hildenbrand (Arm) wrote:
>> Users can create a lot of folio references to pagecache folios, making
>> it possible to overflow the refcount of large folios.
> 
> This is a summary of my Claude review of everything, minus what has
> already been said on the thread.  The numbers are from a TCG guest with
> the limits lowered to user 6000 and admin 9000.

Thanks.

> 
> 1) Small folios, 1/5 and 5/5
> 
> On an order-0 folio one FOLL_PIN is worth GUP_PIN_COUNTING_BIAS
> references, so about 2M pins reach the limit.  That used to make GUP
> refuse more pins, now it is also SIGBUS on new faults and failing fork
> and splice for everybody else.  6 io_uring registered buffers on one
> order-0 shmem page (ref 6146) and another task gets SIGBUS mapping it.
> Mappings alone can't get a small folio near the limit, so could the
> mapping and splice sites skip small folios?

Sure, our limit slightly lower, but not as much as we would care for small
folios? I assume your LLM is confused about the actual limits vs. your fake limits?

> 
> 2) Migration, 1/5
> 
> remove_migration_pte() and remove_migration_pmd() check the limit for
> whoever migrates, not for the owner of the mapping, and kcompactd always
> gets the user limit.  With mappings in the admin reserve the next
> migration drops whatever is over the user limit, in rmap walk order.

Right.

> 
> At ref 7681, move_pages(MPOL_MF_MOVE_ALL) without CAP_SYS_RESOURCE left
> 1682 of 7168 PTEs unrestored, including all 512 of an mlock()ed VMA, and
> touching them again got SIGBUS for all 1682.  Could the restore check
> against the admin limit only?

An actual admin access would, however, not SIGBUS. Sure, we harm mlock() as well
but that's just expected.

I don't see why the extra complexity for this temporary solution would be warranted.

> 
> 3) fork, 1/5
> 
> If the VMA is only copied because it has an anon rmap, the pagecache
> PTEs could be left out to refault in the child, like copy_huge_pmd()
> does, instead of failing the fork.  Only the VM_COPY_ON_FORK cases need
> the error.

That could be done indeed, although I am not sure if really relevant in practice.

I'll see if it can easily be done.

> 
> 4) splice, 2/5
> 
> filemap_splice_read() fails where read() still works.  sendfile(), the
> copy_file_range() fallback and overlayfs copy-up get -ENOMEM, and nfsd
> turns it into nfserr_jukebox so the client retries for as long as the
> folio stays at the limit.  Could do_splice_read() fall back to
> copy_splice_read(), like it does for O_DIRECT and DAX?

Same as the above. This solution here is not going to be the solution we'll have
upstream soon, so I try to keep it as small as possible.

I'll see if it can easily be done.

> 
> backing_file_splice_read() runs capable() under ctx->cred.  overlayfs
> lowers CAP_SYS_RESOURCE there, so root never gets the reserve, and fuse
> passthrough uses the server's cred, so an unprivileged reader may.
> 
> 5) Errors
> 
> The one condition comes out as SIGBUS, -ENOMEM, -EFAULT (tee, pipe to
> pipe, MADV_POPULATE_READ, process_vm_readv), -EIO (/proc/pid/mem),
> -EOVERFLOW (vm_insert_page) and -EINVAL (memfd_pin_folios,
> fuse_dev_splice_write).  Nothing is logged or counted, so there is
> nothing that points at the folio.

Right, again, a temporary solution which we can reasonably backport to stable.

> 
> 6) khugepaged, 1/5
> 
> try_collapse_pte_mapped_thp() clears the PTEs and drops their
> references, and then do_set_pmd() can refuse, so the mapping is gone
> although the collapse would have lowered the refcount.

Am I wrong or #nobodycares?

> 
> 7) Changelogs, comments, stable
> 
> - 3/5 doesn't say what goes wrong without it.  It is tagged for
>   10ebac4f95e7 (v6.9) while 1, 2 and 5 go back to v4.8, so 6.6 and older
>   get the limits without the flush, and it doesn't say why that is fine.

Yeah, probably it should go further back. The PTE batching case was just one
scenario. It was possible when batching over VMAs before (which my commit
handles properly).


> - 3/5 leaves the "queue is (now) full" comments in asm-generic/tlb.h and
>   above __tlb_remove_folio_pages_size() stale.

That seems easy to fix.

> - The ufolios_count fix in 4/5 doesn't depend on the rest of the series,
>   so it could be its own patch for stable.

5/5 makes it easier to trigger. Which is how it was found in the first place.

> - 5/5 needs 1/5 and 4/5, which isn't stated for stable.

lol, they have the same Fixes tags, that sure is good enough in one series.

> - After 5/5 the check is the only guard for pins on small folios, the
>   kernel-doc calls it "mostly unnecessary" for them.  The comment in
>   folio_maybe_dma_pinned() still describes the negative refcount scheme.

Yeah, I'm trying not to change to much becuase the while thing will get ripped
out soon again.

-- 
Cheers,

David