linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH] adjust x86-64 watchdog tick calculation
       [not found] <s2835f7d.038@emea1-mh.id2.novell.com>
@ 2005-05-12 12:52 ` Andi Kleen
  0 siblings, 0 replies; 7+ messages in thread
From: Andi Kleen @ 2005-05-12 12:52 UTC (permalink / raw)
  To: Jan Beulich; +Cc: ak, linux-kernel, discuss

On Thu, May 12, 2005 at 02:52:05PM +0200, Jan Beulich wrote:
> >> Get the x86-64 watchdog tick calculation into a state where it can also
> >> be used with nmi_hz other than 1Hz. Also do not turn on the watchdog by
> >> default (as is already done on i386).
> >
> >I already fixed this some time ago by using the same code as i386.
> >
> >That is what is in the current tree:
> >        wrmsr(MSR_K7_PERFCTR0, -(cpu_khz/nmi_hz*1000), -1);
> >
> >But I guess your version will work with a higher frequency, right?
> 
> For this part of the patch, yes, this will help with frequencies beyond 4GHz. The other change is which deals with nmi_hz being other than 1Hz.

I will queue it for after 2.6.12.
-Andi

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] adjust x86-64 watchdog tick calculation
@ 2005-05-12 12:52 Jan Beulich
  0 siblings, 0 replies; 7+ messages in thread
From: Jan Beulich @ 2005-05-12 12:52 UTC (permalink / raw)
  To: ak; +Cc: linux-kernel, discuss

>> Get the x86-64 watchdog tick calculation into a state where it can also
>> be used with nmi_hz other than 1Hz. Also do not turn on the watchdog by
>> default (as is already done on i386).
>
>I already fixed this some time ago by using the same code as i386.
>
>That is what is in the current tree:
>        wrmsr(MSR_K7_PERFCTR0, -(cpu_khz/nmi_hz*1000), -1);
>
>But I guess your version will work with a higher frequency, right?

For this part of the patch, yes, this will help with frequencies beyond 4GHz. The other change is which deals with nmi_hz being other than 1Hz.

Jan


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] adjust x86-64 watchdog tick calculation
       [not found] <s2835ea9.090@emea1-mh.id2.novell.com>
@ 2005-05-12 12:51 ` Andi Kleen
  0 siblings, 0 replies; 7+ messages in thread
From: Andi Kleen @ 2005-05-12 12:51 UTC (permalink / raw)
  To: Jan Beulich; +Cc: alexn, ak, linux-kernel, discuss

On Thu, May 12, 2005 at 02:48:26PM +0200, Jan Beulich wrote:
> >>> Alexander Nyberg <alexn@telia.com> 12.05.05 12:00:08 >>>
> >tor 2005-05-12 klockan 10:27 +0200 skrev Jan Beulich:
> >> Get the x86-64 watchdog tick calculation into a state where it can also
> >> be used with nmi_hz other than 1Hz. Also do not turn on the watchdog by
> >> default (as is already done on i386).
> >
> >Why shouldn't the watchdog be turned on by default? It's an extremely
> >useful debugging aid and it's not like it fires NMIs often (the nmi_hz
> >is far from reality).
> 
> For the so-called LAPIC one (based on performance monitor counters) that's true; that is for AMD boxes. For Intel boxes, which get defaulted to the IOAPIC version, it runs at HZ, and this is in my opinion significant overhead. If the Intel support for LAPIC watchdog was completed, and that used as the default, I would certainly agree to keeping it on by default.


I have Intel performance counter watchdog code in my tree, so that is already
done.

-Andi

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] adjust x86-64 watchdog tick calculation
@ 2005-05-12 12:48 Jan Beulich
  0 siblings, 0 replies; 7+ messages in thread
From: Jan Beulich @ 2005-05-12 12:48 UTC (permalink / raw)
  To: alexn; +Cc: ak, linux-kernel, discuss

>>> Alexander Nyberg <alexn@telia.com> 12.05.05 12:00:08 >>>
>tor 2005-05-12 klockan 10:27 +0200 skrev Jan Beulich:
>> Get the x86-64 watchdog tick calculation into a state where it can also
>> be used with nmi_hz other than 1Hz. Also do not turn on the watchdog by
>> default (as is already done on i386).
>
>Why shouldn't the watchdog be turned on by default? It's an extremely
>useful debugging aid and it's not like it fires NMIs often (the nmi_hz
>is far from reality).

For the so-called LAPIC one (based on performance monitor counters) that's true; that is for AMD boxes. For Intel boxes, which get defaulted to the IOAPIC version, it runs at HZ, and this is in my opinion significant overhead. If the Intel support for LAPIC watchdog was completed, and that used as the default, I would certainly agree to keeping it on by default.

Jan


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] adjust x86-64 watchdog tick calculation
       [not found] <s2832159.056@emea1-mh.id2.novell.com>
@ 2005-05-12 11:45 ` Andi Kleen
  0 siblings, 0 replies; 7+ messages in thread
From: Andi Kleen @ 2005-05-12 11:45 UTC (permalink / raw)
  To: Jan Beulich; +Cc: ak, linux-kernel, discuss

On Thu, May 12, 2005 at 10:27:09AM +0200, Jan Beulich wrote:
> (Note: Patch also attached because the inline version is certain to get
> line wrapped.)

Can you please only attach it then?

> 
> Get the x86-64 watchdog tick calculation into a state where it can also
> be used with nmi_hz other than 1Hz. Also do not turn on the watchdog by
> default (as is already done on i386).

I already fixed this some time ago by using the same code as i386.

That is what is in the current tree:
        wrmsr(MSR_K7_PERFCTR0, -(cpu_khz/nmi_hz*1000), -1);

But I guess your version will work with a higher frequency, right?	

-Andi

> 
> Signed-off-by: Jan Beulich <jbeulich@novell.com>
> 
> diff -Npru linux-2.6.12-rc4.base/arch/x86_64/kernel/nmi.c linux-2.6.12-rc4/arch/x86_64/kernel/nmi.c
> --- linux-2.6.12-rc4.base/arch/x86_64/kernel/nmi.c	2005-05-11 17:27:54.848855552 +0200
> +++ linux-2.6.12-rc4/arch/x86_64/kernel/nmi.c	2005-05-11 17:50:36.257889920 +0200
> @@ -57,7 +57,7 @@ static unsigned int lapic_nmi_owner;
>  int nmi_active;		/* oprofile uses this */
>  int panic_on_timeout;
>  
> -unsigned int nmi_watchdog = NMI_DEFAULT;
> +unsigned int nmi_watchdog = NMI_NONE;
>  static unsigned int nmi_hz = HZ;
>  unsigned int nmi_perfctr_msr;	/* the MSR to reset in NMI handler */
>  
> @@ -325,7 +325,7 @@ static void setup_k7_watchdog(void)
>  		| K7_NMI_EVENT;
>  
>  	wrmsr(MSR_K7_EVNTSEL0, evntsel, 0);
> -	wrmsrl(MSR_K7_PERFCTR0, -((u64)cpu_khz*1000) / nmi_hz);
> +	wrmsrl(MSR_K7_PERFCTR0, -((u64)cpu_khz * 1000 / nmi_hz));
>  	apic_write(APIC_LVTPC, APIC_DM_NMI);
>  	evntsel |= K7_EVNTSEL_ENABLE;
>  	wrmsr(MSR_K7_EVNTSEL0, evntsel, 0);
> @@ -404,7 +404,7 @@ void nmi_watchdog_tick (struct pt_regs *
>  		alert_counter[cpu] = 0;
>  	}
>  	if (nmi_perfctr_msr)
> -		wrmsr(nmi_perfctr_msr, -(cpu_khz/nmi_hz*1000), -1);
> +		wrmsrl(nmi_perfctr_msr, -((u64)cpu_khz * 1000 / nmi_hz));
>  }
>  
>  static int dummy_nmi_callback(struct pt_regs * regs, int cpu)
> 
> 

> (Note: Patch also attached because the inline version is certain to get
> line wrapped.)
> 
> Get the x86-64 watchdog tick calculation into a state where it can also
> be used with nmi_hz other than 1Hz. Also do not turn on the watchdog by
> default (as is already done on i386).
> 
> Signed-off-by: Jan Beulich <jbeulich@novell.com>
> 
> diff -Npru linux-2.6.12-rc4.base/arch/x86_64/kernel/nmi.c linux-2.6.12-rc4/arch/x86_64/kernel/nmi.c
> --- linux-2.6.12-rc4.base/arch/x86_64/kernel/nmi.c	2005-05-11 17:27:54.848855552 +0200
> +++ linux-2.6.12-rc4/arch/x86_64/kernel/nmi.c	2005-05-11 17:50:36.257889920 +0200
> @@ -57,7 +57,7 @@ static unsigned int lapic_nmi_owner;
>  int nmi_active;		/* oprofile uses this */
>  int panic_on_timeout;
>  
> -unsigned int nmi_watchdog = NMI_DEFAULT;
> +unsigned int nmi_watchdog = NMI_NONE;
>  static unsigned int nmi_hz = HZ;
>  unsigned int nmi_perfctr_msr;	/* the MSR to reset in NMI handler */
>  
> @@ -325,7 +325,7 @@ static void setup_k7_watchdog(void)
>  		| K7_NMI_EVENT;
>  
>  	wrmsr(MSR_K7_EVNTSEL0, evntsel, 0);
> -	wrmsrl(MSR_K7_PERFCTR0, -((u64)cpu_khz*1000) / nmi_hz);
> +	wrmsrl(MSR_K7_PERFCTR0, -((u64)cpu_khz * 1000 / nmi_hz));
>  	apic_write(APIC_LVTPC, APIC_DM_NMI);
>  	evntsel |= K7_EVNTSEL_ENABLE;
>  	wrmsr(MSR_K7_EVNTSEL0, evntsel, 0);
> @@ -404,7 +404,7 @@ void nmi_watchdog_tick (struct pt_regs *
>  		alert_counter[cpu] = 0;
>  	}
>  	if (nmi_perfctr_msr)
> -		wrmsr(nmi_perfctr_msr, -(cpu_khz/nmi_hz*1000), -1);
> +		wrmsrl(nmi_perfctr_msr, -((u64)cpu_khz * 1000 / nmi_hz));
>  }
>  
>  static int dummy_nmi_callback(struct pt_regs * regs, int cpu)


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] adjust x86-64 watchdog tick calculation
  2005-05-12  8:27 Jan Beulich
@ 2005-05-12 10:00 ` Alexander Nyberg
  0 siblings, 0 replies; 7+ messages in thread
From: Alexander Nyberg @ 2005-05-12 10:00 UTC (permalink / raw)
  To: Jan Beulich; +Cc: discuss, linux-kernel, ak

tor 2005-05-12 klockan 10:27 +0200 skrev Jan Beulich:
> (Note: Patch also attached because the inline version is certain to get
> line wrapped.)
> 
> Get the x86-64 watchdog tick calculation into a state where it can also
> be used with nmi_hz other than 1Hz. Also do not turn on the watchdog by
> default (as is already done on i386).
> 

Why shouldn't the watchdog be turned on by default? It's an extremely
useful debugging aid and it's not like it fires NMIs often (the nmi_hz
is far from reality).

> Signed-off-by: Jan Beulich <jbeulich@novell.com>
> 
> diff -Npru linux-2.6.12-rc4.base/arch/x86_64/kernel/nmi.c linux-2.6.12-rc4/arch/x86_64/kernel/nmi.c
> --- linux-2.6.12-rc4.base/arch/x86_64/kernel/nmi.c	2005-05-11 17:27:54.848855552 +0200
> +++ linux-2.6.12-rc4/arch/x86_64/kernel/nmi.c	2005-05-11 17:50:36.257889920 +0200
> @@ -57,7 +57,7 @@ static unsigned int lapic_nmi_owner;
>  int nmi_active;		/* oprofile uses this */
>  int panic_on_timeout;
>  
> -unsigned int nmi_watchdog = NMI_DEFAULT;
> +unsigned int nmi_watchdog = NMI_NONE;
>  static unsigned int nmi_hz = HZ;
>  unsigned int nmi_perfctr_msr;	/* the MSR to reset in NMI handler */
>  
> @@ -325,7 +325,7 @@ static void setup_k7_watchdog(void)
>  		| K7_NMI_EVENT;
>  
>  	wrmsr(MSR_K7_EVNTSEL0, evntsel, 0);
> -	wrmsrl(MSR_K7_PERFCTR0, -((u64)cpu_khz*1000) / nmi_hz);
> +	wrmsrl(MSR_K7_PERFCTR0, -((u64)cpu_khz * 1000 / nmi_hz));
>  	apic_write(APIC_LVTPC, APIC_DM_NMI);
>  	evntsel |= K7_EVNTSEL_ENABLE;
>  	wrmsr(MSR_K7_EVNTSEL0, evntsel, 0);
> @@ -404,7 +404,7 @@ void nmi_watchdog_tick (struct pt_regs *
>  		alert_counter[cpu] = 0;
>  	}
>  	if (nmi_perfctr_msr)
> -		wrmsr(nmi_perfctr_msr, -(cpu_khz/nmi_hz*1000), -1);
> +		wrmsrl(nmi_perfctr_msr, -((u64)cpu_khz * 1000 / nmi_hz));
>  }
>  
>  static int dummy_nmi_callback(struct pt_regs * regs, int cpu)
> 
> 



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH] adjust x86-64 watchdog tick calculation
@ 2005-05-12  8:27 Jan Beulich
  2005-05-12 10:00 ` Alexander Nyberg
  0 siblings, 1 reply; 7+ messages in thread
From: Jan Beulich @ 2005-05-12  8:27 UTC (permalink / raw)
  To: ak; +Cc: linux-kernel, discuss

[-- Attachment #1: Type: text/plain, Size: 1583 bytes --]

(Note: Patch also attached because the inline version is certain to get
line wrapped.)

Get the x86-64 watchdog tick calculation into a state where it can also
be used with nmi_hz other than 1Hz. Also do not turn on the watchdog by
default (as is already done on i386).

Signed-off-by: Jan Beulich <jbeulich@novell.com>

diff -Npru linux-2.6.12-rc4.base/arch/x86_64/kernel/nmi.c linux-2.6.12-rc4/arch/x86_64/kernel/nmi.c
--- linux-2.6.12-rc4.base/arch/x86_64/kernel/nmi.c	2005-05-11 17:27:54.848855552 +0200
+++ linux-2.6.12-rc4/arch/x86_64/kernel/nmi.c	2005-05-11 17:50:36.257889920 +0200
@@ -57,7 +57,7 @@ static unsigned int lapic_nmi_owner;
 int nmi_active;		/* oprofile uses this */
 int panic_on_timeout;
 
-unsigned int nmi_watchdog = NMI_DEFAULT;
+unsigned int nmi_watchdog = NMI_NONE;
 static unsigned int nmi_hz = HZ;
 unsigned int nmi_perfctr_msr;	/* the MSR to reset in NMI handler */
 
@@ -325,7 +325,7 @@ static void setup_k7_watchdog(void)
 		| K7_NMI_EVENT;
 
 	wrmsr(MSR_K7_EVNTSEL0, evntsel, 0);
-	wrmsrl(MSR_K7_PERFCTR0, -((u64)cpu_khz*1000) / nmi_hz);
+	wrmsrl(MSR_K7_PERFCTR0, -((u64)cpu_khz * 1000 / nmi_hz));
 	apic_write(APIC_LVTPC, APIC_DM_NMI);
 	evntsel |= K7_EVNTSEL_ENABLE;
 	wrmsr(MSR_K7_EVNTSEL0, evntsel, 0);
@@ -404,7 +404,7 @@ void nmi_watchdog_tick (struct pt_regs *
 		alert_counter[cpu] = 0;
 	}
 	if (nmi_perfctr_msr)
-		wrmsr(nmi_perfctr_msr, -(cpu_khz/nmi_hz*1000), -1);
+		wrmsrl(nmi_perfctr_msr, -((u64)cpu_khz * 1000 / nmi_hz));
 }
 
 static int dummy_nmi_callback(struct pt_regs * regs, int cpu)



[-- Attachment #2: linux-2.6.12-rc4-x86_64-watchdog.patch --]
[-- Type: text/plain, Size: 1540 bytes --]

(Note: Patch also attached because the inline version is certain to get
line wrapped.)

Get the x86-64 watchdog tick calculation into a state where it can also
be used with nmi_hz other than 1Hz. Also do not turn on the watchdog by
default (as is already done on i386).

Signed-off-by: Jan Beulich <jbeulich@novell.com>

diff -Npru linux-2.6.12-rc4.base/arch/x86_64/kernel/nmi.c linux-2.6.12-rc4/arch/x86_64/kernel/nmi.c
--- linux-2.6.12-rc4.base/arch/x86_64/kernel/nmi.c	2005-05-11 17:27:54.848855552 +0200
+++ linux-2.6.12-rc4/arch/x86_64/kernel/nmi.c	2005-05-11 17:50:36.257889920 +0200
@@ -57,7 +57,7 @@ static unsigned int lapic_nmi_owner;
 int nmi_active;		/* oprofile uses this */
 int panic_on_timeout;
 
-unsigned int nmi_watchdog = NMI_DEFAULT;
+unsigned int nmi_watchdog = NMI_NONE;
 static unsigned int nmi_hz = HZ;
 unsigned int nmi_perfctr_msr;	/* the MSR to reset in NMI handler */
 
@@ -325,7 +325,7 @@ static void setup_k7_watchdog(void)
 		| K7_NMI_EVENT;
 
 	wrmsr(MSR_K7_EVNTSEL0, evntsel, 0);
-	wrmsrl(MSR_K7_PERFCTR0, -((u64)cpu_khz*1000) / nmi_hz);
+	wrmsrl(MSR_K7_PERFCTR0, -((u64)cpu_khz * 1000 / nmi_hz));
 	apic_write(APIC_LVTPC, APIC_DM_NMI);
 	evntsel |= K7_EVNTSEL_ENABLE;
 	wrmsr(MSR_K7_EVNTSEL0, evntsel, 0);
@@ -404,7 +404,7 @@ void nmi_watchdog_tick (struct pt_regs *
 		alert_counter[cpu] = 0;
 	}
 	if (nmi_perfctr_msr)
-		wrmsr(nmi_perfctr_msr, -(cpu_khz/nmi_hz*1000), -1);
+		wrmsrl(nmi_perfctr_msr, -((u64)cpu_khz * 1000 / nmi_hz));
 }
 
 static int dummy_nmi_callback(struct pt_regs * regs, int cpu)

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2005-05-12 12:54 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <s2835f7d.038@emea1-mh.id2.novell.com>
2005-05-12 12:52 ` [PATCH] adjust x86-64 watchdog tick calculation Andi Kleen
2005-05-12 12:52 Jan Beulich
     [not found] <s2835ea9.090@emea1-mh.id2.novell.com>
2005-05-12 12:51 ` Andi Kleen
  -- strict thread matches above, loose matches on Subject: below --
2005-05-12 12:48 Jan Beulich
     [not found] <s2832159.056@emea1-mh.id2.novell.com>
2005-05-12 11:45 ` Andi Kleen
2005-05-12  8:27 Jan Beulich
2005-05-12 10:00 ` Alexander Nyberg

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