Messages in this thread | | | Subject | Re: [PATCH 20/21] rcu/context_tracking: Merge dynticks counter and context tracking states | From | nicolas saenz julienne <> | Date | Wed, 08 Jun 2022 19:43:07 +0200 |
| |
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
| |