On Wed, Jan 23, 2019 at 08:35:22AM -0700, Jens Axboe wrote:
> Proof of concept.
Is that still true?
> 1) Maybe have smarter backoff. Busy loop for X time, then go to
> monitor/mwait, finally the schedule we have now after an idle
> second. Might not be worth the complexity.
>
> 2) Probably want the application to pass in the appropriate grace
> period, not hard code it at 1 second.
2) actually sounds really useful. Should we look into it ASAP?
>
> struct {
> /* CQ ring */
> @@ -264,6 +267,9 @@ static void __io_cqring_add_event(struct io_ring_ctx
> *ctx, u64 ki_user_data,
>
> if (waitqueue_active(&ctx->wait))
> wake_up(&ctx->wait);
> + if ((ctx->flags & IORING_SETUP_SQPOLL) &&
> + waitqueue_active(&ctx->sqo_wait))
waitqueue_active is really cheap and sqo_wait should not otherwise
by active. Do we really need the flags check here?
> + /*
> + * Normal IO, just pretend everything completed.
> + * We don't have to poll completions for that.
> + */
> + if (ctx->flags & IORING_SETUP_IOPOLL) {
> + /*
> + * App should not use IORING_ENTER_GETEVENTS
> + * with thread polling, but if it does, then
> + * ensure we are mutually exclusive.
Should we just return an error early on in this case instead?
> if (to_submit) {
> + if (ctx->flags & IORING_SETUP_SQPOLL) {
> + wake_up(&ctx->sqo_wait);
> + ret = to_submit;
Do these semantics really make sense? Maybe we should have an
IORING_ENTER_WAKE_SQ instead of overloading the to_submit argument?
Especially as we don't really care about returning the number passed in.
> + if (ctx->sqo_thread) {
> + kthread_park(ctx->sqo_thread);
Can you explain why we need the whole kthread_park game? It is only
intended to deal with pausing a thread, and if need it to shut down
a thread we have a bug somewhere.
> static void io_sq_offload_stop(struct io_ring_ctx *ctx)
> {
> + if (ctx->sqo_thread) {
> + kthread_park(ctx->sqo_thread);
> + kthread_stop(ctx->sqo_thread);
> + ctx->sqo_thread = NULL;
Also there isn't really much of a point in setting pointers to NULL
just before freeing the containing structure. In the best case this
now papers over bugs that poisoning or kasan would otherwise find.