how is register_(buffer|file) supposed to work?

how is register_(buffer|file) supposed to work?

[Glauber Costa]

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?

Re: how is register_(buffer|file) supposed to work?

[Jens Axboe]

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 <axboe@kernel.dk>
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

Re: how is register_(buffer|file) supposed to work?

[Glauber Costa]

On Wed, Feb 12, 2020 at 9:20 AM Jens Axboe <axboe@kernel.dk> wrote:
>
> 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:
>

Ok, so this is what I was missing:

> commit 05f3fb3c5397524feae2e73ee8e150a9090a7da2
> Author: Jens Axboe <axboe@kernel.dk>
> 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

I thought I've seen EBADF errors when registering -1, but maybe it
wasn't -1, it was
just really an invalid fd.

For memory I guess I could register early on and draw from a poll.
That's a bit inconvenient
as ideally that poll would grow and shrink dynamically, but it works

I'll give it a try