Re: io_uring force_nonblock vs POSIX_FADV_WILLNEED

[Matthew Wilcox <willy@infradead.org>]

On Sat, Feb 01, 2020 at 01:43:09AM -0800, Andres Freund wrote:
> As far as I can tell POSIX_FADV_WILLNEED synchronously starts readahead,
> including page allocation etc, which of course might trigger quite
> blocking. The fs also quite possibly needs to read metadata.
> 
> 
> Seems like either WILLNEED would have to always be deferred, or
> force_page_cache_readahead, __do_page_cache_readahead would etc need to
> be wired up to know not to block. Including returning EAGAIN, despite
> force_page_cache_readahead and generic_readahead() intentially ignoring
> return values / errors.

The first step is going to be letting the readahead code know that it
should have this behaviour, which is tricky because the code flow looks
like this:

io_fadvise
  vfs_fadvise
    file->f_op->fadvise()

... and we'd be breaking brand new ground trying to add a gfp_t to a
file_operations method.  Which is not to say it couldn't be done, but
would mean changing filesystems, just so we could pass the gfp
flags through from the top level to the low level.  It wouldn't be
too bad; only two filesystems implement an ->fadvise op today.

Next possibility, we could add a POSIX_FADV_WILLNEED_ASYNC advice flag.
This would be kind of gnarly; look at XFS for example:

        if (advice == POSIX_FADV_WILLNEED) {
                lockflags = XFS_IOLOCK_SHARED;
                xfs_ilock(ip, lockflags);
        }
        ret = generic_fadvise(file, start, end, advice);
        if (lockflags)
                xfs_iunlock(ip, lockflags);

so if there's some other filesystem which decides to start taking a lock
here and we miss it, it'll break when executing async.

Something I already want to see in an entirely different context is
a flag in the task_struct which says, essentially, "don't block in
memory allocations" -- ie behave as if __GFP_NOWAIT | __GFP_NOWARN
is set.  See my proposal here:

https://lore.kernel.org/linux-mm/20200106220910.GK6788@bombadil.infradead.org/
(option 2)
You can see Kirill, Vlastimil and Michal are in favour of adding a
memalloc_nowait_*() API, and it would also save us here from having to
pass this information down the stack to force_page_cache_readahead()
and friends.

I've got my head stuck in the middle of the readahead code right now,
so this seems like a good time to add this functionality.  Once I'm done
with finding out who broke my test VM, I'll take a shot at adding this.