Re: Splitting pinned folios

[Matthew Wilcox <willy@infradead.org>]

On Wed, Mar 13, 2024 at 10:20:46AM +0100, David Hildenbrand wrote:
> On 13.03.24 04:16, Matthew Wilcox wrote:
> > On Tue, Mar 12, 2024 at 06:23:43PM -0700, Jane Chu wrote:
> > > I noticed this recently
> > 
> > OK, this is entirely different, so I'm going to start a new thread ;-)
> > 
> > >   * GUP pin and PG_locked transferred to @page. Rest subpages can be freed if
> > >   * they are not mapped.
> > >   *
> > >   * Returns 0 if the hugepage is split successfully.
> > >   * Returns -EBUSY if the page is pinned or if anon_vma disappeared from under
> > >   * us.
> > >   */
> > > int split_huge_page_to_list(struct page *page, struct list_head *list)
> > > {
> > > 
> > > I have a test case with poisoned shmem THP page that was mlocked and
> > > 
> > > GUP pinned (FOLL_LONGTERM|FOLL_WRITE), but the split succeeded.
> > 
> > I'm going to blame John for this!
> 
> The description is wrong. Whoever calls split_huge_page_to_list() must hold
> a folio reference.
> 
> That folio reference will be transferred to @page (not the head page) once
> split. So @page can be used by the caller after the split succeeded.
> 
> 
> > There's no reference to pincount
> > anywhere in huge_memory.c, so I have no clue how this comment is even
> 
> Each pincount increment/decrement must be paired with a folio refcount
> increment. Therefore, no pincount checks are required.

I'd forgotten that.  Any GUP pin will force failure of split.  So I
don't know what Jane is seeing.  memory_failure() tries to split THPs
and outputs an error if it can't.  It doesn't try to handle them.

It probably should, but that's a subject for a different thread.

> In essence: we expect on a folio after completely unmapping it:
> * 1 reference from the caller of split_huge_page_to_list()
> * pagecache: 1 reference per subpage from the pagecache

 * 1 reference if the folio has private data (thank you, bufferheads)

Oh.  Is that the bug?  can_split_folio() doesn't know that.  This
usually isn't a problem because, eg, truncate_inode_partial_folio()
will remove the private data before calling split_folio().  But
memory-failure doesn't know about that rule ...

No, that's not it.  shmem doesn't use the folio private flag.  It's
still a bug, but it's not Jane's bug.

Jane, can we have the results of calling dump_page() on the page?

> Reading "I have a test case with poisoned shmem THP page that was mlocked
> and GUP pinned (FOLL_LONGTERM|FOLL_WRITE), but the split succeeded."