Re: [PATCH 1/2] mm: reorganize internal_get_user_pages_fast()

John Hubbard
2020-10-28T23:21:08Z

On 10/27/20 2:55 AM, Christoph Hellwig wrote:
> On Tue, Oct 27, 2020 at 10:33:01AM +0100, Jan Kara wrote:
>> Actually there are callers that care about partial success. See e.g.
>> iov_iter_get_pages() usage in fs/direct_io.c:dio_refill_pages() or
>> bio_iov_iter_get_pages(). These places handle partial success just fine and
>> not allowing partial success from GUP could regress things...

Good point. And those also happen to be the key call sites that I haven't
yet converted to pin_user_pages*(). Seeing as how I'm three versions into
attempting to convert the various *iov_iter*() routines, I should have
remembered that they are all about partial success. :)

>
> But most users do indeed not care. Maybe an explicit FOLL_PARTIAL to
> opt into partial handling could clean up a lot of the mess. Maybe just
> for pin_user_pages for now.
>

That does seem like the perfect mix. IIRC, all of the pin_user_pages()
call sites today do not accept partial success (and it's easy enough to
audit and confirm). So likely no need to add FOLL_PARTIAL there, and no
huge danger of regressions. It would definitely reduce the line count at
multiple call sites, in return for adding some lines to gup.c.

And maybe it can go further, at some point, but that's a good way to start.

I'm leaning toward just sending out a small series to do that, unless there
are objections and/or better ways to improve this area...

thanks,
--
John Hubbard
NVIDIA