Messages in this thread | | | Date | Mon, 11 Aug 2008 18:02:35 +0200 | From | Ingo Molnar <> | Subject | Re: [PATCH 3/6] x86_64 UV: Use blinking LED for heartbeat display |
| |
* Mike Travis <travis@sgi.com> wrote:
> +#ifdef CONFIG_CLOCKSOURCE_WATCHDOG > +static void uv_display_heartbeat(void) > +{ > + int cpu; > + > + uv_hub_info->led_heartbeat_count = nr_cpu_ids; > + > + for_each_online_cpu(cpu) { > + struct uv_hub_info_s *hub = uv_cpu_hub_info(cpu); > + > + if (hub->led_heartbeat_count > 0) { > + uv_set_led_bits_on(cpu, LED_CPU_BLINK, > + LED_CPU_HEARTBEAT); > + --hub->led_heartbeat_count; > + }
this too is a bad idea. Imagine 16K cores and assume that each such iteration takes a few usecs (we write cross CPU) and you've got a GHz-ish CPU. That can easily be _milliseconds_ of delay (or more) - and in a function (the clocksource watchdog) that is all about precise timings.
It is also very non-preemptable.
Why not have a separate per cpu kthread for this that does this in a preemptable manner?
Also, why not let each CPU's heartbeat be set in a hierarchy instead of by _all_ CPUs. That way you get a nice constant-ish overhead instead of the current crazy quadratic(nr_cpus) behavior. I.e. let each CPU be monitored by its neighbor (cpu_id + 1), by it's second-order neighbor (cpu_id + 2), third-order neighbor (cpu_id + 4), etc.
That still gives pretty good coverage in practice while avoiding the quadratic nature.
Ingo
| |