* [PATCH 0/6] SGI UV: Provide a LED driver and some System Activity Indicators
@ 2008-08-08 0:56 Mike Travis
2008-08-08 0:56 ` [PATCH 1/6] x86_64 UV: Provide a LED driver for UV Systems Mike Travis
` (5 more replies)
0 siblings, 6 replies; 17+ messages in thread
From: Mike Travis @ 2008-08-08 0:56 UTC (permalink / raw)
To: Ingo Molnar; +Cc: Andrew Morton, Jack Steiner, linux-kernel
Add a LED driver for UV systems and some functions to display system
activities such as when a CPU is active, and when a CPU is handling
timer interrupts.
Based on linux-2.6.tip/master.
Signed-off-by: Mike Travis <travis@sgi.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Tony Luck <tony.luck@intel.com>
--
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/6] x86_64 UV: Provide a LED driver for UV Systems
2008-08-08 0:56 [PATCH 0/6] SGI UV: Provide a LED driver and some System Activity Indicators Mike Travis
@ 2008-08-08 0:56 ` Mike Travis
2008-08-08 0:56 ` [PATCH 2/6] x86_64 UV: Use LED to indicate CPU is active Mike Travis
` (4 subsequent siblings)
5 siblings, 0 replies; 17+ messages in thread
From: Mike Travis @ 2008-08-08 0:56 UTC (permalink / raw)
To: Ingo Molnar
Cc: Andrew Morton, Jack Steiner, linux-kernel, H. Peter Anvin,
Thomas Gleixner
[-- Attachment #1: uv_led_driver_x86 --]
[-- Type: text/plain, Size: 4056 bytes --]
* Add base functionality for writing to the LED's on
the x86_64 UV architecture.
Note that this is a RAS feature that allows external monitoring of
various cpu state indicators, not just providing "pretty blinking
lights", as the LED state is readable by the system controller.
Based on linux-2.6.tip/master.
Signed-off-by: Mike Travis <travis@sgi.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
arch/x86/kernel/genx2apic_uv_x.c | 2 +
include/asm-x86/uv/uv_hub.h | 60 ++++++++++++++++++++++++++++++++++++++-
2 files changed, 61 insertions(+), 1 deletion(-)
--- linux-2.6.tip.orig/arch/x86/kernel/genx2apic_uv_x.c
+++ linux-2.6.tip/arch/x86/kernel/genx2apic_uv_x.c
@@ -433,6 +433,8 @@ static __init void uv_system_init(void)
uv_cpu_hub_info(cpu)->gnode_upper = gnode_upper;
uv_cpu_hub_info(cpu)->global_mmr_base = mmr_base;
uv_cpu_hub_info(cpu)->coherency_domain_number = 0;/* ZZZ */
+ uv_cpu_hub_info(cpu)->led_offset = LED_LOCAL_MMR_BASE + lcpu;
+ uv_cpu_hub_info(cpu)->led_state = 0;
uv_node_to_blade[nid] = blade;
uv_cpu_to_blade[cpu] = blade;
max_pnode = max(pnode, max_pnode);
--- linux-2.6.tip.orig/include/asm-x86/uv/uv_hub.h
+++ linux-2.6.tip/include/asm-x86/uv/uv_hub.h
@@ -123,6 +123,7 @@ struct uv_hub_info_s {
unsigned long gnode_upper;
unsigned long lowmem_remap_top;
unsigned long lowmem_remap_base;
+ unsigned long led_offset;
unsigned short pnode;
unsigned short pnode_mask;
unsigned short coherency_domain_number;
@@ -130,6 +131,7 @@ struct uv_hub_info_s {
unsigned char blade_processor_id;
unsigned char m_val;
unsigned char n_val;
+ unsigned char led_state;
};
DECLARE_PER_CPU(struct uv_hub_info_s, __uv_hub_info);
#define uv_hub_info (&__get_cpu_var(__uv_hub_info))
@@ -161,6 +163,19 @@ DECLARE_PER_CPU(struct uv_hub_info_s, __
((unsigned long)(p) << UV_GLOBAL_MMR64_PNODE_SHIFT)
#define UV_APIC_PNODE_SHIFT 6
+/* Local Bus from cpu's perspective */
+#define LOCAL_BUS_BASE 0x1c00000
+#define LOCAL_BUS_SIZE (4 * 1024 * 1024)
+
+/* LED windows - located at top of ACPI MMR space */
+#define LED_WINDOW_COUNT 64
+#define LED_LOCAL_MMR_BASE (LOCAL_BUS_BASE + LOCAL_BUS_SIZE - \
+ LED_WINDOW_COUNT)
+
+#define LED_CPU_HEARTBEAT 0x01 /* timer interrupt */
+#define LED_CPU_ACTIVITY 0x02 /* not idle */
+#define LED_CPU_BLINK 0xffff /* blink led */
+#define LED_CPU_HB_INTERVAL (HZ/2) /* blink once per second */
/*
* Macros for converting between kernel virtual addresses, socket local physical
@@ -276,6 +291,16 @@ static inline void uv_write_local_mmr(un
*uv_local_mmr_address(offset) = val;
}
+static inline unsigned char uv_read_local_mmr8(unsigned long offset)
+{
+ return *((unsigned char *)uv_local_mmr_address(offset));
+}
+
+static inline void uv_write_local_mmr8(unsigned long offset, unsigned char val)
+{
+ *((unsigned char *)uv_local_mmr_address(offset)) = val;
+}
+
/*
* Structures and definitions for converting between cpu, node, pnode, and blade
* numbers.
@@ -350,5 +375,38 @@ static inline int uv_num_possible_blades
return uv_possible_blades;
}
-#endif /* ASM_X86__UV__UV_HUB_H */
+/* Light up the leds */
+static inline void uv_set_led_bits(unsigned short value, unsigned char mask)
+{
+ unsigned char state = uv_hub_info->led_state;
+
+ if (value == LED_CPU_BLINK)
+ state ^= mask;
+ else
+ state = (state & ~mask) | (value & mask);
+ if (uv_hub_info->led_state != state) {
+ uv_hub_info->led_state = state;
+ uv_write_local_mmr8(uv_hub_info->led_offset, state);
+ }
+}
+
+/* Light up the leds */
+static inline void uv_set_led_bits_on(int cpu, unsigned short value,
+ unsigned char mask)
+
+{
+ unsigned char state = uv_cpu_hub_info(cpu)->led_state;
+
+ if (value == LED_CPU_BLINK)
+ state ^= mask;
+ else
+ state = (state & ~mask) | (value & mask);
+
+ if (uv_cpu_hub_info(cpu)->led_state != state) {
+ uv_cpu_hub_info(cpu)->led_state = state;
+ uv_write_local_mmr8(uv_cpu_hub_info(cpu)->led_offset, state);
+ }
+}
+
+#endif /* ASM_X86__UV__UV_HUB_H */
--
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/6] x86_64 UV: Use LED to indicate CPU is active
2008-08-08 0:56 [PATCH 0/6] SGI UV: Provide a LED driver and some System Activity Indicators Mike Travis
2008-08-08 0:56 ` [PATCH 1/6] x86_64 UV: Provide a LED driver for UV Systems Mike Travis
@ 2008-08-08 0:56 ` Mike Travis
2008-08-11 15:53 ` Ingo Molnar
2008-08-08 0:56 ` [PATCH 3/6] x86_64 UV: Use blinking LED for heartbeat display Mike Travis
` (3 subsequent siblings)
5 siblings, 1 reply; 17+ messages in thread
From: Mike Travis @ 2008-08-08 0:56 UTC (permalink / raw)
To: Ingo Molnar
Cc: Andrew Morton, Jack Steiner, linux-kernel, H. Peter Anvin,
Thomas Gleixner
[-- Attachment #1: uv_led_display_idle_x86 --]
[-- Type: text/plain, Size: 3479 bytes --]
* Add an idle callback to turn on/off a LED indicating that
the CPU is "active" (ON) or "idle" (OFF).
* Introduces a callback for "post-smp_cpus_done" processing
to setup callback.
Note that this is a RAS feature that allows external monitoring of
various cpu state indicators, not just providing "pretty blinking
lights", as the LED state is readable by the system controller.
Based on linux-2.6.tip/master.
Signed-off-by: Mike Travis <travis@sgi.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
arch/x86/kernel/genx2apic_uv_x.c | 40 +++++++++++++++++++++++++++++++++++++++
arch/x86/kernel/smpboot.c | 6 +++++
include/asm-x86/genapic_64.h | 3 ++
3 files changed, 49 insertions(+)
--- linux-2.6.tip.orig/arch/x86/kernel/genx2apic_uv_x.c
+++ linux-2.6.tip/arch/x86/kernel/genx2apic_uv_x.c
@@ -18,6 +18,7 @@
#include <linux/bootmem.h>
#include <linux/module.h>
#include <linux/hardirq.h>
+#include <asm/idle.h>
#include <asm/smp.h>
#include <asm/ipi.h>
#include <asm/genapic.h>
@@ -28,6 +29,8 @@
DEFINE_PER_CPU(int, x2apic_extra_bits);
+static __init void uv_start_system(void);
+
static enum uv_system_type uv_system_type;
static int __init uv_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
@@ -449,6 +452,9 @@ static __init void uv_system_init(void)
map_mmr_high(max_pnode);
map_config_high(max_pnode);
map_mmioh_high(max_pnode);
+
+ /* enable post-smp_cpus_done processing */
+ smp_cpus_done_system = uv_start_system;
}
/*
@@ -466,4 +472,38 @@ void __cpuinit uv_cpu_init(void)
set_x2apic_extra_bits(uv_hub_info->pnode);
}
+/*
+ * Illuminate "activity" LED when CPU is going "active",
+ * extinguish when going "idle".
+ */
+static int uv_idle(struct notifier_block *nfb, unsigned long action, void *junk)
+{
+ if (action == IDLE_START)
+ uv_set_led_bits(0, LED_CPU_ACTIVITY);
+
+ else if (action == IDLE_END)
+ uv_set_led_bits(LED_CPU_ACTIVITY, LED_CPU_ACTIVITY);
+
+ return NOTIFY_OK;
+}
+
+static struct notifier_block uv_idle_notifier = {
+ .notifier_call = uv_idle,
+};
+/*
+ * Initialize idle led callback function
+ */
+static __init void uv_init_led_idle_display(void)
+{
+ /* initialize timer for activity monitor */
+ idle_notifier_register(&uv_idle_notifier);
+}
+
+/*
+ * Initialize subsystems that need to start after the system is up
+ */
+static __init void uv_start_system(void)
+{
+ uv_init_led_idle_display();
+}
--- linux-2.6.tip.orig/arch/x86/kernel/smpboot.c
+++ linux-2.6.tip/arch/x86/kernel/smpboot.c
@@ -1200,6 +1200,9 @@ void __init native_smp_prepare_boot_cpu(
per_cpu(cpu_state, me) = CPU_ONLINE;
}
+/* post-smp_cpus_done processing */
+void (*smp_cpus_done_system)(void);
+
void __init native_smp_cpus_done(unsigned int max_cpus)
{
pr_debug("Boot done.\n");
@@ -1210,6 +1213,9 @@ void __init native_smp_cpus_done(unsigne
setup_ioapic_dest();
#endif
check_nmi_watchdog();
+
+ if (smp_cpus_done_system)
+ smp_cpus_done_system();
}
#ifdef CONFIG_HOTPLUG_CPU
--- linux-2.6.tip.orig/include/asm-x86/genapic_64.h
+++ linux-2.6.tip/include/asm-x86/genapic_64.h
@@ -47,6 +47,9 @@ enum uv_system_type {UV_NONE, UV_LEGACY_
extern enum uv_system_type get_uv_system_type(void);
extern int is_uv_system(void);
+/* system tasks to run after smp_cpus_done */
+extern void (*smp_cpus_done_system)(void);
+
extern struct genapic apic_x2apic_uv_x;
DECLARE_PER_CPU(int, x2apic_extra_bits);
extern void uv_cpu_init(void);
--
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/6] x86_64 UV: Use blinking LED for heartbeat display
2008-08-08 0:56 [PATCH 0/6] SGI UV: Provide a LED driver and some System Activity Indicators Mike Travis
2008-08-08 0:56 ` [PATCH 1/6] x86_64 UV: Provide a LED driver for UV Systems Mike Travis
2008-08-08 0:56 ` [PATCH 2/6] x86_64 UV: Use LED to indicate CPU is active Mike Travis
@ 2008-08-08 0:56 ` Mike Travis
2008-08-11 16:02 ` Ingo Molnar
2008-08-08 0:56 ` [PATCH 4/6] ia64 UV: Provide a LED driver for UV Systems Mike Travis
` (2 subsequent siblings)
5 siblings, 1 reply; 17+ messages in thread
From: Mike Travis @ 2008-08-08 0:56 UTC (permalink / raw)
To: Ingo Molnar
Cc: Andrew Morton, Jack Steiner, linux-kernel, H. Peter Anvin,
Thomas Gleixner
[-- Attachment #1: uv_led_display_hb_x86 --]
[-- Type: text/plain, Size: 4158 bytes --]
* Add a function to blink the "heartbeat" LED indicating
that the CPU is responding to timer interrupts.
* Introduces a callback for "heartbeat" display in the
clocksource_watchdog function, which in turn requires
CONFIG_CLOCKSOURCE_WATCHDOG to be set.
Note that this is a RAS feature that allows external monitoring of
various cpu state indicators, not just providing "pretty blinking
lights", as the LED state is readable by the system controller.
Based on linux-2.6.tip/master.
Signed-off-by: Mike Travis <travis@sgi.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
arch/x86/kernel/genx2apic_uv_x.c | 29 +++++++++++++++++++++++++++++
include/asm-x86/uv/uv_hub.h | 1 +
include/linux/clocksource.h | 5 +++++
kernel/time/clocksource.c | 10 +++++++++-
4 files changed, 44 insertions(+), 1 deletion(-)
--- linux-2.6.tip.orig/arch/x86/kernel/genx2apic_uv_x.c
+++ linux-2.6.tip/arch/x86/kernel/genx2apic_uv_x.c
@@ -10,6 +10,7 @@
#include <linux/kernel.h>
#include <linux/threads.h>
+#include <linux/clocksource.h>
#include <linux/cpumask.h>
#include <linux/string.h>
#include <linux/ctype.h>
@@ -360,6 +361,25 @@ static __init void uv_rtc_init(void)
sn_rtc_cycles_per_second = ticks_per_sec;
}
+#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;
+ }
+ }
+}
+#endif
+
static __init void uv_system_init(void)
{
union uvh_si_addr_map_config_u m_n_config;
@@ -438,6 +458,7 @@ static __init void uv_system_init(void)
uv_cpu_hub_info(cpu)->coherency_domain_number = 0;/* ZZZ */
uv_cpu_hub_info(cpu)->led_offset = LED_LOCAL_MMR_BASE + lcpu;
uv_cpu_hub_info(cpu)->led_state = 0;
+ uv_cpu_hub_info(cpu)->led_heartbeat_count = nr_cpu_ids;
uv_node_to_blade[nid] = blade;
uv_cpu_to_blade[cpu] = blade;
max_pnode = max(pnode, max_pnode);
@@ -455,6 +476,14 @@ static __init void uv_system_init(void)
/* enable post-smp_cpus_done processing */
smp_cpus_done_system = uv_start_system;
+
+#ifdef CONFIG_CLOCKSOURCE_WATCHDOG
+ /* enable heartbeat display callback */
+ display_heartbeat = uv_display_heartbeat;
+#else
+ printk(KERN_NOTICE "UV: CLOCKSOURCE_WATCHDOG not configured, "
+ "LED Heartbeat NOT enabled\n");
+#endif
}
/*
--- linux-2.6.tip.orig/include/asm-x86/uv/uv_hub.h
+++ linux-2.6.tip/include/asm-x86/uv/uv_hub.h
@@ -124,6 +124,7 @@ struct uv_hub_info_s {
unsigned long lowmem_remap_top;
unsigned long lowmem_remap_base;
unsigned long led_offset;
+ unsigned int led_heartbeat_count;
unsigned short pnode;
unsigned short pnode_mask;
unsigned short coherency_domain_number;
--- linux-2.6.tip.orig/include/linux/clocksource.h
+++ linux-2.6.tip/include/linux/clocksource.h
@@ -236,4 +236,9 @@ static inline void update_vsyscall_tz(vo
}
#endif
+#ifdef CONFIG_CLOCKSOURCE_WATCHDOG
+/* typically blinks "heartbeat" LED */
+extern void (*display_heartbeat)(void);
+#endif
+
#endif /* _LINUX_CLOCKSOURCE_H */
--- linux-2.6.tip.orig/kernel/time/clocksource.c
+++ linux-2.6.tip/kernel/time/clocksource.c
@@ -76,6 +76,9 @@ static DEFINE_SPINLOCK(watchdog_lock);
static cycle_t watchdog_last;
static unsigned long watchdog_resumed;
+/* arch specific heartbeat display */
+void (*display_heartbeat)(void);
+
/*
* Interval: 0.5sec Threshold: 0.0625s
*/
@@ -140,12 +143,17 @@ static void clocksource_watchdog(unsigne
}
}
+ /* If arch has a heartbeat display, then poke it */
+ if (display_heartbeat)
+ display_heartbeat();
+
if (!list_empty(&watchdog_list)) {
/*
* Cycle through CPUs to check if the CPUs stay
* synchronized to each other.
*/
- int next_cpu = next_cpu_nr(raw_smp_processor_id(), cpu_online_map);
+ int next_cpu = next_cpu_nr(raw_smp_processor_id(),
+ cpu_online_map);
if (next_cpu >= nr_cpu_ids)
next_cpu = first_cpu(cpu_online_map);
--
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 4/6] ia64 UV: Provide a LED driver for UV Systems
2008-08-08 0:56 [PATCH 0/6] SGI UV: Provide a LED driver and some System Activity Indicators Mike Travis
` (2 preceding siblings ...)
2008-08-08 0:56 ` [PATCH 3/6] x86_64 UV: Use blinking LED for heartbeat display Mike Travis
@ 2008-08-08 0:56 ` Mike Travis
2008-08-08 6:07 ` Andrew Morton
2008-08-08 0:56 ` [PATCH 5/6] ia64 UV: Use LED to indicate CPU is active Mike Travis
2008-08-08 0:56 ` [PATCH 6/6] ia64 UV: Use blinking LED for heartbeat display Mike Travis
5 siblings, 1 reply; 17+ messages in thread
From: Mike Travis @ 2008-08-08 0:56 UTC (permalink / raw)
To: Ingo Molnar; +Cc: Andrew Morton, Jack Steiner, linux-kernel, Tony Luck
[-- Attachment #1: uv_led_driver_ia64 --]
[-- Type: text/plain, Size: 3998 bytes --]
* Add base functionality for writing to the LED's on
the ia64 UV architecture.
Note that this is a RAS feature that allows external monitoring of
various cpu state indicators, not just providing "pretty blinking
lights", as the LED state is readable by the system controller.
Based on linux-2.6.tip/master.
Signed-off-by: Mike Travis <travis@sgi.com>
Cc: Tony Luck <tony.luck@intel.com>
---
arch/ia64/uv/kernel/setup.c | 2 +
include/asm-ia64/uv/uv_hub.h | 61 ++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 62 insertions(+), 1 deletion(-)
--- linux-2.6.tip.orig/arch/ia64/uv/kernel/setup.c
+++ linux-2.6.tip/arch/ia64/uv/kernel/setup.c
@@ -104,6 +104,8 @@ void __init uv_setup(char **cmdline_p)
uv_cpu_hub_info(cpu)->gnode_upper = gnode_upper;
uv_cpu_hub_info(cpu)->global_mmr_base = mmr_base;
uv_cpu_hub_info(cpu)->coherency_domain_number = 0;/* ZZZ */
+ uv_cpu_hub_info(cpu)->led_offset = LED_LOCAL_MMR_BASE + lcpu;
+ uv_cpu_hub_info(cpu)->led_state = 0;
printk(KERN_DEBUG "UV cpu %d, nid %d\n", cpu, nid);
}
}
--- linux-2.6.tip.orig/include/asm-ia64/uv/uv_hub.h
+++ linux-2.6.tip/include/asm-ia64/uv/uv_hub.h
@@ -99,6 +99,7 @@ struct uv_hub_info_s {
unsigned long gnode_upper;
unsigned long lowmem_remap_top;
unsigned long lowmem_remap_base;
+ unsigned long led_offset;
unsigned short pnode;
unsigned short pnode_mask;
unsigned short coherency_domain_number;
@@ -106,6 +107,7 @@ struct uv_hub_info_s {
unsigned char blade_processor_id;
unsigned char m_val;
unsigned char n_val;
+ unsigned char led_state;
};
DECLARE_PER_CPU(struct uv_hub_info_s, __uv_hub_info);
#define uv_hub_info (&__get_cpu_var(__uv_hub_info))
@@ -134,6 +136,20 @@ DECLARE_PER_CPU(struct uv_hub_info_s, __
#define UV_GLOBAL_MMR64_PNODE_BITS(p) \
((unsigned long)(p) << UV_GLOBAL_MMR64_PNODE_SHIFT)
+/* Local Bus from cpu's perspective */
+#define LOCAL_BUS_BASE 0x1c00000
+#define LOCAL_BUS_SIZE (4 * 1024 * 1024)
+
+/* LED windows - located at top of ACPI MMR space */
+#define LED_WINDOW_COUNT 64
+#define LED_LOCAL_MMR_BASE (LOCAL_BUS_BASE + LOCAL_BUS_SIZE - \
+ LED_WINDOW_COUNT)
+
+#define LED_CPU_HEARTBEAT 0x01 /* timer interrupt */
+#define LED_CPU_ACTIVITY 0x02 /* not idle */
+#define LED_CPU_BLINK 0xffff /* blink led */
+#define LED_CPU_HB_INTERVAL (HZ/2) /* blink once per second */
+
/*
* Macros for converting between kernel virtual addresses, socket local physical
* addresses, and UV global physical addresses.
@@ -240,6 +256,16 @@ static inline void uv_write_local_mmr(un
*uv_local_mmr_address(offset) = val;
}
+static inline unsigned char uv_read_local_mmr8(unsigned long offset)
+{
+ return *((unsigned char *)uv_local_mmr_address(offset));
+}
+
+static inline void uv_write_local_mmr8(unsigned long offset, unsigned char val)
+{
+ *((unsigned char *)uv_local_mmr_address(offset)) = val;
+}
+
/*
* Structures and definitions for converting between cpu, node, pnode, and blade
* numbers.
@@ -305,5 +331,38 @@ static inline int uv_num_possible_blades
return 1;
}
-#endif /* __ASM_IA64_UV_HUB__ */
+/* Light up the leds */
+static inline void uv_set_led_bits(unsigned short value, unsigned char mask)
+{
+ unsigned char state = uv_hub_info->led_state;
+
+ if (value == LED_CPU_BLINK)
+ state ^= mask;
+ else
+ state = (state & ~mask) | (value & mask);
+ if (uv_hub_info->led_state != state) {
+ uv_hub_info->led_state = state;
+ uv_write_local_mmr8(uv_hub_info->led_offset, state);
+ }
+}
+
+/* Light up the leds */
+static inline void uv_set_led_bits_on(int cpu, unsigned short value,
+ unsigned char mask)
+
+{
+ unsigned char state = uv_cpu_hub_info(cpu)->led_state;
+
+ if (value == LED_CPU_BLINK)
+ state ^= mask;
+ else
+ state = (state & ~mask) | (value & mask);
+
+ if (uv_cpu_hub_info(cpu)->led_state != state) {
+ uv_cpu_hub_info(cpu)->led_state = state;
+ uv_write_local_mmr8(uv_cpu_hub_info(cpu)->led_offset, state);
+ }
+}
+
+#endif /* __ASM_IA64_UV_HUB__ */
--
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 5/6] ia64 UV: Use LED to indicate CPU is active
2008-08-08 0:56 [PATCH 0/6] SGI UV: Provide a LED driver and some System Activity Indicators Mike Travis
` (3 preceding siblings ...)
2008-08-08 0:56 ` [PATCH 4/6] ia64 UV: Provide a LED driver for UV Systems Mike Travis
@ 2008-08-08 0:56 ` Mike Travis
2008-08-08 0:56 ` [PATCH 6/6] ia64 UV: Use blinking LED for heartbeat display Mike Travis
5 siblings, 0 replies; 17+ messages in thread
From: Mike Travis @ 2008-08-08 0:56 UTC (permalink / raw)
To: Ingo Molnar; +Cc: Andrew Morton, Jack Steiner, linux-kernel, Tony Luck
[-- Attachment #1: uv_led_display_idle_ia64 --]
[-- Type: text/plain, Size: 2174 bytes --]
* Add an idle callback to turn on/off a LED indicating that
the CPU is "active" (ON) or "idle" (OFF).
* Introduces a callback for "post-smp_cpus_done" processing
to setup callback.
Note that this is a RAS feature that allows external monitoring of
various cpu state indicators, not just providing "pretty blinking
lights", as the LED state is readable by the system controller.
Based on linux-2.6.tip/master.
Signed-off-by: Mike Travis <travis@sgi.com>
Cc: Tony Luck <tony.luck@intel.com>
---
arch/ia64/uv/kernel/setup.c | 19 +++++++++++++++++--
include/asm-ia64/uv/uv_hub.h | 3 +++
2 files changed, 20 insertions(+), 2 deletions(-)
--- linux-2.6.tip.orig/arch/ia64/uv/kernel/setup.c
+++ linux-2.6.tip/arch/ia64/uv/kernel/setup.c
@@ -52,6 +52,19 @@ static __init void get_lowmem_redirect(u
BUG();
}
+/*
+ * Illuminate "activity" LED when CPU is going "active",
+ * extinguish when going "idle".
+ */
+static void uv_idle(int state)
+{
+ if (state)
+ uv_set_led_bits(0, LED_CPU_ACTIVITY);
+
+ else
+ uv_set_led_bits(LED_CPU_ACTIVITY, LED_CPU_ACTIVITY);
+}
+
void __init uv_setup(char **cmdline_p)
{
union uvh_si_addr_map_config_u m_n_config;
@@ -104,9 +117,11 @@ void __init uv_setup(char **cmdline_p)
uv_cpu_hub_info(cpu)->gnode_upper = gnode_upper;
uv_cpu_hub_info(cpu)->global_mmr_base = mmr_base;
uv_cpu_hub_info(cpu)->coherency_domain_number = 0;/* ZZZ */
- uv_cpu_hub_info(cpu)->led_offset = LED_LOCAL_MMR_BASE + lcpu;
+ uv_cpu_hub_info(cpu)->led_offset = LED_LOCAL_MMR_BASE + cpu;
uv_cpu_hub_info(cpu)->led_state = 0;
printk(KERN_DEBUG "UV cpu %d, nid %d\n", cpu, nid);
}
-}
+ /* enable idle callback */
+ ia64_mark_idle = &uv_idle;
+}
--- linux-2.6.tip.orig/include/asm-ia64/uv/uv_hub.h
+++ linux-2.6.tip/include/asm-ia64/uv/uv_hub.h
@@ -150,6 +150,9 @@ DECLARE_PER_CPU(struct uv_hub_info_s, __
#define LED_CPU_BLINK 0xffff /* blink led */
#define LED_CPU_HB_INTERVAL (HZ/2) /* blink once per second */
+/* idle callback */
+extern void (*ia64_mark_idle) (int);
+
/*
* Macros for converting between kernel virtual addresses, socket local physical
* addresses, and UV global physical addresses.
--
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 6/6] ia64 UV: Use blinking LED for heartbeat display
2008-08-08 0:56 [PATCH 0/6] SGI UV: Provide a LED driver and some System Activity Indicators Mike Travis
` (4 preceding siblings ...)
2008-08-08 0:56 ` [PATCH 5/6] ia64 UV: Use LED to indicate CPU is active Mike Travis
@ 2008-08-08 0:56 ` Mike Travis
5 siblings, 0 replies; 17+ messages in thread
From: Mike Travis @ 2008-08-08 0:56 UTC (permalink / raw)
To: Ingo Molnar; +Cc: Andrew Morton, Jack Steiner, linux-kernel, Tony Luck
[-- Attachment #1: uv_led_display_hb_ia64 --]
[-- Type: text/plain, Size: 2674 bytes --]
* Add a function to blink the "heartbeat" LED indicating
that the CPU is responding to timer interrupts.
* Introduces a callback for "heartbeat" display in the
clocksource_watchdog function, which in turn requires
CONFIG_CLOCKSOURCE_WATCHDOG to be set.
Note that this is a RAS feature that allows external monitoring of
various cpu state indicators, not just providing "pretty blinking
lights", as the LED state is readable by the system controller.
Based on linux-2.6.tip/master.
Signed-off-by: Mike Travis <travis@sgi.com>
Cc: Tony Luck <tony.luck@intel.com>
---
arch/ia64/uv/kernel/setup.c | 28 ++++++++++++++++++++++++++++
include/asm-ia64/uv/uv_hub.h | 4 ++++
2 files changed, 32 insertions(+)
--- linux-2.6.tip.orig/arch/ia64/uv/kernel/setup.c
+++ linux-2.6.tip/arch/ia64/uv/kernel/setup.c
@@ -8,6 +8,7 @@
* Copyright (C) 2008 Silicon Graphics, Inc. All rights reserved.
*/
+#include <linux/clocksource.h>
#include <linux/module.h>
#include <linux/percpu.h>
#include <asm/sn/simulator.h>
@@ -65,6 +66,25 @@ static void uv_idle(int state)
uv_set_led_bits(LED_CPU_ACTIVITY, LED_CPU_ACTIVITY);
}
+#ifdef CONFIG_CLOCKSOURCE_WATCHDOG
+static void uv_display_heartbeat(void)
+{
+ int cpu;
+
+ uv_hub_info->uv_heartbeat = 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;
+ }
+ }
+}
+#endif
+
void __init uv_setup(char **cmdline_p)
{
union uvh_si_addr_map_config_u m_n_config;
@@ -124,4 +144,12 @@ void __init uv_setup(char **cmdline_p)
/* enable idle callback */
ia64_mark_idle = &uv_idle;
+
+#ifdef CONFIG_CLOCKSOURCE_WATCHDOG
+ /* enable heartbeat display callback */
+ display_heartbeat = uv_display_heartbeat;
+#else
+ printk(KERN_NOTICE "UV: CLOCKSOURCE_WATCHDOG not configured, "
+ "LED Heartbeat NOT enabled\n");
+#endif
}
--- linux-2.6.tip.orig/include/asm-ia64/uv/uv_hub.h
+++ linux-2.6.tip/include/asm-ia64/uv/uv_hub.h
@@ -100,6 +100,7 @@ struct uv_hub_info_s {
unsigned long lowmem_remap_top;
unsigned long lowmem_remap_base;
unsigned long led_offset;
+ unsigned int led_heartbeat_count;
unsigned short pnode;
unsigned short pnode_mask;
unsigned short coherency_domain_number;
@@ -153,6 +154,9 @@ DECLARE_PER_CPU(struct uv_hub_info_s, __
/* idle callback */
extern void (*ia64_mark_idle) (int);
+/* idle callback */
+extern void (*ia64_mark_idle) (int);
+
/*
* Macros for converting between kernel virtual addresses, socket local physical
* addresses, and UV global physical addresses.
--
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/6] ia64 UV: Provide a LED driver for UV Systems
2008-08-08 0:56 ` [PATCH 4/6] ia64 UV: Provide a LED driver for UV Systems Mike Travis
@ 2008-08-08 6:07 ` Andrew Morton
0 siblings, 0 replies; 17+ messages in thread
From: Andrew Morton @ 2008-08-08 6:07 UTC (permalink / raw)
To: Mike Travis; +Cc: Ingo Molnar, Jack Steiner, linux-kernel, Tony Luck
On Thu, 07 Aug 2008 17:56:43 -0700 Mike Travis <travis@sgi.com> wrote:
> * Add base functionality for writing to the LED's on
> the ia64 UV architecture.
>
> Note that this is a RAS feature that allows external monitoring of
> various cpu state indicators, not just providing "pretty blinking
> lights", as the LED state is readable by the system controller.
>
> Based on linux-2.6.tip/master.
>
> Signed-off-by: Mike Travis <travis@sgi.com>
> Cc: Tony Luck <tony.luck@intel.com>
> ---
> arch/ia64/uv/kernel/setup.c | 2 +
> include/asm-ia64/uv/uv_hub.h | 61 ++++++++++++++++++++++++++++++++++++++++++-
It's arch/ia64/include/asm/uv/uv_hub.h in my kernel. Weird.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/6] x86_64 UV: Use LED to indicate CPU is active
2008-08-08 0:56 ` [PATCH 2/6] x86_64 UV: Use LED to indicate CPU is active Mike Travis
@ 2008-08-11 15:53 ` Ingo Molnar
2008-08-13 9:56 ` Pavel Machek
2008-08-25 17:49 ` Mike Travis
0 siblings, 2 replies; 17+ messages in thread
From: Ingo Molnar @ 2008-08-11 15:53 UTC (permalink / raw)
To: Mike Travis
Cc: Andrew Morton, Jack Steiner, linux-kernel, H. Peter Anvin,
Thomas Gleixner
* Mike Travis <travis@sgi.com> wrote:
> +/*
> + * Illuminate "activity" LED when CPU is going "active",
> + * extinguish when going "idle".
> + */
> +static int uv_idle(struct notifier_block *nfb, unsigned long action, void *junk)
> +{
> + if (action == IDLE_START)
> + uv_set_led_bits(0, LED_CPU_ACTIVITY);
> +
> + else if (action == IDLE_END)
> + uv_set_led_bits(LED_CPU_ACTIVITY, LED_CPU_ACTIVITY);
> +
> + return NOTIFY_OK;
> +}
> +
> +static struct notifier_block uv_idle_notifier = {
> + .notifier_call = uv_idle,
> +};
>
> +/*
> + * Initialize idle led callback function
> + */
> +static __init void uv_init_led_idle_display(void)
> +{
> + /* initialize timer for activity monitor */
> + idle_notifier_register(&uv_idle_notifier);
> +}
hm, i think this is a bad idea. While putting it into the go-to-idle
codepath probably doesnt matter, putting anything into the idle wakeup
path increases latency of a rather critical codepath. The MMR write that
the LED driver is using will go out to the local bus, which, even if
it's POST-ed, if it's done frequently enough will be the cost of an IO
cycle.
No human needs to know the LED status at _that_ frequency anyway, so
it's quite pointless as well.
A much better (and faster) approach would be to sample the utilization
of the CPU and indicate that via the LED(s).
Ingo
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/6] x86_64 UV: Use blinking LED for heartbeat display
2008-08-08 0:56 ` [PATCH 3/6] x86_64 UV: Use blinking LED for heartbeat display Mike Travis
@ 2008-08-11 16:02 ` Ingo Molnar
2008-08-13 9:57 ` Pavel Machek
2008-08-25 17:55 ` Mike Travis
0 siblings, 2 replies; 17+ messages in thread
From: Ingo Molnar @ 2008-08-11 16:02 UTC (permalink / raw)
To: Mike Travis
Cc: Andrew Morton, Jack Steiner, linux-kernel, H. Peter Anvin,
Thomas Gleixner
* 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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/6] x86_64 UV: Use LED to indicate CPU is active
2008-08-11 15:53 ` Ingo Molnar
@ 2008-08-13 9:56 ` Pavel Machek
2008-08-25 17:53 ` Mike Travis
2008-08-25 17:49 ` Mike Travis
1 sibling, 1 reply; 17+ messages in thread
From: Pavel Machek @ 2008-08-13 9:56 UTC (permalink / raw)
To: Ingo Molnar
Cc: Mike Travis, Andrew Morton, Jack Steiner, linux-kernel,
H. Peter Anvin, Thomas Gleixner
Hi!
> > +/*
> > + * Illuminate "activity" LED when CPU is going "active",
> > + * extinguish when going "idle".
> > + */
> > +static int uv_idle(struct notifier_block *nfb, unsigned long action, void *junk)
> > +{
> > + if (action == IDLE_START)
> > + uv_set_led_bits(0, LED_CPU_ACTIVITY);
> > +
> > + else if (action == IDLE_END)
> > + uv_set_led_bits(LED_CPU_ACTIVITY, LED_CPU_ACTIVITY);
> > +
> > + return NOTIFY_OK;
> > +}
> > +
> > +static struct notifier_block uv_idle_notifier = {
> > + .notifier_call = uv_idle,
> > +};
> >
> > +/*
> > + * Initialize idle led callback function
> > + */
> > +static __init void uv_init_led_idle_display(void)
> > +{
> > + /* initialize timer for activity monitor */
> > + idle_notifier_register(&uv_idle_notifier);
> > +}
>
> hm, i think this is a bad idea. While putting it into the go-to-idle
Plus this really should not be uv-specific. We already have generic
triggers.
And you should really CC led maintainers!
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/6] x86_64 UV: Use blinking LED for heartbeat display
2008-08-11 16:02 ` Ingo Molnar
@ 2008-08-13 9:57 ` Pavel Machek
2008-08-26 20:53 ` Mike Travis
2008-08-25 17:55 ` Mike Travis
1 sibling, 1 reply; 17+ messages in thread
From: Pavel Machek @ 2008-08-13 9:57 UTC (permalink / raw)
To: Ingo Molnar
Cc: Mike Travis, Andrew Morton, Jack Steiner, linux-kernel,
H. Peter Anvin, Thomas Gleixner
>
> * 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.
LED subsystem already has nice heartbeat trigger.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/6] x86_64 UV: Use LED to indicate CPU is active
2008-08-11 15:53 ` Ingo Molnar
2008-08-13 9:56 ` Pavel Machek
@ 2008-08-25 17:49 ` Mike Travis
1 sibling, 0 replies; 17+ messages in thread
From: Mike Travis @ 2008-08-25 17:49 UTC (permalink / raw)
To: Ingo Molnar
Cc: Andrew Morton, Jack Steiner, linux-kernel, H. Peter Anvin,
Thomas Gleixner
Ingo Molnar wrote:
> * Mike Travis <travis@sgi.com> wrote:
>
>> +/*
>> + * Illuminate "activity" LED when CPU is going "active",
>> + * extinguish when going "idle".
>> + */
>> +static int uv_idle(struct notifier_block *nfb, unsigned long action, void *junk)
>> +{
>> + if (action == IDLE_START)
>> + uv_set_led_bits(0, LED_CPU_ACTIVITY);
>> +
>> + else if (action == IDLE_END)
>> + uv_set_led_bits(LED_CPU_ACTIVITY, LED_CPU_ACTIVITY);
>> +
>> + return NOTIFY_OK;
>> +}
>> +
>> +static struct notifier_block uv_idle_notifier = {
>> + .notifier_call = uv_idle,
>> +};
>>
>> +/*
>> + * Initialize idle led callback function
>> + */
>> +static __init void uv_init_led_idle_display(void)
>> +{
>> + /* initialize timer for activity monitor */
>> + idle_notifier_register(&uv_idle_notifier);
>> +}
>
> hm, i think this is a bad idea. While putting it into the go-to-idle
> codepath probably doesnt matter, putting anything into the idle wakeup
> path increases latency of a rather critical codepath. The MMR write that
> the LED driver is using will go out to the local bus, which, even if
> it's POST-ed, if it's done frequently enough will be the cost of an IO
> cycle.
>
> No human needs to know the LED status at _that_ frequency anyway, so
> it's quite pointless as well.
>
> A much better (and faster) approach would be to sample the utilization
> of the CPU and indicate that via the LED(s).
>
> Ingo
Hi Ingo,
I'll take a closer look at this. The actual usage is more in line with
an SNMP type system controller that has direct access to the BIOS registers
including the "LED" register and is used mostly for diagnosing system
problems. The other research I need to do is how fast/slow is this path.
It may be a simple memory write in our UV NUMA hub chip that's not directed
to the general I/O bus and hence, negligible overhead.
Looking at the IA64 code, it has a similar interface which uses a function
pointer variable to set/reset "idle" state. I just wish x86_64 had a similar
common interface for the clock handler that would occur per cpu and not per
group of cpus that clocksource_watchdog() currently uses.
The other architectural change I would like to make is to expose the necessary
data/address via the pda (or something similar) and allow the complete write
to be inlined to one or two instructions.
Thanks,
Mike
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/6] x86_64 UV: Use LED to indicate CPU is active
2008-08-13 9:56 ` Pavel Machek
@ 2008-08-25 17:53 ` Mike Travis
0 siblings, 0 replies; 17+ messages in thread
From: Mike Travis @ 2008-08-25 17:53 UTC (permalink / raw)
To: Pavel Machek
Cc: Ingo Molnar, Andrew Morton, Jack Steiner, linux-kernel,
H. Peter Anvin, Thomas Gleixner
Pavel Machek wrote:
> Hi!
>
>>> +/*
>>> + * Illuminate "activity" LED when CPU is going "active",
>>> + * extinguish when going "idle".
>>> + */
>>> +static int uv_idle(struct notifier_block *nfb, unsigned long action, void *junk)
>>> +{
>>> + if (action == IDLE_START)
>>> + uv_set_led_bits(0, LED_CPU_ACTIVITY);
>>> +
>>> + else if (action == IDLE_END)
>>> + uv_set_led_bits(LED_CPU_ACTIVITY, LED_CPU_ACTIVITY);
>>> +
>>> + return NOTIFY_OK;
>>> +}
>>> +
>>> +static struct notifier_block uv_idle_notifier = {
>>> + .notifier_call = uv_idle,
>>> +};
>>>
>>> +/*
>>> + * Initialize idle led callback function
>>> + */
>>> +static __init void uv_init_led_idle_display(void)
>>> +{
>>> + /* initialize timer for activity monitor */
>>> + idle_notifier_register(&uv_idle_notifier);
>>> +}
>> hm, i think this is a bad idea. While putting it into the go-to-idle
>
> Plus this really should not be uv-specific. We already have generic
> triggers.
>
> And you should really CC led maintainers!
> Pavel
Thanks for the pointers, I will definitely look into this.
(And sorry for not discovering the LED docs and maintainers earlier.)
Mike
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/6] x86_64 UV: Use blinking LED for heartbeat display
2008-08-11 16:02 ` Ingo Molnar
2008-08-13 9:57 ` Pavel Machek
@ 2008-08-25 17:55 ` Mike Travis
1 sibling, 0 replies; 17+ messages in thread
From: Mike Travis @ 2008-08-25 17:55 UTC (permalink / raw)
To: Ingo Molnar
Cc: Andrew Morton, Jack Steiner, linux-kernel, H. Peter Anvin,
Thomas Gleixner
Ingo Molnar wrote:
> * 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
Yes, I agree 100%. There was a trade off with various approaches but I
was hoping for some feedback on alternate approaches (and thanks for that!)
Mike
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/6] x86_64 UV: Use blinking LED for heartbeat display
2008-08-13 9:57 ` Pavel Machek
@ 2008-08-26 20:53 ` Mike Travis
2008-08-27 21:38 ` Pavel Machek
0 siblings, 1 reply; 17+ messages in thread
From: Mike Travis @ 2008-08-26 20:53 UTC (permalink / raw)
To: Pavel Machek
Cc: Ingo Molnar, Andrew Morton, Jack Steiner, linux-kernel,
H. Peter Anvin, Thomas Gleixner
Pavel Machek wrote:
>> * 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.
>
> LED subsystem already has nice heartbeat trigger.
> Pavel
>From Documentation/leds-class.txt:
Future Development
==================
At the moment, a trigger can't be created specifically for a single LED.
There are a number of cases where a trigger might only be mappable to a
particular LED (ACPI?). The addition of triggers provided by the LED driver
should cover this option and be possible to add without breaking the
current interface.
--
The SGI system has a set of leds per cpu, and the goal is that the leds
display heartbeat and "idle-ness" information specific to that cpu. At
first glance I don't see this capability in the LED subsystem. Am I
missing something?
Additionally, the 8 leds are written as one byte with each led being
full on or full off. It seems to be a big overhead to support the led
class since I'd have to kluge together some means of sharing the current
led register value [we really don't want to have to read the current reg
value before updating a single led.]
But I will definitely look at the led_heartbeat_function more closely.
Any advice gladly welcomed!
Thanks!
Mike
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/6] x86_64 UV: Use blinking LED for heartbeat display
2008-08-26 20:53 ` Mike Travis
@ 2008-08-27 21:38 ` Pavel Machek
0 siblings, 0 replies; 17+ messages in thread
From: Pavel Machek @ 2008-08-27 21:38 UTC (permalink / raw)
To: Mike Travis
Cc: Ingo Molnar, Andrew Morton, Jack Steiner, linux-kernel,
H. Peter Anvin, Thomas Gleixner
On Tue 2008-08-26 13:53:48, Mike Travis wrote:
> Pavel Machek wrote:
> >> * 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.
> >
> > LED subsystem already has nice heartbeat trigger.
> > Pavel
>
> From Documentation/leds-class.txt:
> Future Development
> ==================
>
> At the moment, a trigger can't be created specifically for a single LED.
> There are a number of cases where a trigger might only be mappable to a
> particular LED (ACPI?). The addition of triggers provided by the LED driver
> should cover this option and be possible to add without breaking the
> current interface.
I don't think this affects you.
> --
> The SGI system has a set of leds per cpu, and the goal is that the leds
> display heartbeat and "idle-ness" information specific to that cpu. At
> first glance I don't see this capability in the LED subsystem. Am I
> missing something?
Set the heartbeat trigger to the led you care about. Should be doable
with simple echo to /sys after you implement your leds properly.
> Additionally, the 8 leds are written as one byte with each led being
> full on or full off. It seems to be a big overhead to support the led
> class since I'd have to kluge together some means of sharing the current
> led register value [we really don't want to have to read the current reg
> value before updating a single led.]
Big overhead? Just use shadow register of led state in ram.
> But I will definitely look at the led_heartbeat_function more closely.
>
> Any advice gladly welcomed!
You still did not cc LED developers.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2008-08-27 21:37 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-08-08 0:56 [PATCH 0/6] SGI UV: Provide a LED driver and some System Activity Indicators Mike Travis
2008-08-08 0:56 ` [PATCH 1/6] x86_64 UV: Provide a LED driver for UV Systems Mike Travis
2008-08-08 0:56 ` [PATCH 2/6] x86_64 UV: Use LED to indicate CPU is active Mike Travis
2008-08-11 15:53 ` Ingo Molnar
2008-08-13 9:56 ` Pavel Machek
2008-08-25 17:53 ` Mike Travis
2008-08-25 17:49 ` Mike Travis
2008-08-08 0:56 ` [PATCH 3/6] x86_64 UV: Use blinking LED for heartbeat display Mike Travis
2008-08-11 16:02 ` Ingo Molnar
2008-08-13 9:57 ` Pavel Machek
2008-08-26 20:53 ` Mike Travis
2008-08-27 21:38 ` Pavel Machek
2008-08-25 17:55 ` Mike Travis
2008-08-08 0:56 ` [PATCH 4/6] ia64 UV: Provide a LED driver for UV Systems Mike Travis
2008-08-08 6:07 ` Andrew Morton
2008-08-08 0:56 ` [PATCH 5/6] ia64 UV: Use LED to indicate CPU is active Mike Travis
2008-08-08 0:56 ` [PATCH 6/6] ia64 UV: Use blinking LED for heartbeat display Mike Travis
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox