On 11/13/19 2:43 AM, Jan Kara wrote: ...
How does FOLL_PIN result in grabbing (at least normal, for now) page reference? I didn't find that anywhere in this patch but it is a prerequisite to converting any user to pin_user_pages() interface, right?
ohhh, I messed up on this intermediate patch: it doesn't quite stand alone as it should, as you noticed. To correct this, I can do one of the following:
a) move the new pin*() routines into the later patch 16 ("mm/gup: track FOLL_PIN pages"), or
b) do a temporary thing here, such as setting FOLL_GET and adding a TODO, within the pin*() implementations. And this switching it over to FOLL_PIN in patch 16.
I'm thinking (a) is less error-prone, so I'm going with that unless someone points out that that is stupid. :)
...
I was somewhat wondering about the number of functions you add here. So we have:> pin_user_pages() pin_user_pages_fast() pin_user_pages_remote()
and then longterm variants:
pin_longterm_pages() pin_longterm_pages_fast() pin_longterm_pages_remote()
and obviously we have gup like: get_user_pages() get_user_pages_fast() get_user_pages_remote() ... and some other gup variants ...
I think we really should have pin_* vs get_* variants as they are very different in terms of guarantees and after conversion, any use of get_* variant in non-mm code should be closely scrutinized. OTOH pin_longterm_* don't look *that* useful to me and just using pin_* instead with FOLL_LONGTERM flag would look OK to me and somewhat reduce the number of functions which is already large enough? What do people think? I don't feel too strongly about this but wanted to bring this up.
Honza
Sounds just right to me, and I see that Dan and Ira also like it. So I'll proceed with that.
thanks,