On 10/31/19 4:15 PM, Ira Weiny wrote:
On Wed, Oct 30, 2019 at 03:49:16PM -0700, John Hubbard wrote:
...
- FOLL_PIN indicates that a special kind of tracking (not just page->_refcount,
- but an additional pin counting system) will be invoked. This is intended for
- anything that gets a page reference and then touches page data (for example,
- Direct IO). This lets the filesystem know that some non-file-system entity is
- potentially changing the pages' data. In contrast to FOLL_GET (whose pages
- are released via put_page()), FOLL_PIN pages must be released, ultimately, by
- a call to put_user_page().
- FOLL_PIN is similar to FOLL_GET: both of these pin pages. They use different
- and separate refcounting mechanisms, however, and that means that each has
- its own acquire and release mechanisms:
FOLL_GET: get_user_pages*() to acquire, and put_page() to release.
FOLL_PIN: pin_user_pages*() or pin_longterm_pages*() to acquire, and
put_user_pages to release.
- FOLL_PIN and FOLL_GET are mutually exclusive.
You mean the flags are mutually exclusive for any single call, correct? Because my first thought was that you meant that a page which was pin'ed can't be "got". Which I don't think is true or necessary...
Yes, you are correct. And yes you can absolutely mix get_user_pages() and pin_user_pages() calls on the same page(s).
OK, I'll change the wording to "mutually exclusive for a given function call".
- Please see Documentation/vm/pin_user_pages.rst for more information.
NIT: I think we should include this file as part of this patch...
heh. I kept hopping back and forth on this, because I've seen other patchsets that often put Documentation/ into its own patch. But you're right, of course: it's not right to refer to items that are not here until a later patch. I'll merge patch 19 into this one, then.
...
@@ -1603,11 +1630,25 @@ static __always_inline long __gup_longterm_locked(struct task_struct *tsk,
- and mm being operated on are the current task's and don't allow
- passing of a locked parameter. We also obviously don't pass
- FOLL_REMOTE in here.
- A note on gup_flags: FOLL_PIN should only be set internally by the
- pin_user_page*() and pin_longterm_*() APIs, never directly by the caller.
- That's in order to help avoid mismatches when releasing pages:
- get_user_pages*() pages must be released via put_page(), while
- pin_user_pages*() pages must be released via put_user_page().
Rather than put this here should we put it next to the definition of FOLL_PIN? Because now we have this text 2x... :-/
OK, I'll move it up next to FOLL_PIN, and get rid of the 2x places in gup.c
...
+long pin_longterm_pages_remote(struct task_struct *tsk, struct mm_struct *mm,
unsigned long start, unsigned long nr_pages,unsigned int gup_flags, struct page **pages,struct vm_area_struct **vmas, int *locked)+{
- /* FOLL_GET and FOLL_PIN are mutually exclusive. */
- if (WARN_ON_ONCE(gup_flags & FOLL_GET))
return -EINVAL;- /*
* FIXME: as noted in the get_user_pages_remote() implementation, it* is not yet possible to safely set FOLL_LONGTERM here. FOLL_LONGTERM* needs to be set, but for now the best we can do is a "TODO" item.*/Wait? Why can't we set FOLL_LONGTERM here? pin_* are new calls which are not used yet right?
Nope, not quite! See patch #14 ("vfio, mm: pin_longterm_pages (FOLL_PIN) and put_user_page() conversion"), in which I'm converting an existing get_user_pages_remote() caller.
You set it in the other new pin_* functions?
Yes I did. Because those work already in their gup() counterparts.
thanks,
John Hubbard NVIDIA