Skip to contentSkip to conversation

more robust halt_light

PATCH In review Alex Williamson 5 participants 12 messages Mar 3, 2004 – Mar 4, 2004

lore
Share & tools
lore

Alex Williamson

author
#0 lore raw

PATCH 1/1 more robust halt_light

This patch adds some i386-ish enable/disable features to the
pal_halt_light cpu idle implementation as well as tries to avoid bad interactions with certain revs of PAL on McKinley cpus. Hopefully this will provide enough flexibility that we can leave it configured on in the kernel by default. My latest measurements on a 1.3GHz rx2600 show that enabling pal_halt_light in the cpu_idle routine saves 23W/cpu on an idle system.

FWIW, I added the enable/disable_hlt routines. I don't see any
consumers of this for ia64 now, but it looked useful. Patch against latest linux-2.5 bk. Thanks,

Alex

Signature · 136 lines
Alex Williamson                             HP Linux & Open Source Lab

=== arch/ia64/kernel/process.c 1.51 vs edited ==--- 1.51/arch/ia64/kernel/process.c	Sat Feb 28 03:02:47 2004
+++ edited/arch/ia64/kernel/process.c	Wed Mar  3 09:12:44 2004
@@ -39,6 +39,8 @@
 
 #include "sigframe.h"
 
+int hlt_counter;
+
 void (*ia64_mark_idle)(int);
 
 
@@ -159,6 +161,20 @@
 		ia64_do_signal(oldset, scr, in_syscall);
 }
 
+void disable_hlt(void)
+{
+	hlt_counter++;
+}
+
+EXPORT_SYMBOL(disable_hlt);
+
+void enable_hlt(void)
+{
+	hlt_counter--;
+}
+
+EXPORT_SYMBOL(enable_hlt);
+
 /*
  * We use this if we don't have any better idle routine..
  */
@@ -166,7 +182,7 @@
 default_idle (void)
 {
 #ifdef CONFIG_IA64_PAL_IDLE
-	if (!need_resched())
+	if (!hlt_counter && local_cpu_data->hlt_works_ok && !need_resched())
 		safe_halt();
 #endif
 }
=== arch/ia64/kernel/setup.c 1.68 vs edited ==--- 1.68/arch/ia64/kernel/setup.c	Mon Jan 19 16:38:10 2004
+++ edited/arch/ia64/kernel/setup.c	Wed Mar  3 09:12:45 2004
@@ -76,6 +76,10 @@
 
 unsigned char aux_device_present = 0xaa;        /* XXX remove this when legacy I/O is gone */
 
+#ifdef CONFIG_IA64_PAL_IDLE
+static unsigned char nohlt;
+#endif
+
 /*
  * The merge_mask variable needs to be set to (max(iommu_page_size(iommu)) - 1).  This
  * mask specifies a mask of address bits that must be 0 in order for two buffers to be
@@ -429,13 +433,20 @@
 		   "revision   : %u\n"
 		   "archrev    : %u\n"
 		   "features   :%s\n"	/* don't change this---it _is_ right! */
+#ifdef CONFIG_IA64_PAL_IDLE
+		   "halt works : %s\n"
+#endif
 		   "cpu number : %lu\n"
 		   "cpu regs   : %u\n"
 		   "cpu MHz    : %lu.%06lu\n"
 		   "itc MHz    : %lu.%06lu\n"
 		   "BogoMIPS   : %lu.%02lu\n\n",
 		   cpunum, c->vendor, family, c->model, c->revision, c->archrev,
-		   features, c->ppn, c->number,
+		   features,
+#ifdef CONFIG_IA64_PAL_IDLE
+		   c->hlt_works_ok ? "yes" : "no",
+#endif
+		   c->ppn, c->number,
 		   c->proc_freq / 1000000, c->proc_freq % 1000000,
 		   c->itc_freq / 1000000, c->itc_freq % 1000000,
 		   lpj*HZ/500000, (lpj*HZ/5000) % 100);
@@ -523,6 +534,30 @@
 	}
 	c->unimpl_va_mask = ~((7L<<61) | ((1L << (impl_va_msb + 1)) - 1));
 	c->unimpl_pa_mask = ~((1L<<63) | ((1L << phys_addr_size) - 1));
+
+#ifdef CONFIG_IA64_PAL_IDLE
+	if (!nohlt) {
+
+		c->hlt_works_ok = 1;
+
+#ifdef CONFIG_PERFMON
+		/*
+		 * McKinley has a PAL that in some revisions that doesn't behave well with
+		 * the PMU.  The belief is PAL >= 7.64 will fix the problem.
+		 */
+		if (c->family = 0x1f && c->model = 0) {
+			pal_version_u_t min_ver, cur_ver;
+
+			if (ia64_pal_version(&min_ver, &cur_ver) = 0) {
+
+				if (cur_ver.pal_version_s.pv_pal_b_model <= 0x7 &&
+				    cur_ver.pal_version_s.pv_pal_b_rev < 0x64)
+					c->hlt_works_ok = 0;
+			}
+		}
+#endif
+	}
+#endif
 }
 
 void
@@ -674,3 +709,14 @@
 	ia64_patch_mckinley_e9((unsigned long) __start___mckinley_e9_bundles,
 			       (unsigned long) __end___mckinley_e9_bundles);
 }
+
+#ifdef CONFIG_IA64_PAL_IDLE
+static int __init no_halt(char *s)
+{
+	local_cpu_data->hlt_works_ok = 0;
+	nohlt = 1;
+	return 1;
+}
+
+__setup("no-hlt", no_halt);
+#endif
=== include/asm-ia64/processor.h 1.56 vs edited ==--- 1.56/include/asm-ia64/processor.h	Wed Feb 25 15:46:40 2004
+++ edited/include/asm-ia64/processor.h	Wed Mar  3 09:12:46 2004
@@ -180,6 +180,9 @@
 #ifdef CONFIG_NUMA
 	struct ia64_node_data *node_data;
 #endif
+#ifdef CONFIG_IA64_PAL_IDLE
+	__u8 hlt_works_ok;
+#endif
 };
 
 DECLARE_PER_CPU(struct cpuinfo_ia64, cpu_info);
#1 lore raw

The issue with performance counters is only when PAL_HALT_LIGHT is enabled. It is nice to be able to use this power save feature when the performance counters are not currently/actively used system wide. Can we use the disable_hlt and enable_hlt APIs (mentioned in the patch below) in perfmon kind of tools to disable/enable PAL_HALT_LIGHT whenever the system is transitioning in and out of system wide performance monitoring. That way we don't have to have any tests for Mckinley, PAL versions etc. etc.

rohit

Quoted 8 lines

-----Original Message-----
From: [email protected] [mailto:linux-ia64- [email protected]] On Behalf Of Alex Williamson
Sent: Wednesday, March 03, 2004 9:57 AM
To: [email protected]
Subject: [PATCH] more robust halt_light

Quoted 6 lines

This patch adds some i386-ish enable/disable features to the
pal_halt_light cpu idle implementation as well as tries to avoid bad interactions with certain revs of PAL on McKinley cpus. Hopefully this will provide enough flexibility that we can leave it configured on in the kernel by default. My latest measurements on a 1.3GHz rx2600 show that enabling pal_halt_light in the cpu_idle routine saves 23W/cpu on

an

Quoted 44 lines

idle system.

FWIW, I added the enable/disable_hlt routines. I don't see any
consumers of this for ia64 now, but it looked useful. Patch against latest linux-2.5 bk. Thanks,

Alex

--
Alex Williamson HP Linux & Open Source Lab

=== arch/ia64/kernel/process.c 1.51 vs edited ==>--- 1.51/arch/ia64/kernel/process.c Sat Feb 28 03:02:47 2004
+++ edited/arch/ia64/kernel/process.c Wed Mar 3 09:12:44 2004

(quoted diff) +16 -0
@@ -39,6 +39,8 @@
 
 #include "sigframe.h"
 
+int hlt_counter;
+
 void (*ia64_mark_idle)(int);
 
 

@@ -159,6 +161,20 @@
 		ia64_do_signal(oldset, scr, in_syscall);
 }
 
+void disable_hlt(void)
+{
+	hlt_counter++;
+}
+
+EXPORT_SYMBOL(disable_hlt);
+
+void enable_hlt(void)
+{
+	hlt_counter--;
+}
+
+EXPORT_SYMBOL(enable_hlt);
+
 /*
 * We use this if we don't have any better idle routine..
 */

Review line 185

Quoted 5 lines
(quoted diff) +1 -1
@@ -166,7 +182,7 @@
 default_idle (void)
 {
 #ifdef CONFIG_IA64_PAL_IDLE
-	if (!need_resched())
+	if (!hlt_counter && local_cpu_data->hlt_works_ok &&

!need_resched())

Quoted 8 lines
(quoted diff) +0 -0
 		safe_halt();
 #endif
 }

=== arch/ia64/kernel/setup.c 1.68 vs edited ==>--- 1.68/arch/ia64/kernel/setup.c Mon Jan 19 16:38:10 2004
+++ edited/arch/ia64/kernel/setup.c Wed Mar 3 09:12:45 2004

(quoted diff) +0 -0
@@ -76,6 +76,10 @@

unsigned char aux_device_present = 0xaa; /* XXX remove this

when

Quoted 8 lines

legacy I/O is gone */

(quoted diff) +4 -0
+#ifdef CONFIG_IA64_PAL_IDLE
+static unsigned char nohlt;
+#endif
+

/*
* The merge_mask variable needs to be set to

(max(iommu_page_size(iommu))

Quoted 2 lines

- 1). This
* mask specifies a mask of address bits that must be 0 in order for

two

Review line 435

Quoted 5 lines

buffers to be

(quoted diff) +0 -0
@@ -429,13 +433,20 @@
 		   "revision   : %u\n"
 		   "archrev    : %u\n"
 		   "features   :%s\n"	/* don't change this---it _is_

right!

Quoted 21 lines
(quoted diff) +8 -1
 */
+#ifdef CONFIG_IA64_PAL_IDLE
+		   "halt works : %s\n"
+#endif
 		   "cpu number : %lu\n"
 		   "cpu regs   : %u\n"
 		   "cpu MHz    : %lu.%06lu\n"
 		   "itc MHz    : %lu.%06lu\n"
 		   "BogoMIPS   : %lu.%02lu\n\n",
 		   cpunum, c->vendor, family, c->model, c->revision, c-
 >archrev,
-		   features, c->ppn, c->number,
+		   features,
+#ifdef CONFIG_IA64_PAL_IDLE
+		   c->hlt_works_ok ? "yes" : "no",
+#endif
+		   c->ppn, c->number,
 		   c->proc_freq / 1000000, c->proc_freq % 1000000,

		   c->itc_freq / 1000000, c->itc_freq % 1000000,
		   lpj*HZ/500000, (lpj*HZ/5000) % 100);

Review line 535

Quoted 2 lines
(quoted diff) +0 -0
@@ -523,6 +534,30 @@
 	}
 	c->unimpl_va_mask = ~((7L<<61) | ((1L << (impl_va_msb + 1)) -

1));

Review line 545

Quoted 10 lines
(quoted diff) +9 -0
 	c->unimpl_pa_mask = ~((1L<<63) | ((1L << phys_addr_size) - 1));
+
+#ifdef CONFIG_IA64_PAL_IDLE
+	if (!nohlt) {
+
+		c->hlt_works_ok = 1;
+
+#ifdef CONFIG_PERFMON
+		/*
+		 * McKinley has a PAL that in some revisions that

doesn't

Review

Quoted 2 lines

behave well with

(quoted diff) +1 -0
+		 * the PMU.  The belief is PAL >= 7.64 will fix the

problem.

Review

Quoted 7 lines
(quoted diff) +7 -0
+		 */
+		if (c->family = 0x1f && c->model = 0) {
+			pal_version_u_t min_ver, cur_ver;
+
+			if (ia64_pal_version(&min_ver, &cur_ver) = 0) {
+
+				if (cur_ver.pal_version_s.pv_pal_b_model

<= 0x7 &&

Review

Quoted 1 line
(quoted diff) +1 -0
+				    cur_ver.pal_version_s.pv_pal_b_rev <

0x64)

Review line 709

Quoted 11 lines
(quoted diff) +6 -0
+					c->hlt_works_ok = 0;
+			}
+		}
+#endif
+	}
+#endif

}

void

(quoted diff) +0 -0
@@ -674,3 +709,14 @@
 	ia64_patch_mckinley_e9((unsigned long)

__start___mckinley_e9_bundles,

Review line 710

Quoted 1 line
(quoted diff) +0 -0
 			       (unsigned long)

__end___mckinley_e9_bundles);

Quoted 27 lines
(quoted diff) +11 -0
 }
+
+#ifdef CONFIG_IA64_PAL_IDLE
+static int __init no_halt(char *s)
+{
+	local_cpu_data->hlt_works_ok = 0;
+	nohlt = 1;
+	return 1;
+}
+
+__setup("no-hlt", no_halt);
+#endif

=== include/asm-ia64/processor.h 1.56 vs edited ==>--- 1.56/include/asm-ia64/processor.h Wed Feb 25 15:46:40 2004
+++ edited/include/asm-ia64/processor.h Wed Mar 3 09:12:46 2004

(quoted diff) +3 -1
@@ -180,6 +180,9 @@
 #ifdef CONFIG_NUMA
 	struct ia64_node_data *node_data;
 #endif
+#ifdef CONFIG_IA64_PAL_IDLE
+	__u8 hlt_works_ok;
+#endif
 };
 
 DECLARE_PER_CPU(struct cpuinfo_ia64, cpu_info);
 
 
-
Quoted 1 line

To unsubscribe from this list: send the line "unsubscribe linux-ia64"

in

Quoted 2 lines

the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#2 lore raw
In reply to Alex Williamson

I was curious about that myself, but I thought I remembered from
Stephane that the perf counters didn't really recover from a PAL_HALT_LIGHT on McKinley. If that's not the case, maybe we can move the PAL check into perfmon code, and it can selectively disable it as it needs via the disable_hlt API. I assume there are some cases were perfmon wouldn't need to disable halt_light on working PAL, so the rev check might not go away.

Alex

Quoted 31 lines · Rohit

The issue with performance counters is only when PAL_HALT_LIGHT is enabled. It is nice to be able to use this power save feature when the performance counters are not currently/actively used system wide. Can we use the disable_hlt and enable_hlt APIs (mentioned in the patch below) in perfmon kind of tools to disable/enable PAL_HALT_LIGHT whenever the system is transitioning in and out of system wide performance monitoring. That way we don't have to have any tests for Mckinley, PAL versions etc. etc.

rohit

Quoted 6 lines

-----Original Message-----
Sent: Wednesday, March 03, 2004 9:57 AM
To: [email protected]
Subject: [PATCH] more robust halt_light

Quoted 6 lines

This patch adds some i386-ish enable/disable features to the
pal_halt_light cpu idle implementation as well as tries to avoid bad interactions with certain revs of PAL on McKinley cpus. Hopefully this will provide enough flexibility that we can leave it configured on in the kernel by default. My latest measurements on a 1.3GHz rx2600 show that enabling pal_halt_light in the cpu_idle routine saves 23W/cpu on

an

Quoted 8 lines

idle system.

FWIW, I added the enable/disable_hlt routines. I don't see any
consumers of this for ia64 now, but it looked useful. Patch against latest linux-2.5 bk. Thanks,

Alex

Signature · 1 line
Alex Williamson                             HP Linux & Open Source Lab
author
#3 lore raw
In reply to Alex Williamson

Bjorn pointed out that the existing enable/disable_hlt routines that
I copied from the other archs are rather racey. Here's a new version that uses atomic ops to avoid that. Original patch description below.

This patch adds some i386-ish enable/disable features to the
pal_halt_light cpu idle implementation as well as tries to avoid bad interactions with certain revs of PAL on McKinley cpus. Hopefully this will provide enough flexibility that we can leave it configured on in the kernel by default. My latest measurements on a 1.3GHz rx2600 show that enabling pal_halt_light in the cpu_idle routine saves 23W/cpu on an idle system.

Thanks,

Alex

Signature · 137 lines
Alex Williamson                             HP Linux & Open Source Lab

=== arch/ia64/kernel/process.c 1.51 vs edited ==--- 1.51/arch/ia64/kernel/process.c	Sat Feb 28 03:02:47 2004
+++ edited/arch/ia64/kernel/process.c	Wed Mar  3 14:58:24 2004
@@ -39,6 +39,8 @@
 
 #include "sigframe.h"
 
+static atomic_t hlt_counter = ATOMIC_INIT(0);
+
 void (*ia64_mark_idle)(int);
 
 
@@ -159,6 +161,20 @@
 		ia64_do_signal(oldset, scr, in_syscall);
 }
 
+void disable_hlt(void)
+{
+	atomic_inc(&hlt_counter);
+}
+
+EXPORT_SYMBOL(disable_hlt);
+
+void enable_hlt(void)
+{
+	atomic_dec(&hlt_counter);
+}
+
+EXPORT_SYMBOL(enable_hlt);
+
 /*
  * We use this if we don't have any better idle routine..
  */
@@ -166,7 +182,7 @@
 default_idle (void)
 {
 #ifdef CONFIG_IA64_PAL_IDLE
-	if (!need_resched())
+	if (!atomic_read(&hlt_counter) && local_cpu_data->hlt_works_ok && !need_resched())
 		safe_halt();
 #endif
 }
=== arch/ia64/kernel/setup.c 1.68 vs edited ==--- 1.68/arch/ia64/kernel/setup.c	Mon Jan 19 16:38:10 2004
+++ edited/arch/ia64/kernel/setup.c	Wed Mar  3 15:09:39 2004
@@ -76,6 +76,10 @@
 
 unsigned char aux_device_present = 0xaa;        /* XXX remove this when legacy I/O is gone */
 
+#ifdef CONFIG_IA64_PAL_IDLE
+static unsigned char nohlt;
+#endif
+
 /*
  * The merge_mask variable needs to be set to (max(iommu_page_size(iommu)) - 1).  This
  * mask specifies a mask of address bits that must be 0 in order for two buffers to be
@@ -429,13 +433,20 @@
 		   "revision   : %u\n"
 		   "archrev    : %u\n"
 		   "features   :%s\n"	/* don't change this---it _is_ right! */
+#ifdef CONFIG_IA64_PAL_IDLE
+		   "halt works : %s\n"
+#endif
 		   "cpu number : %lu\n"
 		   "cpu regs   : %u\n"
 		   "cpu MHz    : %lu.%06lu\n"
 		   "itc MHz    : %lu.%06lu\n"
 		   "BogoMIPS   : %lu.%02lu\n\n",
 		   cpunum, c->vendor, family, c->model, c->revision, c->archrev,
-		   features, c->ppn, c->number,
+		   features,
+#ifdef CONFIG_IA64_PAL_IDLE
+		   c->hlt_works_ok ? "yes" : "no",
+#endif
+		   c->ppn, c->number,
 		   c->proc_freq / 1000000, c->proc_freq % 1000000,
 		   c->itc_freq / 1000000, c->itc_freq % 1000000,
 		   lpj*HZ/500000, (lpj*HZ/5000) % 100);
@@ -523,6 +534,31 @@
 	}
 	c->unimpl_va_mask = ~((7L<<61) | ((1L << (impl_va_msb + 1)) - 1));
 	c->unimpl_pa_mask = ~((1L<<63) | ((1L << phys_addr_size) - 1));
+
+#ifdef CONFIG_IA64_PAL_IDLE
+	if (!nohlt) {
+
+		c->hlt_works_ok = 1;
+
+#ifdef CONFIG_PERFMON
+		/*
+		 * McKinley has a PAL that in some revisions that doesn't
+		 * behave well with the PMU.  The belief is PAL >= 7.64 will
+		 * fix the problem.
+		 */
+		if (c->family = 0x1f && c->model = 0) {
+			pal_version_u_t min_ver, cur_ver;
+
+			if (ia64_pal_version(&min_ver, &cur_ver) = 0) {
+
+				if (cur_ver.pal_version_s.pv_pal_b_model <= 0x7 &&
+				    cur_ver.pal_version_s.pv_pal_b_rev < 0x64)
+					c->hlt_works_ok = 0;
+			}
+		}
+#endif
+	}
+#endif
 }
 
 void
@@ -674,3 +710,14 @@
 	ia64_patch_mckinley_e9((unsigned long) __start___mckinley_e9_bundles,
 			       (unsigned long) __end___mckinley_e9_bundles);
 }
+
+#ifdef CONFIG_IA64_PAL_IDLE
+static int __init no_halt(char *s)
+{
+	local_cpu_data->hlt_works_ok = 0;
+	nohlt = 1;
+	return 1;
+}
+
+__setup("no-hlt", no_halt);
+#endif
=== include/asm-ia64/processor.h 1.56 vs edited ==--- 1.56/include/asm-ia64/processor.h	Wed Feb 25 15:46:40 2004
+++ edited/include/asm-ia64/processor.h	Wed Mar  3 09:12:46 2004
@@ -180,6 +180,9 @@
 #ifdef CONFIG_NUMA
 	struct ia64_node_data *node_data;
 #endif
+#ifdef CONFIG_IA64_PAL_IDLE
+	__u8 hlt_works_ok;
+#endif
 };
 
 DECLARE_PER_CPU(struct cpuinfo_ia64, cpu_info);
#4 lore raw
In reply to Alex Williamson
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line · Alex Williamson

Alex> I was curious about that myself, but I thought I remembered Alex> from Stephane that the perf counters didn't really recover Alex> from a PAL_HALT_LIGHT on McKinley. If that's not the case, Alex> maybe we can move the PAL check into perfmon code, and it can Alex> selectively disable it as it needs via the disable_hlt API. I Alex> assume there are some cases were perfmon wouldn't need to Alex> disable halt_light on working PAL, so the rev check might not Alex> go away.

Can we get rid of the CONFIG option too? Or do folks who care (too?) much about wake-up latency prefer to turn off a CONFIG option over booting with nohlt?

	--david
#5 lore raw
In reply to Alex Williamson
Quoted 3 lines · David Mosberger

Can we get rid of the CONFIG option too? Or do folks who care (too?) much about wake-up latency prefer to turn off a CONFIG option over booting with nohlt?

Some people have come to me concerned about the wakeup latency of having the halt call in there, and it also looks like the PAL has a bug on our platform that causes hangs when we call PAL_HALT_LIGHT (which we're tracking down), so I wouldn't mind if the config option stuck around a little longer (or was a boot time parameter at least).

Thanks,
Jesse

#6 lore raw
In reply to Alex Williamson
Quoted 4 lines · Alex Williamson

I was curious about that myself, but I thought I remembered from
Stephane that the perf counters didn't really recover from a PAL_HALT_LIGHT on McKinley.

The way I recall Stephane explaining it to me was the PMU was completely disabled after returning from PAL_HALT_LIGHT. ISTR no PAL fix was never released for Mckinley. And even if one was, we'd have to convince HP firmware teams to roll and qualify it. Unlikely.

Quoted 3 lines

If that's not the case, maybe we can move
the PAL check into perfmon code, and it can selectively disable it as it needs via the disable_hlt API.

I don't think that's likely...but poke stephane again when he's back. But I do like rohit's idea that perfmon enable/disable PAL_HALT when it starts/stops using the PMU.
grant

Quoted 46 lines

I assume there are some cases were
perfmon wouldn't need to disable halt_light on working PAL, so the rev check might not go away.

Alex

Quoted 31 lines · Rohit

The issue with performance counters is only when PAL_HALT_LIGHT is enabled. It is nice to be able to use this power save feature when the performance counters are not currently/actively used system wide. Can we use the disable_hlt and enable_hlt APIs (mentioned in the patch below) in perfmon kind of tools to disable/enable PAL_HALT_LIGHT whenever the system is transitioning in and out of system wide performance monitoring. That way we don't have to have any tests for Mckinley, PAL versions etc. etc.

rohit

Quoted 6 lines

-----Original Message-----
Sent: Wednesday, March 03, 2004 9:57 AM
To: [email protected]
Subject: [PATCH] more robust halt_light

Quoted 6 lines

This patch adds some i386-ish enable/disable features to the
pal_halt_light cpu idle implementation as well as tries to avoid bad interactions with certain revs of PAL on McKinley cpus. Hopefully this will provide enough flexibility that we can leave it configured on in the kernel by default. My latest measurements on a 1.3GHz rx2600 show that enabling pal_halt_light in the cpu_idle routine saves 23W/cpu on

an

Quoted 8 lines

idle system.

FWIW, I added the enable/disable_hlt routines. I don't see any
consumers of this for ia64 now, but it looked useful. Patch against latest linux-2.5 bk. Thanks,

Alex

Signature · 6 lines
Alex Williamson                             HP Linux & Open Source Lab

-
To unsubscribe from this list: send the line "unsubscribe linux-ia64" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#7 lore raw
In reply to Alex Williamson
Quoted 9 lines

-----Original Message-----
From: Jesse Barnes [mailto:[email protected]]
Sent: Wednesday, March 03, 2004 2:56 PM
To: [email protected]
Cc: Alex Williamson; Seth, Rohit; linux-ia64
Subject: Re: [PATCH] more robust halt_light

Quoted 6 lines
Quoted 3 lines · David Mosberger

Can we get rid of the CONFIG option too? Or do folks who care (too?) much about wake-up latency prefer to turn off a CONFIG option over booting with nohlt?

Some people have come to me concerned about the wakeup latency of

having

Quoted 7 lines

the halt call in there, and it also looks like the PAL has a bug on our platform that causes hangs when we call PAL_HALT_LIGHT (which we're tracking down), so I wouldn't mind if the config option stuck around a little longer (or was a boot time parameter at least).

Thanks,
Jesse

As far as latency is concerned, do you have any specific workload that is getting impacted (severly) by this transition.

I think we should remove the compile time CONFIG_OPTION for PAL_HALT_LIGHT. And instead use /proc interface to dynamically enable/disable the power transition.

#8 lore raw
In reply to Alex Williamson
Quoted 15 lines

-----Original Message-----
From: Grant Grundler [mailto:[email protected]]
Sent: Wednesday, March 03, 2004 3:35 PM
To: Alex Williamson
Cc: Seth, Rohit; linux-ia64
Subject: Re: [PATCH] more robust halt_light

Quoted 4 lines · Alex Williamson

I was curious about that myself, but I thought I remembered from
Stephane that the perf counters didn't really recover from a PAL_HALT_LIGHT on McKinley.

Quoted 5 lines

The way I recall Stephane explaining it to me was the PMU was completely disabled after returning from PAL_HALT_LIGHT. ISTR no PAL fix was never released for Mckinley. And even if one was, we'd have to convince HP firmware teams to roll and qualify it. Unlikely.

In the new scheme that we are proposing, the PMUs are disabled at the time of entry to PAL_HALT_LIGHT. They will stay that way after returning from PAL call.

author
#9 lore raw
In reply to Alex Williamson
Quoted 10 lines · Jesse Barnes
Quoted 3 lines · David Mosberger

Can we get rid of the CONFIG option too? Or do folks who care (too?) much about wake-up latency prefer to turn off a CONFIG option over booting with nohlt?

Some people have come to me concerned about the wakeup latency of having the halt call in there, and it also looks like the PAL has a bug on our platform that causes hangs when we call PAL_HALT_LIGHT (which we're tracking down), so I wouldn't mind if the config option stuck around a little longer (or was a boot time parameter at least).

Jesse,

Perhaps sn specific code (maybe sn_setup) could call disable_hlt()
until you're comfortable leaving it on by default. Requiring a boot time command like option on your platform sounds error prone.

Alex

Signature · 1 line
Alex Williamson                             HP Linux & Open Source Lab
#10 lore raw
In reply to Alex Williamson
Quoted 2 lines · Rohit

As far as latency is concerned, do you have any specific workload that is getting impacted (severly) by this transition.

No specific concerns, just vague worry :).

Quoted 3 lines

I think we should remove the compile time CONFIG_OPTION for PAL_HALT_LIGHT. And instead use /proc interface to dynamically enable/disable the power transition.

Sure, a sysctl would be fine with me.

Jesse

#11 lore raw
In reply to Alex Williamson
Quoted 3 lines · Alex Williamson

Perhaps sn specific code (maybe sn_setup) could call disable_hlt() until you're comfortable leaving it on by default. Requiring a boot time command like option on your platform sounds error prone.

Yeah, that would probably be better than a boot time option.

Jesse