public inbox for [email protected]
 help / color / mirror / Atom feed
From: Linus Torvalds <[email protected]>
To: Jens Axboe <[email protected]>
Cc: Al Viro <[email protected]>,
	io-uring <[email protected]>,
	"[email protected]" <[email protected]>
Subject: Re: [GIT PULL] io_uring fixes for 5.10-rc
Date: Sat, 21 Nov 2020 10:07:16 -0800	[thread overview]
Message-ID: <CAHk-=wgyRpBW_NOCKpJ1rZGD9jVOX80EWqKwwZxFeief2Khotg@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>

On Fri, Nov 20, 2020 at 7:00 PM Jens Axboe <[email protected]> wrote:
>
> Actually, I think we can do even better. How about just having
> do_filp_open() exit after LOOKUP_RCU fails, if LOOKUP_RCU was already
> set in the lookup flags? Then we don't need to change much else, and
> most of it falls out naturally.

So I was thinking doing the RCU lookup unconditionally, and then doing
the nn-RCU lookup if that fails afterwards.

But your patch looks good to me.

Except for the issue you noticed.

> Except it seems that should work, except LOOKUP_RCU does not guarantee
> that we're not going to do IO:

Well, almost nothing guarantees lack of IO, since allocations etc can
still block, but..

> [   20.463195]  schedule+0x5f/0xd0
> [   20.463444]  io_schedule+0x45/0x70
> [   20.463712]  bit_wait_io+0x11/0x50
> [   20.463981]  __wait_on_bit+0x2c/0x90
> [   20.464264]  out_of_line_wait_on_bit+0x86/0x90
> [   20.464611]  ? var_wake_function+0x30/0x30
> [   20.464932]  __ext4_find_entry+0x2b5/0x410
> [   20.465254]  ? d_alloc_parallel+0x241/0x4e0
> [   20.465581]  ext4_lookup+0x51/0x1b0
> [   20.465855]  ? __d_lookup+0x77/0x120
> [   20.466136]  path_openat+0x4e8/0xe40
> [   20.466417]  do_filp_open+0x79/0x100

Hmm. Is this perhaps an O_CREAT case? I think we only do the dcache
lookups under RCU, not the final path component creation.

And there are probably lots of other situations where we finish with
LOOKUP_RCU (with unlazy_walk()), and then continue.

Example: look at "may_lookup()" - if inode_permission() says "I can't
do this without blocking" the logic actually just tries to validate
the current state (that "unlazy_walk()" thing), and then continue
without RCU.

It obviously hasn't been about lockless semantics, it's been about
really being lockless. So LOOKUP_RCU has been a "try to do this
locklessly" rather than "you cannot take any locks".

I guess we would have to add a LOOKUP_NOBLOCK thing to actually then
say "if the RCU lookup fails, return -EAGAIN".

That's probably not a huge undertaking, but yeah, I didn't think of
it. I think this is a "we need to have Al tell us if it's reasonable".

                Linus

  reply	other threads:[~2020-11-21 18:07 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-20 18:45 [GIT PULL] io_uring fixes for 5.10-rc Jens Axboe
2020-11-20 20:02 ` Linus Torvalds
2020-11-20 21:36   ` Jens Axboe
2020-11-21  0:23     ` Linus Torvalds
2020-11-21  2:41       ` Jens Axboe
2020-11-21  3:00         ` Jens Axboe
2020-11-21 18:07           ` Linus Torvalds [this message]
2020-11-21 22:58             ` Jens Axboe
2020-12-10 17:32               ` namei.c LOOKUP_NONBLOCK (was "Re: [GIT PULL] io_uring fixes for 5.10-rc") Jens Axboe
2020-12-10 18:55                 ` Linus Torvalds
2020-12-10 19:21                   ` Jens Axboe
2020-11-21  0:29 ` [GIT PULL] io_uring fixes for 5.10-rc pr-tracker-bot
  -- strict thread matches above, loose matches on Subject: below --
2020-11-27 20:47 Jens Axboe
2020-11-27 21:21 ` pr-tracker-bot
2020-11-13 21:18 Jens Axboe
2020-11-14  0:15 ` pr-tracker-bot
2020-11-07 20:13 Jens Axboe
2020-11-07 22:08 ` pr-tracker-bot
2020-10-30 17:09 Jens Axboe
2020-10-30 22:10 ` pr-tracker-bot

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 \
    --in-reply-to='CAHk-=wgyRpBW_NOCKpJ1rZGD9jVOX80EWqKwwZxFeief2Khotg@mail.gmail.com' \
    [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