In the synchronize_rcu() common case, we will have less than
SR_MAX_USERS_WAKE_FROM_GP number of users per GP. Waking up the kworker
is pointless just to free the last injected wait head since at that point,
all the users have already been awakened.
Introduce a new counter to track this and prevent the wakeup in the
common case.
Signed-off-by: Joel Fernandes (Google) <[email protected]>
---
kernel/rcu/tree.c | 36 +++++++++++++++++++++++++++++++-----
kernel/rcu/tree.h | 1 +
2 files changed, 32 insertions(+), 5 deletions(-)
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 12978049cb99..cba3a82e9ed9 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -96,6 +96,7 @@ static struct rcu_state rcu_state = {
.ofl_lock = __ARCH_SPIN_LOCK_UNLOCKED,
.srs_cleanup_work = __WORK_INITIALIZER(rcu_state.srs_cleanup_work,
rcu_sr_normal_gp_cleanup_work),
+ .srs_cleanups_pending = ATOMIC_INIT(0),
};
/* Dump rcu_node combining tree at boot to verify correct setup. */
@@ -1641,8 +1642,11 @@ static void rcu_sr_normal_gp_cleanup_work(struct work_struct *work)
* the done tail list manipulations are protected here.
*/
done = smp_load_acquire(&rcu_state.srs_done_tail);
- if (!done)
+ if (!done) {
+ /* See comments below. */
+ atomic_dec_return_release(&rcu_state.srs_cleanups_pending);
return;
+ }
WARN_ON_ONCE(!rcu_sr_is_wait_head(done));
head = done->next;
@@ -1665,6 +1669,9 @@ static void rcu_sr_normal_gp_cleanup_work(struct work_struct *work)
rcu_sr_put_wait_head(rcu);
}
+
+ /* Order list manipulations with atomic access. */
+ atomic_dec_return_release(&rcu_state.srs_cleanups_pending);
}
/*
@@ -1672,7 +1679,7 @@ static void rcu_sr_normal_gp_cleanup_work(struct work_struct *work)
*/
static void rcu_sr_normal_gp_cleanup(void)
{
- struct llist_node *wait_tail, *next, *rcu;
+ struct llist_node *wait_tail, *next = NULL, *rcu = NULL;
int done = 0;
wait_tail = rcu_state.srs_wait_tail;
@@ -1698,16 +1705,35 @@ static void rcu_sr_normal_gp_cleanup(void)
break;
}
- // concurrent sr_normal_gp_cleanup work might observe this update.
- smp_store_release(&rcu_state.srs_done_tail, wait_tail);
+ /*
+ * Fast path, no more users to process. Remove the last wait head
+ * if no inflight-workers. If there are in-flight workers, let them
+ * remove the last wait head.
+ */
+ WARN_ON_ONCE(!rcu);
ASSERT_EXCLUSIVE_WRITER(rcu_state.srs_done_tail);
+ if (rcu && rcu_sr_is_wait_head(rcu) && rcu->next == NULL &&
+ /* Order atomic access with list manipulation. */
+ !atomic_read_acquire(&rcu_state.srs_cleanups_pending)) {
+ wait_tail->next = NULL;
+ rcu_sr_put_wait_head(rcu);
+ smp_store_release(&rcu_state.srs_done_tail, wait_tail);
+ return;
+ }
+
+ /* Concurrent sr_normal_gp_cleanup work might observe this update. */
+ smp_store_release(&rcu_state.srs_done_tail, wait_tail);
+
/*
* We schedule a work in order to perform a final processing
* of outstanding users(if still left) and releasing wait-heads
* added by rcu_sr_normal_gp_init() call.
*/
- queue_work(system_highpri_wq, &rcu_state.srs_cleanup_work);
+ atomic_inc(&rcu_state.srs_cleanups_pending);
+ if (!queue_work(system_highpri_wq, &rcu_state.srs_cleanup_work)) {
+ atomic_dec(&rcu_state.srs_cleanups_pending);
+ }
}
/*
diff --git a/kernel/rcu/tree.h b/kernel/rcu/tree.h
index 2832787cee1d..f162b947c5b6 100644
--- a/kernel/rcu/tree.h
+++ b/kernel/rcu/tree.h
@@ -420,6 +420,7 @@ struct rcu_state {
struct llist_node *srs_done_tail; /* ready for GP users. */
struct sr_wait_node srs_wait_nodes[SR_NORMAL_GP_WAIT_HEAD_MAX];
struct work_struct srs_cleanup_work;
+ atomic_t srs_cleanups_pending; /* srs inflight worker cleanups. */
};
/* Values for rcu_state structure's gp_flags field. */
--
2.34.1
On Thu, Mar 7, 2024 at 6:48 PM Joel Fernandes (Google)
<[email protected]> wrote:
>
> In the synchronize_rcu() common case, we will have less than
> SR_MAX_USERS_WAKE_FROM_GP number of users per GP. Waking up the kworker
> is pointless just to free the last injected wait head since at that point,
> all the users have already been awakened.
>
> Introduce a new counter to track this and prevent the wakeup in the
> common case.
>
> Signed-off-by: Joel Fernandes (Google) <[email protected]>
> ---
Forgot to mention, this is based on the latest RCU -dev branch and
passes light rcutorture testing on all configs. Heavier rcutorture
testing (60 minutes) was performed on TREE03.
Thanks.
On Thu, Mar 07, 2024 at 06:52:14PM -0500, Joel Fernandes wrote:
> On Thu, Mar 7, 2024 at 6:48 PM Joel Fernandes (Google)
> <[email protected]> wrote:
> >
> > In the synchronize_rcu() common case, we will have less than
> > SR_MAX_USERS_WAKE_FROM_GP number of users per GP. Waking up the kworker
> > is pointless just to free the last injected wait head since at that point,
> > all the users have already been awakened.
> >
> > Introduce a new counter to track this and prevent the wakeup in the
> > common case.
> >
> > Signed-off-by: Joel Fernandes (Google) <[email protected]>
> > ---
>
> Forgot to mention, this is based on the latest RCU -dev branch and
> passes light rcutorture testing on all configs. Heavier rcutorture
> testing (60 minutes) was performed on TREE03.
Very good, thank you!
Uladzislau, could you please pull this into the next series you send?
I can then replace your commits in -rcu with the updated series.
Thanx, Paul
On Thu, Mar 07, 2024 at 04:06:06PM -0800, Paul E. McKenney wrote:
> On Thu, Mar 07, 2024 at 06:52:14PM -0500, Joel Fernandes wrote:
> > On Thu, Mar 7, 2024 at 6:48 PM Joel Fernandes (Google)
> > <[email protected]> wrote:
> > >
> > > In the synchronize_rcu() common case, we will have less than
> > > SR_MAX_USERS_WAKE_FROM_GP number of users per GP. Waking up the kworker
> > > is pointless just to free the last injected wait head since at that point,
> > > all the users have already been awakened.
> > >
> > > Introduce a new counter to track this and prevent the wakeup in the
> > > common case.
> > >
> > > Signed-off-by: Joel Fernandes (Google) <[email protected]>
> > > ---
> >
> > Forgot to mention, this is based on the latest RCU -dev branch and
> > passes light rcutorture testing on all configs. Heavier rcutorture
> > testing (60 minutes) was performed on TREE03.
>
> Very good, thank you!
>
> Uladzislau, could you please pull this into the next series you send?
> I can then replace your commits in -rcu with the updated series.
>
Absolutely. I will go through it and send out the next version!
--
Uladzislau Rezki
On Fri, Mar 08, 2024 at 04:05:15PM +0100, Uladzislau Rezki wrote:
> On Thu, Mar 07, 2024 at 04:06:06PM -0800, Paul E. McKenney wrote:
> > On Thu, Mar 07, 2024 at 06:52:14PM -0500, Joel Fernandes wrote:
> > > On Thu, Mar 7, 2024 at 6:48 PM Joel Fernandes (Google)
> > > <[email protected]> wrote:
> > > >
> > > > In the synchronize_rcu() common case, we will have less than
> > > > SR_MAX_USERS_WAKE_FROM_GP number of users per GP. Waking up the kworker
> > > > is pointless just to free the last injected wait head since at that point,
> > > > all the users have already been awakened.
> > > >
> > > > Introduce a new counter to track this and prevent the wakeup in the
> > > > common case.
> > > >
> > > > Signed-off-by: Joel Fernandes (Google) <[email protected]>
> > > > ---
> > >
> > > Forgot to mention, this is based on the latest RCU -dev branch and
> > > passes light rcutorture testing on all configs. Heavier rcutorture
> > > testing (60 minutes) was performed on TREE03.
> >
> > Very good, thank you!
> >
> > Uladzislau, could you please pull this into the next series you send?
> > I can then replace your commits in -rcu with the updated series.
> >
> Absolutely. I will go through it and send out the next version!
>
Joel, i sent out the v6: [PATCH v6 0/6] Reduce synchronize_rcu() latency(v6)
Could you please rework the patch on latest tip once the series i sent is
settled on Paul's dev?
I have not sent your patch because it is not cleanly applied and needs some
review.
Thank you in advance!
--
Uladzislau Rezki
On Fri, Mar 08, 2024 at 06:38:37PM +0100, Uladzislau Rezki wrote:
> On Fri, Mar 08, 2024 at 04:05:15PM +0100, Uladzislau Rezki wrote:
> > On Thu, Mar 07, 2024 at 04:06:06PM -0800, Paul E. McKenney wrote:
> > > On Thu, Mar 07, 2024 at 06:52:14PM -0500, Joel Fernandes wrote:
> > > > On Thu, Mar 7, 2024 at 6:48 PM Joel Fernandes (Google)
> > > > <[email protected]> wrote:
> > > > >
> > > > > In the synchronize_rcu() common case, we will have less than
> > > > > SR_MAX_USERS_WAKE_FROM_GP number of users per GP. Waking up the kworker
> > > > > is pointless just to free the last injected wait head since at that point,
> > > > > all the users have already been awakened.
> > > > >
> > > > > Introduce a new counter to track this and prevent the wakeup in the
> > > > > common case.
> > > > >
> > > > > Signed-off-by: Joel Fernandes (Google) <[email protected]>
> > > > > ---
> > > >
> > > > Forgot to mention, this is based on the latest RCU -dev branch and
> > > > passes light rcutorture testing on all configs. Heavier rcutorture
> > > > testing (60 minutes) was performed on TREE03.
> > >
> > > Very good, thank you!
> > >
> > > Uladzislau, could you please pull this into the next series you send?
> > > I can then replace your commits in -rcu with the updated series.
> > >
> > Absolutely. I will go through it and send out the next version!
> >
>
> Joel, i sent out the v6: [PATCH v6 0/6] Reduce synchronize_rcu() latency(v6)
>
> Could you please rework the patch on latest tip once the series i sent is
> settled on Paul's dev?
It is there now.
Thanx, Paul
> I have not sent your patch because it is not cleanly applied and needs some
> review.
>
> Thank you in advance!
>
> --
> Uladzislau Rezki
Le Thu, Mar 07, 2024 at 06:48:51PM -0500, Joel Fernandes (Google) a ?crit :
> In the synchronize_rcu() common case, we will have less than
> SR_MAX_USERS_WAKE_FROM_GP number of users per GP. Waking up the kworker
> is pointless just to free the last injected wait head since at that point,
> all the users have already been awakened.
>
> Introduce a new counter to track this and prevent the wakeup in the
> common case.
>
> Signed-off-by: Joel Fernandes (Google) <[email protected]>
Reviewed-by: Frederic Weisbecker <[email protected]>
On 3/8/2024 4:52 PM, Paul E. McKenney wrote:
> On Fri, Mar 08, 2024 at 06:38:37PM +0100, Uladzislau Rezki wrote:
>> On Fri, Mar 08, 2024 at 04:05:15PM +0100, Uladzislau Rezki wrote:
>>> On Thu, Mar 07, 2024 at 04:06:06PM -0800, Paul E. McKenney wrote:
>>>> On Thu, Mar 07, 2024 at 06:52:14PM -0500, Joel Fernandes wrote:
>>>>> On Thu, Mar 7, 2024 at 6:48 PM Joel Fernandes (Google)
>>>>> <[email protected]> wrote:
>>>>>>
>>>>>> In the synchronize_rcu() common case, we will have less than
>>>>>> SR_MAX_USERS_WAKE_FROM_GP number of users per GP. Waking up the kworker
>>>>>> is pointless just to free the last injected wait head since at that point,
>>>>>> all the users have already been awakened.
>>>>>>
>>>>>> Introduce a new counter to track this and prevent the wakeup in the
>>>>>> common case.
>>>>>>
>>>>>> Signed-off-by: Joel Fernandes (Google) <[email protected]>
>>>>>> ---
>>>>>
>>>>> Forgot to mention, this is based on the latest RCU -dev branch and
>>>>> passes light rcutorture testing on all configs. Heavier rcutorture
>>>>> testing (60 minutes) was performed on TREE03.
>>>>
>>>> Very good, thank you!
>>>>
>>>> Uladzislau, could you please pull this into the next series you send?
>>>> I can then replace your commits in -rcu with the updated series.
>>>>
>>> Absolutely. I will go through it and send out the next version!
>>>
>>
>> Joel, i sent out the v6: [PATCH v6 0/6] Reduce synchronize_rcu() latency(v6)
>>
>> Could you please rework the patch on latest tip once the series i sent is
>> settled on Paul's dev?
>
> It is there now.
Thanks, I rebased on it and sent a v2 (along with that other comment for the
hotplug race ;-)).
- Joel