[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <3074f89b32c440749ae4adbe52ae968170bef7ff.camel@kernel.org>
Date: Wed, 08 Jun 2022 19:43:07 +0200
From: nicolas saenz julienne <nsaenz@...nel.org>
To: Frederic Weisbecker <frederic@...nel.org>
Cc: LKML <linux-kernel@...r.kernel.org>,
Peter Zijlstra <peterz@...radead.org>,
Phil Auld <pauld@...hat.com>,
Alex Belits <abelits@...vell.com>,
Xiongfeng Wang <wangxiongfeng2@...wei.com>,
Neeraj Upadhyay <quic_neeraju@...cinc.com>,
Thomas Gleixner <tglx@...utronix.de>,
Yu Liao <liaoyu15@...wei.com>,
Boqun Feng <boqun.feng@...il.com>,
"Paul E . McKenney" <paulmck@...nel.org>,
Marcelo Tosatti <mtosatti@...hat.com>,
Paul Gortmaker <paul.gortmaker@...driver.com>,
Uladzislau Rezki <uladzislau.rezki@...y.com>,
Joel Fernandes <joel@...lfernandes.org>,
Mark Rutland <mark.rutland@....com>
Subject: Re: [PATCH 20/21] rcu/context_tracking: Merge dynticks counter and
context tracking states
Hi Frederic,
On Wed, 2022-06-08 at 16:29 +0200, Frederic Weisbecker wrote:
> On Tue, May 31, 2022 at 06:15:36PM +0200, nicolas saenz julienne wrote:
> > On Tue, 2022-05-31 at 16:23 +0200, Frederic Weisbecker wrote:
> > > But idle at least is an exception and CONTEXT_IDLE will remain during the
> > > interrupt handling. It's not that trivial to handle the idle case because
> > > ct_irq_exit() needs to know that it is called between ct_idle_enter() and
> > > ct_idle_exit().
> >
> > Just for the record, this behaviour was already here regardless of this series,
> > so it's not something it needs to fix.
>
> Right.
>
> >
> > Something like this should work, right?
> >
> > ct_idle_enter()
> > //IRQ or NMI
> > if (__ct_state() == CONTEXT_IDLE)
> > ct_idle_exit()
>
> Right but that's one more costly operation (atomic_add_return())
>
> > ct_irq_enter()
>
> Ideally this should increment by RCU_DYNTICKS_IDX - CONTEXT_IDLE
>
> > ...
> > ct_irq_exit()
>
> And this should increment by RCU_DYNTICKS_IDX + CONTEXT_IDLE
>
> I guess the CONTEXT_IDLE state should be remembered on some per cpu
> variable somewhere.
>
> BTW one interesting optimization to do when an idle interrupt leads to
> setting need_resched() would be to have:
>
> idle_loop() {
> while (!need_resched) {
> rcu_idle_enter();
> mwait();
> //IRQ {
> rcu_irq_enter();
> do_irq()... //set need_resched()
> rcu_irq_exit() // but no need to do the atomic_add_return() here
> // since we want to keep RCU watching as we'll
> // escape from idle
> }
> rcu_idle_exit() // and no need to do the atomic_add_return() here either
>
>
> That's two expensive operations spared for a pretty common event.
>
>
>
> > if (needs_update_state()) //using irqentry_state_t for ex.
> > ct_idle_entry()
> > ct_idle_exit()
> >
> > Note that it's not a big issue as we can work around this behaviour by checking
> > through dynticks whether a CPU is really idle.
> >
> > Do you think it's worth fixing nonetheless?
>
> Nothing urgent for sure.
Thanks for the feedback, I'll think about it.
BTW if you're patient I'll try to make a last test run on v4 of the series next
week.
Regards,
Nicolas
Powered by blists - more mailing lists