Re: Splitting pinned folios

[John Hubbard <jhubbard@nvidia.com>]

On 3/13/24 08:27, Matthew Wilcox wrote:
> 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!

That's a reasonable initial step, yes. haha :)

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

Thanks David for responding so quickly with these gup/pup answers. Perhaps I
can fix up that documentation comment a bit.

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


Yes! In try_grab_folio(), that behavior is documented, although due to
code movement and refactoring, the comment refers to behavior a few lines
above it:

	/*
	 * When pinning a large folio, use an exact count to track it.
	 *
	 * However, be sure to *also* increment the normal folio
	 * refcount field at least once, so that the folio really
	 * is pinned.  That's why the refcount from the earlier
	 * try_get_folio() is left intact.
	 */


thanks,
-- 
John Hubbard
NVIDIA