linux-block.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Pavel Begunkov <asml.silence@gmail.com>
Cc: Jens Axboe <axboe@kernel.dk>, Ingo Molnar <mingo@redhat.com>,
	linux-block@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/2] sched/wait: Add wait_threshold
Date: Mon, 23 Sep 2019 21:27:42 +0200	[thread overview]
Message-ID: <20190923192742.GH2369@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <3e359937-5b19-8a4c-4243-ba2edff68504@gmail.com>


A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?
A: Top-posting.
Q: What is the most annoying thing in e-mail?

On Mon, Sep 23, 2019 at 07:37:46PM +0300, Pavel Begunkov wrote:
> Just in case duplicating a mail from the cover-letter thread:

Just because every patch should have a self contained and coherent
Changelog.

> It could be done with @cond indeed, that's how it works for now.
> However, this addresses performance issues only.
> 
> The problem with wait_event_*() is that, if we have a counter and are
> trying to wake up tasks after each increment, it would schedule each
> waiting task O(threshold) times just for it to spuriously check @cond
> and go back to sleep. All that overhead (memory barriers, registers
> save/load, accounting, etc) turned out to be enough for some workloads
> to slow down the system.
> 
> With this specialisation it still traverses a wait list and makes
> indirect calls to the checker callback, but the list supposedly is
> fairly  small, so performance there shouldn't be a problem, at least for
> now.
> 
> Regarding semantics; It should wake a task when a value passed to
> wake_up_threshold() is greater or equal then a task's threshold, that is
> specified individually for each task in wait_threshold_*().
> 
> In pseudo code:
> ```
> def wake_up_threshold(n, wait_queue):
> 	for waiter in wait_queue:
> 		waiter.wake_up_if(n >= waiter.threshold);
> ```
> 
> Any thoughts how to do it better? Ideas are very welcome.
> 
> BTW, this monster is mostly a copy-paste from wait_event_*(),
> wait_bit_*(). We could try to extract some common parts from these
> three, but that's another topic.

I don't think that is another topic at all. It is a quality of
implementation issue. We already have too many copies of all that (3).

So basically you want to fudge the wake function to do the/a @cond test,
not unlike what wait_bit already does, but differenly.

I'm really rather annoyed with C for not having proper lambda functions;
that would make all this so much easier. Anyway, let me have a poke at
this in the morning, it's late already.

Also, is anything actually using wait_queue_entry::private ? I'm
not finding any in a hurry.



  reply	other threads:[~2019-09-23 19:27 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-22  8:08 [PATCH v2 0/2] Optimise io_uring completion waiting Pavel Begunkov (Silence)
2019-09-22  8:08 ` [PATCH v2 1/2] sched/wait: Add wait_threshold Pavel Begunkov (Silence)
2019-09-23  7:19   ` Peter Zijlstra
2019-09-23 16:37     ` Pavel Begunkov
2019-09-23 19:27       ` Peter Zijlstra [this message]
2019-09-23 20:23         ` Peter Zijlstra
2019-09-24  6:44         ` Pavel Begunkov
2019-09-22  8:08 ` [PATCH v2 2/2] io_uring: Optimise cq waiting with wait_threshold Pavel Begunkov (Silence)
2019-09-22 15:51 ` [PATCH v2 0/2] Optimise io_uring completion waiting Jens Axboe
2019-09-23  8:35   ` Ingo Molnar
2019-09-23 16:21     ` Pavel Begunkov
2019-09-23 16:32       ` Pavel Begunkov
2019-09-23 20:48         ` Jens Axboe
2019-09-23 23:00           ` Jens Axboe
2019-09-24  7:06             ` Pavel Begunkov
2019-09-24  8:02               ` Jens Axboe
2019-09-24  8:27                 ` Jens Axboe
2019-09-24  8:36                   ` Jens Axboe
2019-09-24  9:33                     ` Pavel Begunkov
2019-09-24 10:11                       ` Jens Axboe
2019-09-24  9:49                     ` Peter Zijlstra
2019-09-24 10:13                       ` Jens Axboe
2019-09-24 10:34                         ` Jens Axboe
2019-09-24 11:11                           ` Pavel Begunkov
2019-09-24 11:15                             ` Jens Axboe
2019-09-24 11:23                               ` Pavel Begunkov
2019-09-24 13:13                                 ` Jens Axboe
2019-09-24 17:33                                   ` Pavel Begunkov
2019-09-24 17:46                                     ` Jens Axboe
2019-09-24 18:28                                       ` Pavel Begunkov
2019-09-24 19:32                                         ` Jens Axboe
2019-09-24 11:43                             ` Peter Zijlstra
2019-09-24 12:57                               ` Jens Axboe
2019-09-24 11:33                           ` Peter Zijlstra
2019-09-24  9:20                   ` Pavel Begunkov
2019-09-24 10:09                     ` Jens Axboe
2019-09-24  9:21                 ` Pavel Begunkov
2019-09-24 10:09                   ` Jens Axboe

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 \
    --in-reply-to=20190923192742.GH2369@hirez.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=asml.silence@gmail.com \
    --cc=axboe@kernel.dk \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    /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;
as well as URLs for NNTP newsgroup(s).