public inbox for [email protected]
 help / color / mirror / Atom feed
From: Jens Axboe <[email protected]>
To: Glauber Costa <[email protected]>,
	[email protected], Avi Kivity <[email protected]>
Subject: Re: how is register_(buffer|file) supposed to work?
Date: Wed, 12 Feb 2020 07:20:03 -0700	[thread overview]
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAD-J=zbYp0D5NSV1hqxpU7C-Ki+ffwretuXEYCxX5cbazA5WqQ@mail.gmail.com>

On 2/11/20 9:12 PM, Glauber Costa wrote:
> Hi,
> 
> I am trying to experiment with the interface for registering files and buffers.
> 
> (almost) Every time I call io_uring_register with those opcodes, my
> application hangs.
> 
> It's easy to see the reason. I am blocking here:
> 
>                 mutex_unlock(&ctx->uring_lock);
>                 ret = wait_for_completion_interruptible(&ctx->completions[0]);
>                 mutex_lock(&ctx->uring_lock);
> 
> Am I right in my understanding that this is waiting for everything
> that was submitted to complete? Some things in my ring may never
> complete: for instance one may be polling for file descriptors that
> may never really become ready.
> 
> This sounds a bit too restrictive to me. Is this really the intended
> use of the interface?

For files, this was added in the current merge window:

commit 05f3fb3c5397524feae2e73ee8e150a9090a7da2
Author: Jens Axboe <[email protected]>
Date:   Mon Dec 9 11:22:50 2019 -0700

    io_uring: avoid ring quiesce for fixed file set unregister and update

which allows you to call IORING_REGISTER_FILES_UPDATE without having to
quiesce the ring. File sets can be sparse, you can register with an fd
of -1 and then later use FILES_UPDATE (or IORING_OP_FILES_UPDATE) to
replace it with a real entry. You can also replace a real entry with a
new one, or switch it to sparse again.

This helps the file case, but the buffer case is the same as originally,
it is intended to be used at ring setup time, not really during runtime.
As you have found, it needs to quiesce the ring, hence if you have any
requests pending, it'll wait for those. If they are unbounded requests
(like poll, for instance), then it won't really work that well.

That said, there's absolutely no reason why buffer registration can't
mimick what was done for file updates - allow registering a larger
sparse set, and update buffers as you need to. The latter without
needing a quiesce. The above commit basically just needs to be
implemented for buffers as well, which would be pretty trivial.

-- 
Jens Axboe


  reply	other threads:[~2020-02-12 14:20 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-12  4:12 how is register_(buffer|file) supposed to work? Glauber Costa
2020-02-12 14:20 ` Jens Axboe [this message]
2020-02-12 14:23   ` Glauber Costa

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] \
    /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