[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20220925220936.GA182999@lothringen>
Date: Mon, 26 Sep 2022 00:09:36 +0200
From: Frederic Weisbecker <frederic@...nel.org>
To: Joel Fernandes <joel@...lfernandes.org>
Cc: rcu@...r.kernel.org, linux-kernel@...r.kernel.org,
rushikesh.s.kadam@...el.com, urezki@...il.com,
neeraj.iitr10@...il.com, paulmck@...nel.org, rostedt@...dmis.org
Subject: Re: [PATCH v6 1/4] rcu: Make call_rcu() lazy to save power
On Sat, Sep 24, 2022 at 07:28:16PM -0400, Joel Fernandes wrote:
> > And then FLUSH_BP_WAKE is probably not needed anymore.
>
> It is needed as the API is in tree_nocb.h and we
> have to have that handle the details of laziness
> there rather than tree.c. We could add new apis
> to get rid of flag but it’s cleaner (and Paul seemed
> to be ok with it).
If the wake up is handled outside the flush function, as in the
diff I just posted, there is no more user left of FLUSH_BP_WAKE, IIRC...
> >> @@ -512,9 +598,16 @@ static void __call_rcu_nocb_wake(struct rcu_data *rdp, bool was_alldone,
> >> }
> >> // Need to actually to a wakeup.
> >> len = rcu_segcblist_n_cbs(&rdp->cblist);
> >> + bypass_len = rcu_cblist_n_cbs(&rdp->nocb_bypass);
> >> + lazy_len = READ_ONCE(rdp->lazy_len);
> >> if (was_alldone) {
> >> rdp->qlen_last_fqs_check = len;
> >> - if (!irqs_disabled_flags(flags)) {
> >> + // Only lazy CBs in bypass list
> >> + if (lazy_len && bypass_len == lazy_len) {
> >> + rcu_nocb_unlock_irqrestore(rdp, flags);
> >> + wake_nocb_gp_defer(rdp, RCU_NOCB_WAKE_LAZY,
> >> + TPS("WakeLazy"));
> >
> > I'm trying to think of a case where rcu_nocb_try_bypass() returns false
> > (queue to regular list) but then call_rcu() -> __call_rcu_nocb_wake() ends up
> > seeing a lazy bypass queue even though we are queueing a non-lazy callback
> > (should have flushed in this case).
> >
> > Looks like it shouldn't happen, even with concurrent (de-offloading) but just
> > in case, can we add:
>
> Yes I also feel this couldn’t happen because irq is
> off and nocb lock is held throughout the calls to
> the above 2 functions. Unless I missed the race
> you’re describing?
At least I can't find any either...
>
> >
> > WARN_ON_ONCE(lazy_len != len)
>
> But this condition can be true even in normal
> circumstances? len also contains DONE CBs
> which are ready to be invoked. Or did I miss
> something?
Duh, good point, nevermind then :-)
Thanks.
>
> Thanks,
>
> - Joel
>
> >
> >> + } else if (!irqs_disabled_flags(flags)) {
> >> /* ... if queue was empty ... */
> >> rcu_nocb_unlock_irqrestore(rdp, flags);
> >> wake_nocb_gp(rdp, false);
> >
> > Thanks.
Powered by blists - more mailing lists