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

From: Mathieu Desnoyers
Date: Wed Sep 08 2010 - 12:51:06 EST


* Thomas Gleixner (tglx@xxxxxxxxxxxxx) 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/