LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Mike Travis <travis@sgi.com>
To: Ingo Molnar <mingo@elte.hu>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	rpurdie@rpsys.net, Jack Steiner <steiner@sgi.com>,
	linux-kernel@vger.kernel.org,
	Thomas Gleixner <tglx@linutronix.de>,
	"H. Peter Anvin" <hpa@zytor.com>
Subject: Re: [PATCH 1/1] SGI X86 UV: Provide a System Activity Indicator	driver
Date: Mon, 22 Sep 2008 07:42:46 -0700	[thread overview]
Message-ID: <48D7AEE6.7080804@sgi.com> (raw)
In-Reply-To: <20080922114820.GA13990@elte.hu>

Ingo Molnar wrote:
> * Mike Travis <travis@sgi.com> wrote:
> 
>>  arch/x86/kernel/genx2apic_uv_x.c |  138 +++++++++++++++++++++++++++++++++++++++
>>  include/asm-x86/uv/uv_hub.h      |   62 +++++++++++++++++
> 
> hm, why is this in genx2apic_uv_x.c, not a separate file?
> 
>> +	idle_notifier_register(&uv_idle_notifier);
> 
> no, we really dont want such overhead in an idle notifier. (those 
> interfaces will go away)

Hi Ingo,

The overhead is very low and I've not been able to figure out how to 
"estimate" per cpu usage without tons more code (duplicating "top" in
the kernel.)  The actual I/O (well O ;-) is once per second.  The other
important point is this is only on an SGI UV system, so no other systems
are affected.  Because of this, I'm a bit confused by your objection.

I tried a couple of other approaches, but because the timer wakeup
causes the idle state to be exited, it's difficult to determine if
the cpu was idle before the timer interrupt.  (I even tried putting a
"wasidle" in the pda to check the idle state prior to the last exit
from idle, but this did not appear to be reliable.)  The idle callback
solves this rather nicely.

Here's the same code that's been in ia64 for quite a while:

arch/ia64/sn/kernel/setup.c:
	void __init sn_setup(char **cmdline_p)
	{
		...
		ia64_mark_idle = &snidle;

arch/ia64/kernel/process.c:
	cpu_idle (void)
	{
        	void (*mark_idle)(int) = ia64_mark_idle;
		...

                if (mark_idle)
                        (*mark_idle)(1);

(The x86 callback approach is much cleaner.)

Another relevant point is that I will be adding a bit more functionality
to disable the timer interrupt on truly "idle" cpus (like have been idle
for some amount of seconds).  We would then use the "exit from idle"
callback to reestablish the timer interrupt.  [This would allow them to
enter power down states if appropriate.]

> 
> As i suggested in my previous mail about this topic, a low-frequency 
> sampling method should be used instead, to indicate system status. I 
> thought the leds drivers have all that in place already.

It is low frequency (once per second), this is just setting what's to
be sampled.

As I mentioned, this is not for LED displays (human readable), it's for the
system controller to monitor how all parts of the system are running, and
this one is just the cpu parts.  The LED driver approach would have me
registering 4096 led devices, with all their callbacks, 4096 strings saying
"LED0001", etc., and I still cannot associate a specific register bit
(AKA LED if that's what it was), with a specific cpu using the LED driver.

The LED driver is fine for a couple of blinking lights indicating overall
system activity, disk activity, etc.  (Btw, I did not see a network trigger,
or a paging trigger, or an out of memory trigger, or some other things that
might be useful for real time monitoring of the system.)

But the LED driver has way more overhead than needed for this simple application.

Thanks,
Mike

  reply	other threads:[~2008-09-22 14:42 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-09-19 14:37 [PATCH 0/1] " Mike Travis
2008-09-19 14:37 ` [PATCH 1/1] " Mike Travis
2008-09-22 11:48   ` Ingo Molnar
2008-09-22 14:42     ` Mike Travis [this message]
2008-09-28 19:42       ` Pavel Machek
2008-10-01 18:15         ` Mike Travis
2008-10-01 19:41           ` Mike Travis
2008-10-02  8:37           ` Pavel Machek
2008-10-02 14:33             ` Mike Travis
2008-09-22 14:47     ` Mike Travis
2008-10-24 11:19 Mike Travis
2008-10-24 12:01 ` Pavel Machek
2008-10-24 12:05   ` Ingo Molnar
2008-10-24 12:27     ` Mike Travis
2008-10-24 18:12       ` Andi Kleen
2008-10-24 22:18         ` Mike Travis
2008-10-24 22:24           ` Mike Travis
2008-10-27 11:42             ` Ingo Molnar
2008-10-27 14:38               ` Mike Travis
2008-10-25  6:56           ` Andi Kleen
2008-10-27 15:12             ` Mike Travis
2008-10-27 11:36           ` Ingo Molnar
2008-10-27 11:43       ` Ingo Molnar
2008-10-24 12:14   ` Mike Travis

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=48D7AEE6.7080804@sgi.com \
    --to=travis@sgi.com \
    --cc=akpm@linux-foundation.org \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=rpurdie@rpsys.net \
    --cc=steiner@sgi.com \
    --cc=tglx@linutronix.de \
    --subject='Re: [PATCH 1/1] SGI X86 UV: Provide a System Activity Indicator	driver' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).