From: Uladzislau Rezki <urezki@gmail.com>
To: "Paul E. McKenney" <paulmck@kernel.org>,
Joel Fernandes <joel@joelfernandes.org>
Cc: "Paul E. McKenney" <paulmck@kernel.org>,
Joel Fernandes <joel@joelfernandes.org>,
rcu@vger.kernel.org, linux-kernel@vger.kernel.org,
kernel-team@fb.com, rostedt@goodmis.org
Subject: Re: [PATCH rcu 13/14] workqueue: Make queue_rcu_work() use call_rcu_flush()
Date: Mon, 24 Oct 2022 14:23:39 +0200 [thread overview]
Message-ID: <Y1aDy3maaO39ClSU@pc636> (raw)
In-Reply-To: <Y1ZtyjxKCcV0Hfjn@pc636>
> > On Sun, Oct 23, 2022 at 08:36:00PM -0400, Joel Fernandes wrote:
> > > Hello,
> > >
> > > On Wed, Oct 19, 2022 at 6:51 PM Paul E. McKenney <paulmck@kernel.org> wrote:
> > > >
> > > > From: Uladzislau Rezki <urezki@gmail.com>
> > > >
> > > > call_rcu() changes to save power will slow down RCU workqueue items
> > > > queued via queue_rcu_work(). This may not be an issue, however we cannot
> > > > assume that workqueue users are OK with long delays. Use
> > > > call_rcu_flush() API instead which reverts to the old behavio
> > >
> > > On ChromeOS, I can see that queue_rcu_work() is pretty noisy and the
> > > batching is much better if we can just keep it as call_rcu() instead
> > > of call_rcu_flush().
> > >
> > > Is there really any reason to keep it as call_rcu_flush() ? If I
> > > recall, the real reason Vlad's system was slowing down was because of
> > > scsi and the queue_rcu_work() conversion was really a red herring.
> >
> <snip>
> *** drivers/acpi/osl.c:
> acpi_os_drop_map_ref[401] queue_rcu_work(system_wq, &map->track.rwork);
>
> *** drivers/gpu/drm/i915/gt/intel_execlists_submission.c:
> virtual_context_destroy[3653] queue_rcu_work(system_wq, &ve->rcu);
>
> *** fs/aio.c:
> free_ioctx_reqs[632] queue_rcu_work(system_wq, &ctx->free_rwork);
>
> *** fs/fs-writeback.c:
> inode_switch_wbs[604] queue_rcu_work(isw_wq, &isw->work);
> cleanup_offline_cgwb[676] queue_rcu_work(isw_wq, &isw->work);
>
> *** include/linux/workqueue.h:
> __printf[446] extern bool queue_rcu_work(struct workqueue_struct *wq, struct rcu_work *rwork);
>
> *** kernel/cgroup/cgroup.c:
> css_release_work_fn[5253] queue_rcu_work(cgroup_destroy_wq, &css->destroy_rwork);
> css_create[5384] queue_rcu_work(cgroup_destroy_wq, &css->destroy_rwork);
>
> *** kernel/rcu/tree.c:
> kfree_rcu_monitor[3192] queue_rcu_work(system_wq, &krwp->rcu_work);
>
> *** net/core/skmsg.c:
> sk_psock_drop[852] queue_rcu_work(system_wq, &psock->rwork);
>
> *** net/sched/act_ct.c:
> tcf_ct_flow_table_put[355] queue_rcu_work(act_ct_wq, &ct_ft->rwork);
>
> *** net/sched/cls_api.c:
> tcf_queue_work[225] return queue_rcu_work(tc_filter_wq, rwork);
> <snip>
> There are 9 users of the queue_rcu_work() functions. I think there can be
> a side effect if we keep it as lazy variant. Please note that i have not
> checked all those users.
>
> > There are less than 20 invocations of queue_rcu_work(), so it should
> > be possible look through each. The low-risk approach is of course to
> > have queue_rcu_work() use call_rcu_flush().
> >
> > The next approach might be to have a Kconfig option and/or kernel
> > boot parameter that allowed a per-system choice.
> >
> > But it would not hurt to double-check on Android.
> >
> I did not see such noise but i will come back some data on 5.10 kernel
> today.
>
Home screen swipe:
<snip>
<...>-15 [003] d..1 202.142205: rcu_batch_start: rcu_preempt CBs=105 bl=10
<...>-55 [001] d..1 202.166174: rcu_batch_start: rcu_preempt CBs=135 bl=10
<...>-26 [001] d..1 202.402182: rcu_batch_start: rcu_preempt CBs=221 bl=10
rcuop/3-40 [003] d..1 202.650323: rcu_batch_start: rcu_preempt CBs=213 bl=10
rcuop/3-40 [000] d..1 203.210537: rcu_batch_start: rcu_preempt CBs=90 bl=10
rcuop/5-55 [001] d..1 204.675671: rcu_batch_start: rcu_preempt CBs=14 bl=10
rcuop/2-33 [002] d..1 205.162229: rcu_batch_start: rcu_preempt CBs=649 bl=10
rcuop/3-40 [000] d..1 205.418214: rcu_batch_start: rcu_preempt CBs=291 bl=10
rcuop/3-40 [003] d..1 206.134204: rcu_batch_start: rcu_preempt CBs=174 bl=10
rcuop/0-15 [003] d..1 206.726311: rcu_batch_start: rcu_preempt CBs=738 bl=10
rcuop/1-26 [001] d..1 206.814168: rcu_batch_start: rcu_preempt CBs=865 bl=10
rcuop/3-40 [003] d..1 207.278178: rcu_batch_start: rcu_preempt CBs=287 bl=10
rcuop/1-26 [001] d..1 208.826279: rcu_batch_start: rcu_preempt CBs=506 bl=10
<snip>
An app launch:
<snip>
rcuop/3-40 [002] d..1 322.118620: rcu_batch_start: rcu_preempt CBs=99 bl=10
rcuop/4-48 [005] dn.1 322.454052: rcu_batch_start: rcu_preempt CBs=270 bl=10
rcuop/5-55 [005] d..1 322.454109: rcu_batch_start: rcu_preempt CBs=91 bl=10
rcuop/5-55 [007] d..1 322.470054: rcu_batch_start: rcu_preempt CBs=106 bl=10
rcuop/6-62 [005] d..1 322.482120: rcu_batch_start: rcu_preempt CBs=231 bl=10
rcuop/4-48 [001] d..1 322.494150: rcu_batch_start: rcu_preempt CBs=227 bl=10
<...>-69 [002] d..1 322.502442: rcu_batch_start: rcu_preempt CBs=3350 bl=26
rcuop/1-26 [001] d..1 322.646099: rcu_batch_start: rcu_preempt CBs=1685 bl=13
rcuop/2-33 [001] d..1 322.670071: rcu_batch_start: rcu_preempt CBs=438 bl=10
rcuop/1-26 [001] d..1 322.674120: rcu_batch_start: rcu_preempt CBs=18 bl=10
rcuop/2-33 [003] d..1 322.690152: rcu_batch_start: rcu_preempt CBs=10 bl=10
rcuop/1-26 [002] d..1 322.698104: rcu_batch_start: rcu_preempt CBs=10 bl=10
rcuop/3-40 [002] d..1 322.706167: rcu_batch_start: rcu_preempt CBs=313 bl=10
rcuop/2-33 [003] d..1 322.710075: rcu_batch_start: rcu_preempt CBs=15 bl=10
rcuop/3-40 [002] d..1 322.742137: rcu_batch_start: rcu_preempt CBs=13 bl=10
rcuop/5-55 [000] d..1 322.754270: rcu_batch_start: rcu_preempt CBs=157 bl=10
rcuop/3-40 [000] d..1 322.762182: rcu_batch_start: rcu_preempt CBs=17 bl=10
rcuop/2-33 [003] d..1 322.774088: rcu_batch_start: rcu_preempt CBs=38 bl=10
rcuop/3-40 [000] d..1 322.778131: rcu_batch_start: rcu_preempt CBs=23 bl=10
rcuop/1-26 [002] d..1 322.790105: rcu_batch_start: rcu_preempt CBs=33 bl=10
rcuop/4-48 [001] d..1 322.798074: rcu_batch_start: rcu_preempt CBs=340 bl=10
rcuop/2-33 [002] d..1 322.806158: rcu_batch_start: rcu_preempt CBs=18 bl=10
rcuop/1-26 [002] d..1 322.814057: rcu_batch_start: rcu_preempt CBs=18 bl=10
rcuop/0-15 [001] d..1 322.822476: rcu_batch_start: rcu_preempt CBs=333 bl=10
rcuop/4-48 [003] d..1 322.830102: rcu_batch_start: rcu_preempt CBs=11 bl=10
rcuop/2-33 [001] d..1 322.846109: rcu_batch_start: rcu_preempt CBs=80 bl=10
rcuop/3-40 [001] d..1 322.854162: rcu_batch_start: rcu_preempt CBs=145 bl=10
rcuop/4-48 [003] d..1 322.874129: rcu_batch_start: rcu_preempt CBs=21 bl=10
rcuop/3-40 [001] d..1 322.878149: rcu_batch_start: rcu_preempt CBs=43 bl=10
rcuop/3-40 [001] d..1 322.906273: rcu_batch_start: rcu_preempt CBs=10 bl=10
rcuop/4-48 [001] d..1 322.918201: rcu_batch_start: rcu_preempt CBs=23 bl=10
rcuop/2-33 [001] d..1 322.926212: rcu_batch_start: rcu_preempt CBs=86 bl=10
rcuop/2-33 [001] d..1 322.946251: rcu_batch_start: rcu_preempt CBs=12 bl=10
rcuop/5-55 [003] d..1 322.954482: rcu_batch_start: rcu_preempt CBs=70 bl=10
rcuop/2-33 [003] d..1 322.978146: rcu_batch_start: rcu_preempt CBs=20 bl=10
rcuop/1-26 [002] d..1 323.014290: rcu_batch_start: rcu_preempt CBs=230 bl=10
rcuop/4-48 [001] d..1 323.026119: rcu_batch_start: rcu_preempt CBs=73 bl=10
rcuop/5-55 [003] d..1 323.026175: rcu_batch_start: rcu_preempt CBs=94 bl=10
rcuop/3-40 [001] d..1 323.035310: rcu_batch_start: rcu_preempt CBs=70 bl=10
rcuop/0-15 [001] d..1 323.046231: rcu_batch_start: rcu_preempt CBs=165 bl=10
rcuop/6-62 [005] d..1 323.066132: rcu_batch_start: rcu_preempt CBs=179 bl=10
rcuop/1-26 [002] d..1 323.174202: rcu_batch_start: rcu_preempt CBs=61 bl=10
rcuop/2-33 [003] d..1 323.190203: rcu_batch_start: rcu_preempt CBs=80 bl=10
rcuop/3-40 [003] d..1 323.206210: rcu_batch_start: rcu_preempt CBs=84 bl=10
rcuop/2-33 [003] d..1 323.226880: rcu_batch_start: rcu_preempt CBs=5 bl=10
<snip>
It is on Android with 5.10 kernel running. I do not see that queue_rcu_work() makes
some noise.
Joel Could you please post your batch_start trace point output where you see the noise?
--
Uladzislau Rezki
next prev parent reply other threads:[~2022-10-24 19:57 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-19 22:51 [PATCH rcu 0/14] Lazy call_rcu() updates for v6.2 Paul E. McKenney
2022-10-19 22:51 ` [PATCH rcu 01/14] rcu: Simplify rcu_init_nohz() cpumask handling Paul E. McKenney
2022-10-19 22:51 ` [PATCH rcu 02/14] rcu: Fix late wakeup when flush of bypass cblist happens Paul E. McKenney
2022-10-19 22:51 ` [PATCH rcu 03/14] rcu: Fix missing nocb gp wake on rcu_barrier() Paul E. McKenney
2022-10-19 22:51 ` [PATCH rcu 04/14] rcu: Make call_rcu() lazy to save power Paul E. McKenney
2022-10-19 22:51 ` [PATCH rcu 05/14] rcu: Refactor code a bit in rcu_nocb_do_flush_bypass() Paul E. McKenney
2022-10-19 22:51 ` [PATCH rcu 06/14] rcu: Shrinker for lazy rcu Paul E. McKenney
2022-10-19 22:51 ` [PATCH rcu 07/14] rcuscale: Add laziness and kfree tests Paul E. McKenney
2022-10-19 22:51 ` [PATCH rcu 08/14] percpu-refcount: Use call_rcu_flush() for atomic switch Paul E. McKenney
2022-10-19 22:51 ` [PATCH rcu 09/14] rcu/sync: Use call_rcu_flush() instead of call_rcu Paul E. McKenney
2022-10-19 22:51 ` [PATCH rcu 10/14] rcu/rcuscale: Use call_rcu_flush() for async reader test Paul E. McKenney
2022-10-19 22:51 ` [PATCH rcu 11/14] rcu/rcutorture: Use call_rcu_flush() where needed Paul E. McKenney
2022-10-19 22:51 ` [PATCH rcu 12/14] scsi/scsi_error: Use call_rcu_flush() instead of call_rcu() Paul E. McKenney
2022-10-19 22:51 ` [PATCH rcu 13/14] workqueue: Make queue_rcu_work() use call_rcu_flush() Paul E. McKenney
2022-10-24 0:36 ` Joel Fernandes
2022-10-24 3:15 ` Paul E. McKenney
2022-10-24 10:49 ` Uladzislau Rezki
2022-10-24 12:23 ` Uladzislau Rezki [this message]
2022-10-24 14:31 ` Joel Fernandes
2022-10-24 15:39 ` Paul E. McKenney
2022-10-24 16:25 ` Uladzislau Rezki
2022-10-24 16:48 ` Paul E. McKenney
2022-10-24 16:55 ` Uladzislau Rezki
2022-10-24 17:08 ` Uladzislau Rezki
2022-10-24 17:20 ` Joel Fernandes
2022-10-24 17:35 ` Paul E. McKenney
2022-10-24 20:12 ` Joel Fernandes
2022-10-24 20:16 ` Joel Fernandes
2022-10-25 10:48 ` Uladzislau Rezki
2022-10-25 15:05 ` Joel Fernandes
2022-10-26 20:35 ` Uladzislau Rezki
2022-10-24 20:19 ` Paul E. McKenney
2022-10-24 20:26 ` Joel Fernandes
2022-10-24 17:40 ` Uladzislau Rezki
2022-10-24 20:08 ` Joel Fernandes
2022-10-25 10:47 ` Uladzislau Rezki
2022-10-28 21:23 ` Joel Fernandes
2022-10-28 21:42 ` Joel Fernandes
2022-10-31 13:21 ` Uladzislau Rezki
2022-10-31 13:37 ` Joel Fernandes
2022-10-31 18:15 ` Joel Fernandes
2022-11-01 4:49 ` Uladzislau Rezki
2022-10-24 16:54 ` Joel Fernandes
2022-10-19 22:51 ` [PATCH rcu 14/14] rxrpc: Use call_rcu_flush() instead of call_rcu() Paul E. McKenney
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=Y1aDy3maaO39ClSU@pc636 \
--to=urezki@gmail.com \
--cc=joel@joelfernandes.org \
--cc=kernel-team@fb.com \
--cc=linux-kernel@vger.kernel.org \
--cc=paulmck@kernel.org \
--cc=rcu@vger.kernel.org \
--cc=rostedt@goodmis.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.