Re: [PATCH 1/3] io_uring: fix invalid ctx->sq_thread_idle

[Pavel Begunkov <asml.silence@gmail.com>]

On 10/03/2021 13:56, Stefan Metzmacher wrote:
> 
> Hi Pavel,
> 
> I wondered about the exact same change this morning, while researching
> the IORING_SETUP_ATTACH_WQ behavior :-)
> 
> It still seems to me that IORING_SETUP_ATTACH_WQ changed over time.
> As you introduced that flag, can you summaries it's behavior (and changes)
> over time (over the releases).

Not sure I remember the story in details, but from the beginning it was
for io-wq sharing only, then it had expanded to SQPOLL as well. Now it's
only about SQPOLL sharing, because of the recent io-wq changes that made
it per-task and shared by default.

In all cases it should be checking the passed in file, that should retain
the old behaviour of failing setup if the flag is set but wq_fd is not valid.

> 
> I'm wondering if ctx->sq_creds is really the only thing we need to take care of.

io-wq is not affected by IORING_SETUP_ATTACH_WQ. It's per-task and mimics
all the resources of the creator (on the moment of io-wq creation). Off
ATTACH_WQ topic, but that's almost matches what it has been before, and
with dropped unshare bit, should be totally same.

Regarding SQPOLL, it was always using resources of the first task, so
those are just reaped of from it, and not only some particular like
mm/files but all of them, like fork does, so should be safer.

Creds are just a special case because of that personality stuff, at least
if we add back iowq unshare handling.

> 
> Do we know about existing users of IORING_SETUP_ATTACH_WQ and their use case?

Have no clue.

> As mm, files and other things may differ now between sqe producer and the sq_thread.

It was always using mm/files of the ctx creator's task, aka ctx->sqo_task,
but right, for the sharing case those may be different b/w ctx, so looks
like a regression to me

-- 
Pavel Begunkov