From: Peter Zijlstra <peterz@infradead.org>
To: Boqun Feng <boqun.feng@gmail.com>
Cc: linux-kernel@vger.kernel.org, rcu@vger.kernel.org,
lkmm@lists.linux.dev, Ingo Molnar <mingo@kernel.org>,
Will Deacon <will@kernel.org>, Waiman Long <longman@redhat.com>,
Davidlohr Bueso <dave@stgolabs.net>,
"Paul E. McKenney" <paulmck@kernel.org>,
Josh Triplett <josh@joshtriplett.org>,
Frederic Weisbecker <frederic@kernel.org>,
Neeraj Upadhyay <neeraj.upadhyay@kernel.org>,
Joel Fernandes <joelagnelf@nvidia.com>,
Uladzislau Rezki <urezki@gmail.com>,
Steven Rostedt <rostedt@goodmis.org>,
Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
Lai Jiangshan <jiangshanlai@gmail.com>,
Zqiang <qiang.zhang@linux.dev>, Breno Leitao <leitao@debian.org>,
aeh@meta.com, netdev@vger.kernel.org, edumazet@google.com,
jhs@mojatatu.com, kernel-team@meta.com,
Erik Lundgren <elundgren@meta.com>
Subject: Re: [PATCH 4/8] shazptr: Avoid synchronize_shaptr() busy waiting
Date: Wed, 25 Jun 2025 13:56:43 +0200 [thread overview]
Message-ID: <20250625115643.GE1613376@noisy.programming.kicks-ass.net> (raw)
In-Reply-To: <20250625031101.12555-5-boqun.feng@gmail.com>
Response is a bit weird because non-linear editing..
On Tue, Jun 24, 2025 at 08:10:57PM -0700, Boqun Feng wrote:
> + /* Whether the scan kthread has been scheduled to scan */
> + bool scheduled;
> +static int __noreturn shazptr_scan_kthread(void *unused)
> +{
> + for (;;) {
> + swait_event_idle_exclusive(shazptr_scan.wq,
> + READ_ONCE(shazptr_scan.scheduled));
This seems weird; why use a whole wait-queue, in exclusive mode no less,
for something that is one known thread.
Also, I think this thing needs to be FREEZABLE, otherwise suspend might
have issues.
Why not just write it like:
for (;;) {
set_current_state(TASK_IDLE | TASK_FREEZABLE);
if (!list_empty(&scan->queue))
break;
schedule();
}
__set_current_state(TASK_RUNNABLE);
for (;;) {
scoped_guard (mutex, scan->lock) {
if (list_empty(scan->queued) &&
list_empty(scan->scanning))
break;
}
shazptr_do_scan(scan);
}
> + shazptr_do_scan(&shazptr_scan);
> +
> + scoped_guard(mutex, &shazptr_scan.lock) {
> + if (list_empty(&shazptr_scan.queued) &&
> + list_empty(&shazptr_scan.scanning))
> + shazptr_scan.scheduled = false;
This condition, why can't we directly use this condition instead of
scheduled?
> + }
> + }
> +}
> +static void synchronize_shazptr_normal(void *ptr)
> +{
> +
> + /* Found blocking slots, prepare to wait. */
> + if (blocking_grp_mask) {
> + struct shazptr_scan *scan = &shazptr_scan;
> + struct shazptr_wait wait = {
> + .blocking_grp_mask = blocking_grp_mask,
> + };
> +
> + INIT_LIST_HEAD(&wait.list);
> + init_completion(&wait.done);
> +
> + scoped_guard(mutex, &scan->lock) {
> + list_add_tail(&wait.list, &scan->queued);
> +
> + if (!scan->scheduled) {
> + WRITE_ONCE(scan->scheduled, true);
> + swake_up_one(&shazptr_scan.wq);
> + }
Or perhaps; just write this like:
bool was_empty = list_empty(&scan->queued);
list_add_tail(&wait.list, &scan->queued);
if (was_empty)
wake_up_process(scan->thread);
> + }
> +
> + wait_for_completion(&wait.done);
> + }
> +}
next prev parent reply other threads:[~2025-06-25 11:56 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-06-25 3:10 [PATCH 0/8] Introduce simple hazard pointers for lockdep Boqun Feng
2025-06-25 3:10 ` [PATCH 1/8] Introduce simple hazard pointers Boqun Feng
2025-06-25 10:00 ` Peter Zijlstra
2025-06-25 14:25 ` Mathieu Desnoyers
2025-06-25 15:05 ` Boqun Feng
2025-06-25 15:52 ` Waiman Long
2025-06-25 16:09 ` Boqun Feng
2025-06-25 17:47 ` Waiman Long
2025-06-25 3:10 ` [PATCH 2/8] shazptr: Add refscale test Boqun Feng
2025-06-25 10:02 ` Peter Zijlstra
2025-06-25 3:10 ` [PATCH 3/8] shazptr: Add refscale test for wildcard Boqun Feng
2025-06-25 10:03 ` Peter Zijlstra
2025-06-25 3:10 ` [PATCH 4/8] shazptr: Avoid synchronize_shaptr() busy waiting Boqun Feng
2025-06-25 11:40 ` Peter Zijlstra
2025-06-25 11:56 ` Peter Zijlstra [this message]
2025-06-25 13:56 ` Frederic Weisbecker
2025-06-25 15:24 ` Boqun Feng
2025-06-26 13:45 ` Frederic Weisbecker
2025-06-25 3:10 ` [PATCH 5/8] shazptr: Allow skip self scan in synchronize_shaptr() Boqun Feng
2025-06-25 3:10 ` [PATCH 6/8] rcuscale: Allow rcu_scale_ops::get_gp_seq to be NULL Boqun Feng
2025-06-25 3:11 ` [PATCH 7/8] rcuscale: Add tests for simple hazard pointers Boqun Feng
2025-06-25 3:11 ` [PATCH 8/8] locking/lockdep: Use shazptr to protect the key hashlist Boqun Feng
2025-06-25 11:59 ` Peter Zijlstra
2025-06-25 14:18 ` Boqun Feng
2025-07-10 14:06 ` Breno Leitao
2025-07-11 2:31 ` Boqun Feng
2025-06-25 12:05 ` [PATCH 0/8] Introduce simple hazard pointers for lockdep Christoph Hellwig
2025-06-25 14:08 ` Boqun Feng
2025-06-26 10:16 ` Christoph Hellwig
2025-06-26 13:45 ` Mathieu Desnoyers
2025-06-26 15:47 ` Boqun Feng
2025-06-27 2:56 ` Paul E. McKenney
2025-06-25 12:25 ` Mathieu Desnoyers
2025-06-25 13:21 ` Boqun Feng
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=20250625115643.GE1613376@noisy.programming.kicks-ass.net \
--to=peterz@infradead.org \
--cc=aeh@meta.com \
--cc=boqun.feng@gmail.com \
--cc=dave@stgolabs.net \
--cc=edumazet@google.com \
--cc=elundgren@meta.com \
--cc=frederic@kernel.org \
--cc=jhs@mojatatu.com \
--cc=jiangshanlai@gmail.com \
--cc=joelagnelf@nvidia.com \
--cc=josh@joshtriplett.org \
--cc=kernel-team@meta.com \
--cc=leitao@debian.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lkmm@lists.linux.dev \
--cc=longman@redhat.com \
--cc=mathieu.desnoyers@efficios.com \
--cc=mingo@kernel.org \
--cc=neeraj.upadhyay@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=paulmck@kernel.org \
--cc=qiang.zhang@linux.dev \
--cc=rcu@vger.kernel.org \
--cc=rostedt@goodmis.org \
--cc=urezki@gmail.com \
--cc=will@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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).