public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Leonardo Bras <leobras@redhat.com>
To: Tejun Heo <tj@kernel.org>
Cc: Leonardo Bras <leobras@redhat.com>,
	Lai Jiangshan <jiangshanlai@gmail.com>,
	Marcelo Tosatti <mtosatti@redhat.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH v1 1/1] wq: Avoid using isolated cpus' timers on unbounded queue_delayed_work
Date: Wed, 24 Jan 2024 22:45:50 -0300	[thread overview]
Message-ID: <ZbG9TjHAMJYIvwsg@LeoBras> (raw)
In-Reply-To: <ZbGFce8ixJKb6umE@slm.duckdns.org>

On Wed, Jan 24, 2024 at 11:47:29AM -1000, Tejun Heo wrote:
> On Wed, Jan 24, 2024 at 05:29:37AM -0300, Leonardo Bras wrote:
> > +	/*
> > +	 * If the work is cpu-unbound, and cpu isolation is in place, only
> > +	 * schedule use timers from housekeeping cpus. In favor of avoiding
> > +	 * cacheline bouncing, run the WQ in the same cpu as the timer.
> > +	 */
> > +	if (cpu == WORK_CPU_UNBOUND && housekeeping_enabled(HK_TYPE_TIMER))
> > +		cpu = housekeeping_any_cpu(HK_TYPE_TIMER);
> 
> Would it make more sense to use wq_unbound_cpumask?

Hello Tejun, thank you for this reply!

That's a good suggestion, but looking at workqueue_init_early() I see that, 
in short:
wq_unbound_cpumask = 	cpu_possible_mask & 
			housekeeping_cpumask(HK_TYPE_WQ) & 
			housekeeping_cpumask(HK_TYPE_DOMAIN) &
			wq_cmdline_cpumask

So wq_unbound_cpumask relates to domain and workqueue cpu isolation.

In our case, we are using this to choose in which cpu is the timer we want 
to use, so it makes sense to use timer-related cpu isolation, instead.

As of today, your suggestion would work the same, as the only way to enable 
WQ cpu isolation is to use nohz_full, which also enables TIMER cpu 
isolation. But since that can change in the future, for any reason, I would 
suggest that we stick to using the HK_TYPE_TIMER cpumask.

I can now notice that this can end up introducing an issue: possibly 
running on a workqueue on a cpu outside of a valid wq_cmdline_cpumask.

I would suggest fixing this in a couple ways:
1 - We introduce a new cpumask which is basically 
    housekeeping_cpumask(HK_TYPE_DOMAIN) & wq_cmdline_cpumask, allowing us 
    to keep the timer interrupt in the same cpu as the scheduled function,
2- We use the resulting cpu only to pick the right timer.

What are your thouhts on that?

Thank you!
Leo


  reply	other threads:[~2024-01-25  1:46 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-24  8:29 [RFC PATCH v1 1/1] wq: Avoid using isolated cpus' timers on unbounded queue_delayed_work Leonardo Bras
2024-01-24 21:47 ` Tejun Heo
2024-01-25  1:45   ` Leonardo Bras [this message]
2024-01-25 18:38     ` Tejun Heo
2024-01-25 21:46       ` Leonardo Bras

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=ZbG9TjHAMJYIvwsg@LeoBras \
    --to=leobras@redhat.com \
    --cc=jiangshanlai@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mtosatti@redhat.com \
    --cc=tj@kernel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox