On 1/28/19 6:32 PM, Jann Horn wrote:
> On Tue, Jan 29, 2019 at 2:31 AM Jens Axboe <axboe@xxxxxxxxx> wrote:
>> On 1/28/19 6:29 PM, Jann Horn wrote:
>>> On Mon, Jan 28, 2019 at 10:35 PM Jens Axboe <axboe@xxxxxxxxx> wrote:
>>>> The submission queue (SQ) and completion queue (CQ) rings are shared
>>>> between the application and the kernel. This eliminates the need to
>>>> copy data back and forth to submit and complete IO.
>>> [...]
>>>> +static struct io_kiocb *io_get_req(struct io_ring_ctx *ctx)
>>>> +{
>>>> + struct io_kiocb *req;
>>>> +
>>>> + /* safe to use the non tryget, as we're inside ring ref already */
>>>> + percpu_ref_get(&ctx->refs);
>>>
>>> Is that true? In the path io_sq_thread() -> io_submit_sqes() ->
>>> io_submit_sqe() -> io_get_req(), I don't see anything that's already
>>> holding a reference for you. Is the worker thread holding a reference
>>> somewhere that I'm missing?
>>
>> If the thread is alive, then the ctx is alive. Before we drop the last
>> ref to the ctx (and kill it), we wait for the thread to exit.
>
> Where in __io_uring_register() are you waiting for the thread to exit
> before killing it? As far as I can tell, you come straight in from
> syscall context, take a mutex, and do percpu_ref_kill().
I was just referring to the regular shutdown path. You are right, for
the later io_uring_register() more care needs to be taken. I'll just
switch to the tryget, it's not like the non-try is a big cycle saver
by any stretch.
--
Jens Axboe
