From: Jens Axboe <axboe@kernel.dk>
To: peterz@infradead.org, Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: linux-kernel@vger.kernel.org, io-uring@vger.kernel.org,
Ingo Molnar <mingo@redhat.com>,
Juri Lelli <juri.lelli@redhat.com>,
Vincent Guittot <vincent.guittot@linaro.org>,
Dietmar Eggemann <dietmar.eggemann@arm.com>,
Steven Rostedt <rostedt@goodmis.org>,
Ben Segall <bsegall@google.com>, Mel Gorman <mgorman@suse.de>,
Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [RFC PATCH] sched: Invoke io_wq_worker_sleeping() with enabled preemption
Date: Wed, 19 Aug 2020 07:18:12 -0600 [thread overview]
Message-ID: <f26205ac-9da9-253e-ea43-db2417714a94@kernel.dk> (raw)
In-Reply-To: <20200819131507.GC2674@hirez.programming.kicks-ass.net>
On 8/19/20 6:15 AM, peterz@infradead.org wrote:
> On Wed, Aug 19, 2020 at 02:37:58PM +0200, Sebastian Andrzej Siewior wrote:
>
>> I don't see a significant reason why this lock should become a
>> raw_spinlock_t therefore I suggest to move it after the
>> tsk_is_pi_blocked() check.
>
>> Any feedback on this vs raw_spinlock_t?
>>
>> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
>> ---
>> fs/io-wq.c | 8 ++++----
>> kernel/sched/core.c | 10 +++++-----
>> 2 files changed, 9 insertions(+), 9 deletions(-)
>>
>
>> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
>> index 3bbb60b97c73c..b76c0f27bd95e 100644
>> --- a/kernel/sched/core.c
>> +++ b/kernel/sched/core.c
>> @@ -4694,18 +4694,18 @@ static inline void sched_submit_work(struct task_struct *tsk)
>> * in the possible wakeup of a kworker and because wq_worker_sleeping()
>> * requires it.
>> */
>> - if (tsk->flags & (PF_WQ_WORKER | PF_IO_WORKER)) {
>> + if (tsk->flags & PF_WQ_WORKER) {
>> preempt_disable();
>> - if (tsk->flags & PF_WQ_WORKER)
>> - wq_worker_sleeping(tsk);
>> - else
>> - io_wq_worker_sleeping(tsk);
>> + wq_worker_sleeping(tsk);
>> preempt_enable_no_resched();
>> }
>>
>> if (tsk_is_pi_blocked(tsk))
>> return;
>>
>> + if (tsk->flags & PF_IO_WORKER)
>> + io_wq_worker_sleeping(tsk);
>> +
>
> Urgh, so this adds a branch in what is normally considered a fairly hot
> path.
>
>
> I'm thinking that the raw_spinlock_t option would permit leaving that
> single:
>
> if (tsk->flags & (PF_WQ_WORKER | PF_IO_WORKER))
>
> branch intact?
Yes, the raw spinlock would do it, and leave the single branch intact
in the hot path. I'd be fine with going that route for io-wq.
--
Jens Axboe
next prev parent reply other threads:[~2020-08-19 13:23 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-19 12:37 [RFC PATCH] sched: Invoke io_wq_worker_sleeping() with enabled preemption Sebastian Andrzej Siewior
2020-08-19 13:15 ` peterz
2020-08-19 13:18 ` Jens Axboe [this message]
2020-08-19 19:44 ` [PATCH] io_wq: Make io_wqe::lock a raw_spinlock_t Sebastian Andrzej Siewior
2020-09-01 8:41 ` [PATCH v2] " Sebastian Andrzej Siewior
2020-09-01 14:17 ` Jens Axboe
2020-08-19 13:33 ` [RFC PATCH] sched: Invoke io_wq_worker_sleeping() with enabled preemption Sebastian Andrzej Siewior
2020-08-19 14:21 ` peterz
2020-08-19 19:55 ` [PATCH 1/2] sched: Bring the PF_IO_WORKER and PF_WQ_WORKER bits closer together Sebastian Andrzej Siewior
2020-08-19 20:00 ` [PATCH 2/2] sched: Cache task_struct::flags in sched_submit_work() Sebastian Andrzej Siewior
2020-08-19 20:11 ` Peter Zijlstra
2020-08-27 7:54 ` [tip: sched/core] " tip-bot2 for Sebastian Andrzej Siewior
2020-08-27 7:54 ` [tip: sched/core] sched: Bring the PF_IO_WORKER and PF_WQ_WORKER bits closer together tip-bot2 for Sebastian Andrzej Siewior
2020-09-07 12:58 ` [PATCH 1/2] " Will Deacon
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=f26205ac-9da9-253e-ea43-db2417714a94@kernel.dk \
--to=axboe@kernel.dk \
--cc=bigeasy@linutronix.de \
--cc=bsegall@google.com \
--cc=dietmar.eggemann@arm.com \
--cc=io-uring@vger.kernel.org \
--cc=juri.lelli@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mgorman@suse.de \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=rostedt@goodmis.org \
--cc=tglx@linutronix.de \
--cc=vincent.guittot@linaro.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.