Re: [PATCHv2 1/2] mm/memory: Do not populate page table entries beyond i_size

[Kiryl Shutsemau <kirill@shutemov.name>]

On Wed, Oct 29, 2025 at 10:59:24PM -0700, Hugh Dickins wrote:
> On Wed, 29 Oct 2025, Kiryl Shutsemau wrote:
> > On Wed, Oct 29, 2025 at 01:31:45AM -0700, Hugh Dickins wrote:
> > > On Mon, 27 Oct 2025, David Hildenbrand wrote:
> > > ...
> > > > 
> > > > Just so we are on the same page: this is not about which folio sizes we
> > > > allocate (like what Baolin fixed) but what/how much to map.
> > > > 
> > > > I guess this patch here would imply the following changes
> > > > 
> > > > 1) A file with a size that is not PMD aligned will have the last (unaligned
> > > > part) not mapped by PMDs.
> > > > 
> > > > 2) Once growing a file, the previously-last-part would not be mapped by PMDs.
> > > 
> > > Yes, the v2 patch was so, and the v3 patch fixes it.
> > > 
> > > khugepaged might have fixed it up later on, I suppose.
> > > 
> > > Hmm, does hpage_collapse_scan_file() or collapse_pte_mapped_thp()
> > > want a modification, to prevent reinserting a PMD after a failed
> > > non-shmem truncation folio_split?  And collapse_file() after a
> > > successful non-shmem truncation folio_split?
> > 
> > I operated from an assumption that file collapse is still lazy as I
> > wrote it back it the days and doesn't install PMDs. It *seems* to be
> > true for khugepaged, but not MADV_COLLAPSE.
> > 
> > Hm...
> > 
> > > Conversely, shouldn't MADV_COLLAPSE be happy to give you a PMD
> > > if the map size permits, even when spanning EOF?
> > 
> > Filesystem folks say allowing the folio to be mapped beyond
> > round_up(i_size, PAGE_SIZE) is a correctness issue, not only POSIX
> > violation.
> > 
> > I consider dropping 'install_pmd' from collapse_pte_mapped_thp() so the
> > fault path is source of truth of whether PMD can be installed or not.
> 
> (Didn't you yourself just recently enhance that?)

I failed to adjust my mental model :P

> > 
> > Objections?
> 
> Yes, I would probably object (or perhaps want to allow until EOF);
> but now it looks to me like we can agree no change is needed there.
> 
> I was mistaken in raising those khugepaged/MADV_COLLAPSE doubts,
> because file_thp_enabled(vma) is checked in the !shmem !anonymous
> !dax case, and file_thp_enabled(vma) still limits to
> CONFIG_READ_ONLY_THP_FOR_FS=y, refusing to allow collapse if anyone
> has the file open for writing (and you cannot truncate or hole-punch
> without write permission); and pagecache is invalidated afterwards
> if there are any THPs when reopened for writing (presumably for
> page_mkwrite()-ish consistency reasons, which you interestingly
> pointed to in another mail where I had worried about ENOSPC after
> split failure).
> 
> But shmem is simple, does not use page_mkwrite(), and is fine to
> continue with install_pmd here, just as it's fine to continue
> with huge page spanning EOF as you're now allowing in v3.
> 
> But please double check my conclusion there, it's so easy to
> get lost in the maze of hugepage permissions and prohibitions.

Your analysis looks correct to me.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov