public inbox for [email protected]
 help / color / mirror / Atom feed
From: Sebastian Andrzej Siewior <[email protected]>
To: [email protected]
Cc: [email protected], [email protected],
	Ingo Molnar <[email protected]>,
	Juri Lelli <[email protected]>,
	Vincent Guittot <[email protected]>,
	Dietmar Eggemann <[email protected]>,
	Steven Rostedt <[email protected]>,
	Ben Segall <[email protected]>, Mel Gorman <[email protected]>,
	Jens Axboe <[email protected]>,
	Thomas Gleixner <[email protected]>
Subject: Re: [RFC PATCH] sched: Invoke io_wq_worker_sleeping() with enabled preemption
Date: Wed, 19 Aug 2020 15:33:20 +0200	[thread overview]
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>

On 2020-08-19 15:15:07 [+0200], [email protected] wrote:

> > -	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?

The compiler generates code to test for both flags at once. If none of
both possible flags are set then there is one branch (get out and bring
me to tst_is_pi…).
And yes, with raw_spinlock_t we could keep that one branch.

If you want to optimize further, we could move PF_IO_WORKER to an lower
bit. x86 can test for both via
(gcc-10)
|         testl   $536870944, 44(%rbp)    #, _11->flags
|         jne     .L1635  #,

(clang-9)
|         testl   $536870944, 44(%rbx)    # imm = 0x20000020
|         je      .LBB112_6


but ARM can't and does
|          ldr     r1, [r5, #16]   @ tsk_3->flags, tsk_3->flags
|         mov     r2, #32 @ tmp157,
|         movt    r2, 8192        @ tmp157,
|         tst     r2, r1  @ tmp157, tsk_3->flags
|         beq     .L998           @,

same ARM64
|         ldr     w0, [x20, 60]   //, _11->flags
|         and     w0, w0, 1073741792      // tmp117, _11->flags,
|         and     w0, w0, -536870849      // tmp117, tmp117,
|         cbnz    w0, .L453       // tmp117,

using 0x10 for PF_IO_WORKER instead will turn this into:
|         ldr     w0, [x20, 60]   //, _11->flags
|         tst     w0, 48  // _11->flags,
|         bne     .L453           //,

ARM:
|         ldr     r2, [r5, #16]   @ tsk_3->flags, tsk_3->flags
|         tst     r2, #48 @ tsk_3->flags,
|         beq     .L998           @,

Sebastian

  parent reply	other threads:[~2020-08-19 13:33 UTC|newest]

Thread overview: 12+ 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
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   ` Sebastian Andrzej Siewior [this message]
2020-08-19 14:21     ` [RFC PATCH] sched: Invoke io_wq_worker_sleeping() with enabled preemption 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-09-07 12:58         ` [PATCH 1/2] sched: Bring the PF_IO_WORKER and PF_WQ_WORKER bits closer together 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 \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [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