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."
