public inbox for [email protected]
 help / color / mirror / Atom feed
From: Pavel Begunkov <[email protected]>
To: Gabriel Krisman Bertazi <[email protected]>
Cc: [email protected], [email protected], [email protected]
Subject: Re: [PATCH RFC 0/9] Launching processes with io_uring
Date: Tue, 31 Dec 2024 14:35:54 +0000	[thread overview]
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>

On 12/30/24 23:38, Gabriel Krisman Bertazi wrote:
> Pavel Begunkov <[email protected]> writes:
> 
> Hi Pavel,
> 
> Sorry for the delay. I took the chance to stay offline for a while on
> the christmas week.

No worries at all

>>> I fully agree this is one of the main problem with the series.  I'm
>>> interested in how we can merge this implementation into the existing
>>> io_uring paths.  My idea, which I hinted in the cover letter, is to have
>>> a flavor of io-wq that executes one linked sequence and then terminates.
>>> When a work is queued there, the newly spawned worker thread will live
>>> only until the end of that link.  This wq is only used to execute the
>>> link following a IORING_OP_CLONE and the user can pass CLONE_ flags to
>>> determine how it is created.  This allows the user to create a detached
>>> file descriptor table in the worker thread, for instance.
>>> It'd allows us to reuse the dispatching infrastructure of io-wq, hide
>>> io_uring internals from the OP_CLONE implementation, and
>>> enable, if I understand correctly, the workarounds to execute
>>> task_works.  We'd need to ensure nothing from the link gets
>>> executed outside of this context.
>>
>> One problem with io-wq is that it's not guaranteed that it's able to
>> serve all types of requests. Though it's limited to multishots atm,
>> which you might not need, but the situation might change. And there
>> is no guarantee that the request is completed by the time it returns
>> from ->issue(), it might even change hands from inside the callback
>> via task_work or by any other mean.
> 
> Multishot is the least of my concerns for this feature, tbh.  I don't
> see how it could be useful in the context of spawning a new thread, so
> in terms of finding sane semantics, we could just reject them at
> submission time if linked from a CLONE.

Right, for multishot that is, but the point here is that io-wq
is an internal detail, and it might get to a point where relying
solely on iowq way of execution would fence you off operations
that you care about. Regardless, it would need to follow common
rules like request refcounting and lifetime

>> It also sounds like you want the cloned task to be a normal
>> io_uring submmiter in terms of infra even though it can't
>> initiate a syscall, which also sounds a bit like an SQPOLL task.
>>
>> And do we really need to execute everything from the new task
>> context, or ops can take a task as an argument and run whenever
>> while final exec could be special cased inside the callback?
> 
> Wouldn't this be similar to the original design of the io-wq/sqpoll, which
> attempted to impersonate the submitter task and resulted in some issues?

The problem there was that it was a kthread, which are not
prepared to run what is basically a random syscall path, and
no amount of whack-a-mole'ing with likes of kthread_use_mm()
was ever enough.

That's the same reason why I'm saying that unless the the new
task is completely initialised and has all resources as far as
the kernel execution goes, there would be no way to run a random
io_uring request from it. "Initialised" would mean here having
->mm and all other task resources to anything valid.

> Executing directly from the new task is much simpler than trying to do the
> operations on the context of another thread.

I agree, if that's about executing e.g. a read request, but if
I'd doubt it if it's about setting a ns to a new not yet running
task.

>>>> requests be run as normal by the original task, each will take the
>>>> half created and not yet launched task as a parameter (in some form),
>>>> modify it, and the final exec would launch it?
>>> A single operation would be a very complex operation doing many things
>>> at once , and much less flexible.  This approach is flexible: you
>>> can combine any (in theory) io_uring operation to obtain the desired
>>> behavior.
>>
>> Ok. And links are not flexible enough for it either. Think of
>> error handling, passing results from one request to another and
>> more complex relations. Unless chains are supposed to be very
>> short and simple, it'd need to be able to return back to user
>> space (the one issuing requests) for error handling.
> 
> We are posting the completions to the submitter ring.  If a request
> fails, we kill the context, but the user is notified of what operation
> failed and need to resubmit the entire link with a new spawn.

That's fine in case of a short simple chain, but there is a
talk of random requests. Let's say a read op fails and you
need to redo it, that's not only expensive but might even be
impossible. E.g. it already consumed data from a pipe / socket.

But the biggest problem is flexibility, any non-trivial relation
between request would need some user processing. Take Josh's
example of linking 2+ exec requests, which instead of relying
on some special status of exec requests can be well implemented
by returning the control back to user, and that would be much
more flexible at that.

> We could avoid links by letting the spawned task linger, and provide a
> way for the user to submit more operations to be executed by this
> specific context.  The new task exists until an op to kill the worker is
> issued or when the execve command executes.  This would allow the user
> to keep multiple partially initialized contexts around for quick
> userspace thread dispatching.  we could provide a mechanism to clone
> these pre-initialized tasks.
> 
> Perhaps it is a stupid idea, but not new - i've seen it discussed
> before: I'm thinking of a workload like a thread scheduler in userspace
> or a network server.  It could keep a partially initialized worker
> thread that never and, when needed, the main task would duplicate it with
> another uring command and make the copy return to userspace, mitigating
> the cost of copying and reinitializing the task.
> 

-- 
Pavel Begunkov


      reply	other threads:[~2024-12-31 14:34 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-09 23:43 [PATCH RFC 0/9] Launching processes with io_uring Gabriel Krisman Bertazi
2024-12-09 23:43 ` [PATCH RFC 1/9] io_uring: Drop __io_req_find_next_prep Gabriel Krisman Bertazi
2024-12-09 23:43 ` [PATCH RFC 2/9] io_uring: Expose failed request helper in internal header Gabriel Krisman Bertazi
2024-12-09 23:43 ` [PATCH RFC 3/9] kernel/fork: Don't inherit PF_USER_WORKER from parent Gabriel Krisman Bertazi
2024-12-09 23:43 ` [PATCH RFC 4/9] fs/exec: Expose do_execveat symbol Gabriel Krisman Bertazi
2024-12-09 23:43 ` [PATCH RFC 5/9] kernel/fork: Add helper to fork from io_uring Gabriel Krisman Bertazi
2024-12-09 23:43 ` [PATCH RFC 6/9] io_uring: Let commands run with current credentials Gabriel Krisman Bertazi
2024-12-11 14:48   ` Pavel Begunkov
2024-12-09 23:43 ` [PATCH RFC 7/9] io_uring: Introduce IORING_OP_CLONE Gabriel Krisman Bertazi
2024-12-11 13:37   ` Pavel Begunkov
2024-12-11 17:26     ` Josh Triplett
2024-12-17 11:03       ` Pavel Begunkov
2024-12-17 19:14         ` Josh Triplett
2024-12-09 23:43 ` [PATCH RFC 8/9] io_uring: Let ->issue know if it was called from spawn thread Gabriel Krisman Bertazi
2024-12-09 23:43 ` [PATCH RFC 9/9] io_uring: Introduce IORING_OP_EXEC command Gabriel Krisman Bertazi
2024-12-10 21:01   ` Josh Triplett
2024-12-10 21:10 ` [PATCH RFC 0/9] Launching processes with io_uring Josh Triplett
2024-12-11 14:02 ` Pavel Begunkov
2024-12-11 17:34   ` Josh Triplett
2024-12-13 20:13   ` Gabriel Krisman Bertazi
2024-12-17 16:10     ` Pavel Begunkov
2024-12-30 23:38       ` Gabriel Krisman Bertazi
2024-12-31 14:35         ` Pavel Begunkov [this message]

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