Re: [PATCH v7 18/19] iov_iter: Introduce nofault flag to disable page faults

On Fri, Aug 27, 2021 at 8:47 PM Al Viro [off-list ref] wrote:

On Fri, Aug 27, 2021 at 06:49:25PM +0200, Andreas Gruenbacher wrote:

quoted

Introduce a new nofault flag to indicate to get_user_pages to use the
FOLL_NOFAULT flag.  This will cause get_user_pages to fail when it
would otherwise fault in a page.

Currently, the noio flag is only checked in iov_iter_get_pages and
iov_iter_get_pages_alloc.  This is enough for iomaop_dio_rw, but it
may make sense to check in other contexts as well.

I can live with that, but
        * direct assignments (as in the next patch) are fucking hard to
grep for.  Is it intended to be "we set it for duration of primitive",
or...?

It's for this kind of pattern:

       pagefault_disable();
       to->nofault = true;
       ret = iomap_dio_rw(iocb, to, &gfs2_iomap_ops, NULL,
                          IOMAP_DIO_PARTIAL, written);
       to->nofault = false;
       pagefault_enable();

Clearing the flag at the end isn't strictly necessary, but it kind of
documents that the flag pertains to iomap_dio_rw and not something
else.

        * it would be nice to have a description of intended semantics
for that thing.  This "may make sense to check in other contexts" really
needs to be elaborated (and agreed) upon.  Details, please.

Maybe the description should just be something like:

"Introduce a new nofault flag to indicate to iov_iter_get_pages not to
fault in user pages.

This is implemented by passing the FOLL_NOFAULT flag to get_user_pages,
which causes get_user_pages to fail when it would otherwise fault in a
page. We'll use the ->nofault flag to prevent iomap_dio_rw from faulting
in pages when page faults are not allowed."

Thanks,
Andreas