On 8/19/20 6:15 AM, [email protected] 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 <[email protected]> >> --- >> 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
