[PATCH] more robust halt_light

[PATCH] more robust halt_light

[Alex Williamson]

  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

-- 
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);

RE: [PATCH] more robust halt_light

[Seth, 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
>-----Original Message-----
>From: linux-ia64-owner@vger.kernel.org [mailto:linux-ia64-
>owner@vger.kernel.org] On Behalf Of Alex Williamson
>Sent: Wednesday, March 03, 2004 9:57 AM
>To: linux-ia64@vger.kernel.org
>Subject: [PATCH] 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
>
>--
>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);
>
>
>-
>To unsubscribe from this list: send the line "unsubscribe linux-ia64"
in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] more robust halt_light

[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

On Wed, 2004-03-03 at 13:38, Seth, Rohit wrote:
> 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
> >-----Original Message-----
> >Sent: Wednesday, March 03, 2004 9:57 AM
> >To: linux-ia64@vger.kernel.org
> >Subject: [PATCH] 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
> >

-- 
Alex Williamson                             HP Linux & Open Source Lab

RE: [PATCH] more robust halt_light

[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

-- 
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);

RE: [PATCH] more robust halt_light

[David Mosberger]

>>>>> On Wed, 03 Mar 2004 14:08:00 -0700, Alex Williamson <alex.williamson@hp.com> said:

  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

Re: [PATCH] more robust halt_light

[Jesse Barnes]

On Wed, Mar 03, 2004 at 02:45:11PM -0800, David Mosberger wrote:
> 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

Re: [PATCH] more robust halt_light

[Grant Grundler]

On Wed, Mar 03, 2004 at 02:08:00PM -0700, Alex Williamson wrote:
> 
>    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.

> 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

> 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
> 
> On Wed, 2004-03-03 at 13:38, Seth, Rohit wrote:
> > 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
> > >-----Original Message-----
> > >Sent: Wednesday, March 03, 2004 9:57 AM
> > >To: linux-ia64@vger.kernel.org
> > >Subject: [PATCH] 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
> > >
> 
> -- 
> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] more robust halt_light

[Seth, Rohit]

>-----Original Message-----
>From: Jesse Barnes [mailto:jbarnes@sgi.com]
>Sent: Wednesday, March 03, 2004 2:56 PM
>To: davidm@hpl.hp.com
>Cc: Alex Williamson; Seth, Rohit; linux-ia64
>Subject: Re: [PATCH] more robust halt_light
>
>On Wed, Mar 03, 2004 at 02:45:11PM -0800, David Mosberger wrote:
>> 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

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.

RE: [PATCH] more robust halt_light

[Seth, Rohit]

>-----Original Message-----
>From: Grant Grundler [mailto:iod00d@hp.com]
>Sent: Wednesday, March 03, 2004 3:35 PM
>To: Alex Williamson
>Cc: Seth, Rohit; linux-ia64
>Subject: Re: [PATCH] more robust halt_light
>
>On Wed, Mar 03, 2004 at 02:08:00PM -0700, Alex Williamson wrote:
>>
>>    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.

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.

Re: [PATCH] more robust halt_light

[Alex Williamson]

On Wed, 2004-03-03 at 15:56, Jesse Barnes wrote:
> On Wed, Mar 03, 2004 at 02:45:11PM -0800, David Mosberger wrote:
> > 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

-- 
Alex Williamson                             HP Linux & Open Source Lab

Re: [PATCH] more robust halt_light

[Jesse Barnes]

On Wed, Mar 03, 2004 at 06:40:41PM -0800, Seth, Rohit wrote:
> 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 :).

> 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

Re: [PATCH] more robust halt_light

[Jesse Barnes]

On Thu, Mar 04, 2004 at 08:47:27AM -0700, Alex Williamson wrote:
> 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