Message-ID: <20100908165054.GA22185@Krystal>
Date:	Wed, 8 Sep 2010 12:50:54 -0400
From:	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
To:	Thomas Gleixner <tglx@...utronix.de>
Cc:	linux-kernel@...r.kernel.org, ltt-dev@...ts.casi.polymtl.ca,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Ingo Molnar <mingo@...e.hu>,
	Peter Zijlstra <peterz@...radead.org>,
	Steven Rostedt <rostedt@...dmis.org>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Li Zefan <lizf@...fujitsu.com>,
	Lai Jiangshan <laijs@...fujitsu.com>,
	Johannes Berg <johannes.berg@...el.com>,
	Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>,
	Arnaldo Carvalho de Melo <acme@...radead.org>,
	Tom Zanussi <tzanussi@...il.com>,
	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
	Andi Kleen <andi@...stfloor.org>
Subject: Re: [RFC PATCH 1/2] Idle notifier standardization (v2)

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

Powered by blists - more mailing lists