Re: [RFC PATCH 1/2] Idle notifier standardization (v2)

[Mathieu Desnoyers <mathieu.desnoyers@efficios.com>]

* Thomas Gleixner (tglx@linutronix.de) wrote:
> On Wed, 8 Sep 2010, Mathieu Desnoyers wrote:
> 
> > Move idle notifiers into arch-agnostic code. Adapt x86 64 accordingly to call
> > the new architecture-agnostic notifiers rather than its own.
> > 
> > The architectures implementing the idle notifier define the config option:
> > 
> > CONFIG_HAVE_IDLE_NOTIFIER
> > 
> > Changelog since v1:
> > * Add CONFIG_HAVE_IDLE_NOTIFIER.
> > 
> > 
> > This is needed by the generic ring buffer. It needs to let the system sleep if
> > there is nothing going on other than tracing on a cpu, but for streaming it also
> > has to provide an upper bound on the delay before the information is sent out
> > (for merging across event streams coming from different CPUs). These notifiers
> > lets the ring buffer use deferrable timers to perform data delivery by forcing a
> > buffer flush before going to sleep.
> 
> I really have a hard time to understand how this is related to
> deferrable timers. The whole point of deferrable timers is that they
> do not fire when the machine is idle. 
> 
> I understand that you want to not care about the timer, but at the
> same time you want to flush the buffer when going idle. 
> 
> So why do you keep the timer armed ? Just that it fires when the CPU
> comes out of a long idle sleep and you flush the buffer again? So why
> not cancel the timer on idle enter and rearm it when the machine
> starts again?

That sounds exactly like what I am trying to achieve. Letting the timer fire
upon exit from idle was a side-effect I could really do without.

> 
> So really, the reason why you want those notifiers is to flush the
> buffer and _not_ to allow you the usage of deferrable timers.

Yep.

> 
> Aside of that I really hate it to sprinkle the same notifier crap into
> all arch idle functions - you even blindly copied the 64 bit
> implementation to 32bit instead of moving it into the shared process.c
> file.

Yep, I would have moved it to process.c, but I guess I'll hook on nohz instead.

> 
> The whole point of your exercise seems to be power saving related, so
> why don't you hook that tracer flush stuff into
> tick_nohz_stop_sched_tick() and tick_nohz_restart_sched_tick()
> instead?  Those are called on idle enter and exit from all archs which
> use NOHZ, so you should be all set. No need for adding that notifier
> horror to every arch, really.

Yep. I'll do that. Thanks a ton for looking into this.

Mathieu

> 
> Thanks,
> 
> 	tglx

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com