On 8/29/22 05:07, David Hildenbrand wrote:
>> +/**
>> + * pin_user_page() - apply a FOLL_PIN reference to a page
>> + *
>> + * @page: the page to be pinned.
>> + *
>> + * This is similar to get_user_pages(), except that the page's refcount is
>> + * elevated using FOLL_PIN, instead of FOLL_GET.
Actually, my commit log has a more useful documentation of this routine,
and given the questions below, I think I'll change to that:
* pin_user_page() is an externally-usable version of try_grab_page(), but with
* semantics that match get_page(), so that it can act as a drop-in replacement
* for get_page().
*
* pin_user_page() elevates a page's refcount using FOLL_PIN rules. This means
* that the caller must release the page via unpin_user_page().
>> + *
>> + * IMPORTANT: The caller must release the page via unpin_user_page().
>> + *
>> + */
>> +void pin_user_page(struct page *page)
>> +{
>> + struct folio *folio = page_folio(page);
>> +
>> + WARN_ON_ONCE(folio_ref_count(folio) <= 0);
>> +
>
> We should warn if the page is anon and !exclusive.
That would be sort of OK, because pin_user_page() is being created
specifically for file system (O_DIRECT cases) use, and so the pages
should mostly be file-backed, rather than anon. Although I'm a little
vague about whether all of these iov_iter cases are really always
file-backed pages, especially for cases such as splice(2) to an
O_DIRECT-opened file, that Al Viro mentioned [1].
Can you walk me through the reasoning for why we need to keep out
anon shared pages?
>
> I assume the intend is to use pin_user_page() only to duplicate pins, right?
>
Well, yes or no, depending on your use of the term "pin":
pin_user_page() is used on a page that already has a refcount >= 1 (so
no worries about speculative pinning should apply here), but the page
does not necessarily have any FOLL_PIN's applied to it yet (so it's not
"pinned" in the FOLL_PIN sense).
[1] https://lore.kernel.org/all/Ywq5VrSrY341UVpL@ZenIV/
thanks,
--
John Hubbard
NVIDIA
