Re: [PATCH 3/3] io_uring: refactor io_sq_offload_create()

[Al Viro <viro@zeniv.linux.org.uk>]

On Fri, Jul 23, 2021 at 10:17:27AM -0600, Jens Axboe wrote:
> On 7/22/21 6:10 PM, Al Viro wrote:
> > On Thu, Jul 22, 2021 at 05:42:55PM -0600, Jens Axboe wrote:
> > 
> >>> So how can we possibly get there with tsk->files == NULL and what does it
> >>> have to do with files, anyway?
> >>
> >> It's not the clearest, but the files check is just to distinguish between
> >> exec vs normal cancel. For exec, we pass in files == NULL. It's not
> >> related to task->files being NULL or not, we explicitly pass NULL for
> >> exec.
> > 
> > Er...  So turn that argument into bool cancel_all, and pass false on exit and
> > true on exec? 
> 
> Yes
> 
> > While we are at it, what happens if you pass io_uring descriptor
> > to another process, close yours and then have the recepient close the one it
> > has gotten?  AFAICS, io_ring_ctx_wait_and_kill(ctx) will be called in context
> > of a process that has never done anything io_uring-related.  Can it end up
> > trying to resubmit some requests?> 
> > I rather hope it can't happen, but I don't see what would prevent it...
> 
> No, the pending request would either have gone to a created thread of
> the original task on submission, or it would be sitting in a
> ready-to-retry state. The retry would attempt to queue to original task,
> and either succeed (if still alive) or get failed with -ECANCELED. Any
> given request is tied to the original task.

Hmm...  Sure, you'll be pushing it to the same io_wqe it went through originally,
but you are still in context of io_uring_release() caller, aren't you?

So you call io_wqe_wake_worker(), and it decides that all threads are busy,
but ->nr_workers is still below ->max_workers.  And proceeds to
	create_io_worker(wqe->wq, wqe, acct->index);
which will create a new io-worker thread, but do that in the thread group of
current, i.e. the caller of io_uring_release().  Looks like we'd get
an io-worker thread with the wrong parent...

What am I missing here?