public inbox for [email protected]
 help / color / mirror / Atom feed
From: Matthew Wilcox <[email protected]>
To: Andres Freund <[email protected]>
Cc: Jens Axboe <[email protected]>,
	[email protected], [email protected]
Subject: Re: io_uring force_nonblock vs POSIX_FADV_WILLNEED
Date: Sun, 2 Feb 2020 22:40:47 -0800	[thread overview]
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>

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/[email protected]/
(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.

  parent reply	other threads:[~2020-02-03  6:40 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-01  9:43 io_uring force_nonblock vs POSIX_FADV_WILLNEED Andres Freund
2020-02-01 16:22 ` Jens Axboe
2020-02-02  7:14   ` Andres Freund
2020-02-02 16:34     ` Jens Axboe
2020-02-03  6:40 ` Matthew Wilcox [this message]
2020-02-03  7:42   ` Andres Freund

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox