* Re: [PATCH] x86: Distinguish TLB shootdown interrupts from other functions call interrupts [not found] <50611D82.4010702@intel.com> @ 2012-09-26 2:11 ` Tomoki Sekiyama 2012-09-26 2:17 ` Alex Shi ` (2 more replies) 0 siblings, 3 replies; 11+ messages in thread From: Tomoki Sekiyama @ 2012-09-26 2:11 UTC (permalink / raw) To: alex.shi Cc: x86, linux-kernel, yrl.pp-manager.tt, Ingo Molnar, H. Peter Anvin, Thomas Gleixner Hi Alex, On 2012/09/25 11:57, Alex Shi wrote: > On 09/24/2012 09:37 AM, Alex Shi wrote: > >> On 09/20/2012 04:50 PM, Tomoki Sekiyama wrote: >> >>> unsigned int irq_resched_count; >>> unsigned int irq_call_count; >>> + /* irq_tlb_count is double-counted in irq_call_count, so it must be >>> + subtracted from irq_call_count when displaying irq_call_count */ >>> unsigned int irq_tlb_count; >> >> Review again this patch, above comments is not kernel compatible format. >> Could you change it like standard comment format: >> >> /* >> * xxxxxxx >> * xxxx >> */ >> > > the 3.6 kernel will closed soon. it will be great to has this patch in. > So, could you like to refresh your patch with popular comments format? :) Fixed patch is below. Thank you for the review again. -- As TLB shootdown requests to other CPU cores are now using function call interrupts, TLB shootdowns entry in /proc/interrupts is always shown as 0. This behavior change was introduced by commit 52aec3308db8 ("x86/tlb: replace INVALIDATE_TLB_VECTOR by CALL_FUNCTION_VECTOR"). This patch reverts TLB shootdowns entry in /proc/interrupts to count TLB shootdowns separately from the other function call interrupts. Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama.qu@hitachi.com> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Ingo Molnar <mingo@redhat.com> Cc: "H. Peter Anvin" <hpa@zytor.com> Cc: Alex Shi <alex.shi@intel.com> --- arch/x86/include/asm/hardirq.h | 4 ++++ arch/x86/kernel/irq.c | 4 ++-- arch/x86/mm/tlb.c | 2 ++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/arch/x86/include/asm/hardirq.h b/arch/x86/include/asm/hardirq.h index d3895db..81f04ce 100644 --- a/arch/x86/include/asm/hardirq.h +++ b/arch/x86/include/asm/hardirq.h @@ -18,6 +18,10 @@ typedef struct { #ifdef CONFIG_SMP unsigned int irq_resched_count; unsigned int irq_call_count; + /* + * irq_tlb_count is double-counted in irq_call_count, so it must be + * subtracted from irq_call_count when displaying irq_call_count + */ unsigned int irq_tlb_count; #endif #ifdef CONFIG_X86_THERMAL_VECTOR diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c index d44f782..e4595f1 100644 --- a/arch/x86/kernel/irq.c +++ b/arch/x86/kernel/irq.c @@ -92,7 +92,8 @@ int arch_show_interrupts(struct seq_file *p, int prec) seq_printf(p, " Rescheduling interrupts\n"); seq_printf(p, "%*s: ", prec, "CAL"); for_each_online_cpu(j) - seq_printf(p, "%10u ", irq_stats(j)->irq_call_count); + seq_printf(p, "%10u ", irq_stats(j)->irq_call_count - + irq_stats(j)->irq_tlb_count); seq_printf(p, " Function call interrupts\n"); seq_printf(p, "%*s: ", prec, "TLB"); for_each_online_cpu(j) @@ -147,7 +148,6 @@ u64 arch_irq_stat_cpu(unsigned int cpu) #ifdef CONFIG_SMP sum += irq_stats(cpu)->irq_resched_count; sum += irq_stats(cpu)->irq_call_count; - sum += irq_stats(cpu)->irq_tlb_count; #endif #ifdef CONFIG_X86_THERMAL_VECTOR sum += irq_stats(cpu)->irq_thermal_count; diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c index 613cd83..2d6d8ed 100644 --- a/arch/x86/mm/tlb.c +++ b/arch/x86/mm/tlb.c @@ -98,6 +98,8 @@ static void flush_tlb_func(void *info) { struct flush_tlb_info *f = info; + inc_irq_stat(irq_tlb_count); + if (f->flush_mm != this_cpu_read(cpu_tlbstate.active_mm)) return; ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] x86: Distinguish TLB shootdown interrupts from other functions call interrupts 2012-09-26 2:11 ` [PATCH] x86: Distinguish TLB shootdown interrupts from other functions call interrupts Tomoki Sekiyama @ 2012-09-26 2:17 ` Alex Shi 2012-09-27 7:02 ` Alex Shi 2012-09-28 6:13 ` [tip:x86/mm] " tip-bot for Tomoki Sekiyama 2 siblings, 0 replies; 11+ messages in thread From: Alex Shi @ 2012-09-26 2:17 UTC (permalink / raw) To: Tomoki Sekiyama Cc: x86, linux-kernel, yrl.pp-manager.tt, Ingo Molnar, H. Peter Anvin, Thomas Gleixner On 09/26/2012 10:11 AM, Tomoki Sekiyama wrote: > Hi Alex, > > On 2012/09/25 11:57, Alex Shi wrote: >> On 09/24/2012 09:37 AM, Alex Shi wrote: >> >>> On 09/20/2012 04:50 PM, Tomoki Sekiyama wrote: >>> >>>> unsigned int irq_resched_count; >>>> unsigned int irq_call_count; >>>> + /* irq_tlb_count is double-counted in irq_call_count, so it must be >>>> + subtracted from irq_call_count when displaying irq_call_count */ >>>> unsigned int irq_tlb_count; >>> >>> Review again this patch, above comments is not kernel compatible format. >>> Could you change it like standard comment format: >>> >>> /* >>> * xxxxxxx >>> * xxxx >>> */ >>> >> >> the 3.6 kernel will closed soon. it will be great to has this patch in. >> So, could you like to refresh your patch with popular comments format? :) > > Fixed patch is below. > Thank you for the review again. > Acked-by: Alex Shi <alex.shi@intel.com> > -- > As TLB shootdown requests to other CPU cores are now using function call > interrupts, TLB shootdowns entry in /proc/interrupts is always shown as 0. > > This behavior change was introduced by commit 52aec3308db8 ("x86/tlb: > replace INVALIDATE_TLB_VECTOR by CALL_FUNCTION_VECTOR"). > > This patch reverts TLB shootdowns entry in /proc/interrupts to count TLB > shootdowns separately from the other function call interrupts. > > Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama.qu@hitachi.com> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: "H. Peter Anvin" <hpa@zytor.com> > Cc: Alex Shi <alex.shi@intel.com> > --- > arch/x86/include/asm/hardirq.h | 4 ++++ > arch/x86/kernel/irq.c | 4 ++-- > arch/x86/mm/tlb.c | 2 ++ > 3 files changed, 8 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/include/asm/hardirq.h b/arch/x86/include/asm/hardirq.h > index d3895db..81f04ce 100644 > --- a/arch/x86/include/asm/hardirq.h > +++ b/arch/x86/include/asm/hardirq.h > @@ -18,6 +18,10 @@ typedef struct { > #ifdef CONFIG_SMP > unsigned int irq_resched_count; > unsigned int irq_call_count; > + /* > + * irq_tlb_count is double-counted in irq_call_count, so it must be > + * subtracted from irq_call_count when displaying irq_call_count > + */ > unsigned int irq_tlb_count; > #endif > #ifdef CONFIG_X86_THERMAL_VECTOR > diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c > index d44f782..e4595f1 100644 > --- a/arch/x86/kernel/irq.c > +++ b/arch/x86/kernel/irq.c > @@ -92,7 +92,8 @@ int arch_show_interrupts(struct seq_file *p, int prec) > seq_printf(p, " Rescheduling interrupts\n"); > seq_printf(p, "%*s: ", prec, "CAL"); > for_each_online_cpu(j) > - seq_printf(p, "%10u ", irq_stats(j)->irq_call_count); > + seq_printf(p, "%10u ", irq_stats(j)->irq_call_count - > + irq_stats(j)->irq_tlb_count); > seq_printf(p, " Function call interrupts\n"); > seq_printf(p, "%*s: ", prec, "TLB"); > for_each_online_cpu(j) > @@ -147,7 +148,6 @@ u64 arch_irq_stat_cpu(unsigned int cpu) > #ifdef CONFIG_SMP > sum += irq_stats(cpu)->irq_resched_count; > sum += irq_stats(cpu)->irq_call_count; > - sum += irq_stats(cpu)->irq_tlb_count; > #endif > #ifdef CONFIG_X86_THERMAL_VECTOR > sum += irq_stats(cpu)->irq_thermal_count; > diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c > index 613cd83..2d6d8ed 100644 > --- a/arch/x86/mm/tlb.c > +++ b/arch/x86/mm/tlb.c > @@ -98,6 +98,8 @@ static void flush_tlb_func(void *info) > { > struct flush_tlb_info *f = info; > > + inc_irq_stat(irq_tlb_count); > + > if (f->flush_mm != this_cpu_read(cpu_tlbstate.active_mm)) > return; > > -- Thanks Alex ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] x86: Distinguish TLB shootdown interrupts from other functions call interrupts 2012-09-26 2:11 ` [PATCH] x86: Distinguish TLB shootdown interrupts from other functions call interrupts Tomoki Sekiyama 2012-09-26 2:17 ` Alex Shi @ 2012-09-27 7:02 ` Alex Shi 2012-09-28 5:49 ` H. Peter Anvin 2012-09-28 6:13 ` [tip:x86/mm] " tip-bot for Tomoki Sekiyama 2 siblings, 1 reply; 11+ messages in thread From: Alex Shi @ 2012-09-27 7:02 UTC (permalink / raw) To: H. Peter Anvin Cc: Tomoki Sekiyama, x86, linux-kernel, yrl.pp-manager.tt, Ingo Molnar, Thomas Gleixner >> >> the 3.6 kernel will closed soon. it will be great to has this patch in. >> So, could you like to refresh your patch with popular comments format? :) > > Fixed patch is below. > Thank you for the review again. > Peter: Maybe the patch doesn't looks perfect for this issue. So I am wondering if the following patch is better, if we don't care the irq_tlb was counted again in irq_call? === diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c index 613cd83..2d6d8ed 100644 --- a/arch/x86/mm/tlb.c +++ b/arch/x86/mm/tlb.c @@ -98,6 +98,8 @@ static void flush_tlb_func(void *info) { struct flush_tlb_info *f = info; + inc_irq_stat(irq_tlb_count); + if (f->flush_mm != this_cpu_read(cpu_tlbstate.active_mm)) return; === Or another solution is also reducing double counted irq_tlb_count from irq_call_count What's your comments of this? === diff --git a/arch/x86/include/asm/hardirq.h b/arch/x86/include/asm/hardirq.h index d3895db..aac7886 100644 --- a/arch/x86/include/asm/hardirq.h +++ b/arch/x86/include/asm/hardirq.h @@ -36,6 +36,7 @@ DECLARE_PER_CPU_SHARED_ALIGNED(irq_cpustat_t, irq_stat); #define __ARCH_IRQ_STAT #define inc_irq_stat(member) this_cpu_inc(irq_stat.member) +#define dec_irq_stat(member) this_cpu_dec(irq_stat.member) #define local_softirq_pending() this_cpu_read(irq_stat.__softirq_pending) diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c index 613cd83..b82bd9f 100644 --- a/arch/x86/mm/tlb.c +++ b/arch/x86/mm/tlb.c @@ -98,6 +98,10 @@ static void flush_tlb_func(void *info) { struct flush_tlb_info *f = info; + /* remove double counted irq_tlb_count from irq_call_count */ + dec_irq_stat(irq_call_count); + inc_irq_stat(irq_tlb_count); + if (f->flush_mm != this_cpu_read(cpu_tlbstate.active_mm)) return; === Thanks Alex ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] x86: Distinguish TLB shootdown interrupts from other functions call interrupts 2012-09-27 7:02 ` Alex Shi @ 2012-09-28 5:49 ` H. Peter Anvin 2012-09-28 6:08 ` Alex Shi 0 siblings, 1 reply; 11+ messages in thread From: H. Peter Anvin @ 2012-09-28 5:49 UTC (permalink / raw) To: Alex Shi Cc: Tomoki Sekiyama, x86, linux-kernel, yrl.pp-manager.tt, Ingo Molnar, Thomas Gleixner On 09/27/2012 12:02 AM, Alex Shi wrote: > > Peter: > > Maybe the patch doesn't looks perfect for this issue. > So I am wondering if the following patch is better, if we don't care the irq_tlb > was counted again in irq_call? > Tomoki-san's patch looked sane to me, I should just apply it. -hpa -- H. Peter Anvin, Intel Open Source Technology Center I work for Intel. I don't speak on their behalf. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] x86: Distinguish TLB shootdown interrupts from other functions call interrupts 2012-09-28 5:49 ` H. Peter Anvin @ 2012-09-28 6:08 ` Alex Shi 0 siblings, 0 replies; 11+ messages in thread From: Alex Shi @ 2012-09-28 6:08 UTC (permalink / raw) To: H. Peter Anvin Cc: Tomoki Sekiyama, x86, linux-kernel, yrl.pp-manager.tt, Ingo Molnar, Thomas Gleixner On 09/28/2012 01:49 PM, H. Peter Anvin wrote: > On 09/27/2012 12:02 AM, Alex Shi wrote: >> >> Peter: >> >> Maybe the patch doesn't looks perfect for this issue. >> So I am wondering if the following patch is better, if we don't care >> the irq_tlb >> was counted again in irq_call? >> > > Tomoki-san's patch looked sane to me, I should just apply it. > > -hpa > Glad to see this! :) -- Thanks Alex ^ permalink raw reply [flat|nested] 11+ messages in thread
* [tip:x86/mm] x86: Distinguish TLB shootdown interrupts from other functions call interrupts 2012-09-26 2:11 ` [PATCH] x86: Distinguish TLB shootdown interrupts from other functions call interrupts Tomoki Sekiyama 2012-09-26 2:17 ` Alex Shi 2012-09-27 7:02 ` Alex Shi @ 2012-09-28 6:13 ` tip-bot for Tomoki Sekiyama 2 siblings, 0 replies; 11+ messages in thread From: tip-bot for Tomoki Sekiyama @ 2012-09-28 6:13 UTC (permalink / raw) To: linux-tip-commits Cc: linux-kernel, hpa, mingo, alex.shi, tglx, hpa, tomoki.sekiyama.qu Commit-ID: fd0f5869724ff6195c6e7f12f8287c66a132e0ba Gitweb: http://git.kernel.org/tip/fd0f5869724ff6195c6e7f12f8287c66a132e0ba Author: Tomoki Sekiyama <tomoki.sekiyama.qu@hitachi.com> AuthorDate: Wed, 26 Sep 2012 11:11:28 +0900 Committer: H. Peter Anvin <hpa@linux.intel.com> CommitDate: Thu, 27 Sep 2012 22:52:34 -0700 x86: Distinguish TLB shootdown interrupts from other functions call interrupts As TLB shootdown requests to other CPU cores are now using function call interrupts, TLB shootdowns entry in /proc/interrupts is always shown as 0. This behavior change was introduced by commit 52aec3308db8 ("x86/tlb: replace INVALIDATE_TLB_VECTOR by CALL_FUNCTION_VECTOR"). This patch reverts TLB shootdowns entry in /proc/interrupts to count TLB shootdowns separately from the other function call interrupts. Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama.qu@hitachi.com> Link: http://lkml.kernel.org/r/20120926021128.22212.20440.stgit@hpxw Acked-by: Alex Shi <alex.shi@intel.com> Signed-off-by: H. Peter Anvin <hpa@linux.intel.com> --- arch/x86/include/asm/hardirq.h | 4 ++++ arch/x86/kernel/irq.c | 4 ++-- arch/x86/mm/tlb.c | 2 ++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/arch/x86/include/asm/hardirq.h b/arch/x86/include/asm/hardirq.h index d3895db..81f04ce 100644 --- a/arch/x86/include/asm/hardirq.h +++ b/arch/x86/include/asm/hardirq.h @@ -18,6 +18,10 @@ typedef struct { #ifdef CONFIG_SMP unsigned int irq_resched_count; unsigned int irq_call_count; + /* + * irq_tlb_count is double-counted in irq_call_count, so it must be + * subtracted from irq_call_count when displaying irq_call_count + */ unsigned int irq_tlb_count; #endif #ifdef CONFIG_X86_THERMAL_VECTOR diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c index 1f5f1d5..355b13f 100644 --- a/arch/x86/kernel/irq.c +++ b/arch/x86/kernel/irq.c @@ -92,7 +92,8 @@ int arch_show_interrupts(struct seq_file *p, int prec) seq_printf(p, " Rescheduling interrupts\n"); seq_printf(p, "%*s: ", prec, "CAL"); for_each_online_cpu(j) - seq_printf(p, "%10u ", irq_stats(j)->irq_call_count); + seq_printf(p, "%10u ", irq_stats(j)->irq_call_count - + irq_stats(j)->irq_tlb_count); seq_printf(p, " Function call interrupts\n"); seq_printf(p, "%*s: ", prec, "TLB"); for_each_online_cpu(j) @@ -147,7 +148,6 @@ u64 arch_irq_stat_cpu(unsigned int cpu) #ifdef CONFIG_SMP sum += irq_stats(cpu)->irq_resched_count; sum += irq_stats(cpu)->irq_call_count; - sum += irq_stats(cpu)->irq_tlb_count; #endif #ifdef CONFIG_X86_THERMAL_VECTOR sum += irq_stats(cpu)->irq_thermal_count; diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c index a085c56..0777f04 100644 --- a/arch/x86/mm/tlb.c +++ b/arch/x86/mm/tlb.c @@ -98,6 +98,8 @@ static void flush_tlb_func(void *info) { struct flush_tlb_info *f = info; + inc_irq_stat(irq_tlb_count); + if (f->flush_mm != this_cpu_read(cpu_tlbstate.active_mm)) return; ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH] x86: Distinguish TLB shootdown interrupts from other functions call interrupts
@ 2012-09-19 9:04 Tomoki Sekiyama
2012-09-20 1:18 ` Alex Shi
0 siblings, 1 reply; 11+ messages in thread
From: Tomoki Sekiyama @ 2012-09-19 9:04 UTC (permalink / raw)
To: x86
Cc: Alex Shi, yrl.pp-manager.tt, linux-kernel, Ingo Molnar,
H. Peter Anvin, Thomas Gleixner
As TLB shootdown requests to other CPU cores are now done using function call
interrupts, TLB shootdowns entry in /proc/interrupts is always shown as 0.
This behavior change was introduced by commit 52aec3308db8 ("x86/tlb:
replace INVALIDATE_TLB_VECTOR by CALL_FUNCTION_VECTOR").
This patch reverts TLB shootdowns entry in /proc/interrupts to count TLB
shootdowns separately from the other function call interrupts.
Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama.qu@hitachi.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Alex Shi <alex.shi@intel.com>
---
arch/x86/include/asm/hardirq.h | 2 +-
arch/x86/kernel/irq.c | 5 +++--
arch/x86/mm/tlb.c | 3 +++
3 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/arch/x86/include/asm/hardirq.h b/arch/x86/include/asm/hardirq.h
index d3895db..af60ab5 100644
--- a/arch/x86/include/asm/hardirq.h
+++ b/arch/x86/include/asm/hardirq.h
@@ -18,7 +18,7 @@ typedef struct {
#ifdef CONFIG_SMP
unsigned int irq_resched_count;
unsigned int irq_call_count;
- unsigned int irq_tlb_count;
+ unsigned int irq_tlb_count; /* double-counted in irq_call_count */
#endif
#ifdef CONFIG_X86_THERMAL_VECTOR
unsigned int irq_thermal_count;
diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c
index d44f782..6dfa8b1 100644
--- a/arch/x86/kernel/irq.c
+++ b/arch/x86/kernel/irq.c
@@ -92,7 +92,8 @@ int arch_show_interrupts(struct seq_file *p, int prec)
seq_printf(p, " Rescheduling interrupts\n");
seq_printf(p, "%*s: ", prec, "CAL");
for_each_online_cpu(j)
- seq_printf(p, "%10u ", irq_stats(j)->irq_call_count);
+ seq_printf(p, "%10u ", irq_stats(j)->irq_call_count -
+ irq_stats(j)->irq_tlb_count);
seq_printf(p, " Function call interrupts\n");
seq_printf(p, "%*s: ", prec, "TLB");
for_each_online_cpu(j)
@@ -147,7 +148,7 @@ u64 arch_irq_stat_cpu(unsigned int cpu)
#ifdef CONFIG_SMP
sum += irq_stats(cpu)->irq_resched_count;
sum += irq_stats(cpu)->irq_call_count;
- sum += irq_stats(cpu)->irq_tlb_count;
+ /* irq_tlb_count is already added to irq_call_count */
#endif
#ifdef CONFIG_X86_THERMAL_VECTOR
sum += irq_stats(cpu)->irq_thermal_count;
diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
index 613cd83..0a054db 100644
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -98,6 +98,9 @@ static void flush_tlb_func(void *info)
{
struct flush_tlb_info *f = info;
+ /* irq_call_cnt is also incremented; be subtracted on display */
+ inc_irq_stat(irq_tlb_count);
+
if (f->flush_mm != this_cpu_read(cpu_tlbstate.active_mm))
return;
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH] x86: Distinguish TLB shootdown interrupts from other functions call interrupts 2012-09-19 9:04 [PATCH] " Tomoki Sekiyama @ 2012-09-20 1:18 ` Alex Shi 2012-09-20 8:50 ` Tomoki Sekiyama 0 siblings, 1 reply; 11+ messages in thread From: Alex Shi @ 2012-09-20 1:18 UTC (permalink / raw) To: Tomoki Sekiyama Cc: x86, yrl.pp-manager.tt, linux-kernel, Ingo Molnar, H. Peter Anvin, Thomas Gleixner > @@ -147,7 +148,7 @@ u64 arch_irq_stat_cpu(unsigned int cpu) > #ifdef CONFIG_SMP > sum += irq_stats(cpu)->irq_resched_count; > sum += irq_stats(cpu)->irq_call_count; > - sum += irq_stats(cpu)->irq_tlb_count; > + /* irq_tlb_count is already added to irq_call_count */ redundant comments here? > #endif > #ifdef CONFIG_X86_THERMAL_VECTOR > sum += irq_stats(cpu)->irq_thermal_count; > diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c > index 613cd83..0a054db 100644 > --- a/arch/x86/mm/tlb.c > +++ b/arch/x86/mm/tlb.c > @@ -98,6 +98,9 @@ static void flush_tlb_func(void *info) > { > struct flush_tlb_info *f = info; > > + /* irq_call_cnt is also incremented; be subtracted on display */ If is it better to move above explanation to irq_call_cnt definition place: harirq.h? > + inc_irq_stat(irq_tlb_count); > + > if (f->flush_mm != this_cpu_read(cpu_tlbstate.active_mm)) > return; > > -- Thanks Alex ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] x86: Distinguish TLB shootdown interrupts from other functions call interrupts 2012-09-20 1:18 ` Alex Shi @ 2012-09-20 8:50 ` Tomoki Sekiyama 2012-09-21 8:08 ` Alex Shi 2012-09-24 1:37 ` Alex Shi 0 siblings, 2 replies; 11+ messages in thread From: Tomoki Sekiyama @ 2012-09-20 8:50 UTC (permalink / raw) To: x86 Cc: Alex Shi, yrl.pp-manager.tt, linux-kernel, Ingo Molnar, H. Peter Anvin, Thomas Gleixner Hi Alex, thank you for the review. >> sum += irq_stats(cpu)->irq_call_count; >> - sum += irq_stats(cpu)->irq_tlb_count; >> + /* irq_tlb_count is already added to irq_call_count */ > >redundant comments here? >> @@ -98,6 +98,9 @@ static void flush_tlb_func(void *info) >> { >> struct flush_tlb_info *f = info; >> >> + /* irq_call_cnt is also incremented; be subtracted on display */ > >If is it better to move above explanation to irq_call_cnt definition place: harirq.h? Agreed. In the patch below, I reduced the redundant comments. -- As TLB shootdown requests to other CPU cores are now using function call interrupts, TLB shootdowns entry in /proc/interrupts is always shown as 0. This behavior change was introduced by commit 52aec3308db8 ("x86/tlb: replace INVALIDATE_TLB_VECTOR by CALL_FUNCTION_VECTOR"). This patch reverts TLB shootdowns entry in /proc/interrupts to count TLB shootdowns separately from the other function call interrupts. Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama.qu@hitachi.com> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Ingo Molnar <mingo@redhat.com> Cc: "H. Peter Anvin" <hpa@zytor.com> Cc: Alex Shi <alex.shi@intel.com> --- arch/x86/include/asm/hardirq.h | 2 ++ arch/x86/kernel/irq.c | 4 ++-- arch/x86/mm/tlb.c | 2 ++ 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/arch/x86/include/asm/hardirq.h b/arch/x86/include/asm/hardirq.h index d3895db..e34b252 100644 --- a/arch/x86/include/asm/hardirq.h +++ b/arch/x86/include/asm/hardirq.h @@ -18,6 +18,8 @@ typedef struct { #ifdef CONFIG_SMP unsigned int irq_resched_count; unsigned int irq_call_count; + /* irq_tlb_count is double-counted in irq_call_count, so it must be + subtracted from irq_call_count when displaying irq_call_count */ unsigned int irq_tlb_count; #endif #ifdef CONFIG_X86_THERMAL_VECTOR diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c index d44f782..e4595f1 100644 --- a/arch/x86/kernel/irq.c +++ b/arch/x86/kernel/irq.c @@ -92,7 +92,8 @@ int arch_show_interrupts(struct seq_file *p, int prec) seq_printf(p, " Rescheduling interrupts\n"); seq_printf(p, "%*s: ", prec, "CAL"); for_each_online_cpu(j) - seq_printf(p, "%10u ", irq_stats(j)->irq_call_count); + seq_printf(p, "%10u ", irq_stats(j)->irq_call_count - + irq_stats(j)->irq_tlb_count); seq_printf(p, " Function call interrupts\n"); seq_printf(p, "%*s: ", prec, "TLB"); for_each_online_cpu(j) @@ -147,7 +148,6 @@ u64 arch_irq_stat_cpu(unsigned int cpu) #ifdef CONFIG_SMP sum += irq_stats(cpu)->irq_resched_count; sum += irq_stats(cpu)->irq_call_count; - sum += irq_stats(cpu)->irq_tlb_count; #endif #ifdef CONFIG_X86_THERMAL_VECTOR sum += irq_stats(cpu)->irq_thermal_count; diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c index 613cd83..2d6d8ed 100644 --- a/arch/x86/mm/tlb.c +++ b/arch/x86/mm/tlb.c @@ -98,6 +98,8 @@ static void flush_tlb_func(void *info) { struct flush_tlb_info *f = info; + inc_irq_stat(irq_tlb_count); + if (f->flush_mm != this_cpu_read(cpu_tlbstate.active_mm)) return; ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] x86: Distinguish TLB shootdown interrupts from other functions call interrupts 2012-09-20 8:50 ` Tomoki Sekiyama @ 2012-09-21 8:08 ` Alex Shi 2012-09-24 1:37 ` Alex Shi 1 sibling, 0 replies; 11+ messages in thread From: Alex Shi @ 2012-09-21 8:08 UTC (permalink / raw) To: Tomoki Sekiyama Cc: x86, yrl.pp-manager.tt, linux-kernel, Ingo Molnar, H. Peter Anvin, Thomas Gleixner >> If is it better to move above explanation to irq_call_cnt definition place: harirq.h? > > Agreed. > > In the patch below, I reduced the redundant comments. Acked-by: Alex Shi <alex.shi@intel.com> > > -- > As TLB shootdown requests to other CPU cores are now using function call > interrupts, TLB shootdowns entry in /proc/interrupts is always shown as 0. > > This behavior change was introduced by commit 52aec3308db8 ("x86/tlb: > replace INVALIDATE_TLB_VECTOR by CALL_FUNCTION_VECTOR"). > > This patch reverts TLB shootdowns entry in /proc/interrupts to count TLB > shootdowns separately from the other function call interrupts. > > Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama.qu@hitachi.com> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: "H. Peter Anvin" <hpa@zytor.com> > Cc: Alex Shi <alex.shi@intel.com> > --- > arch/x86/include/asm/hardirq.h | 2 ++ > arch/x86/kernel/irq.c | 4 ++-- > arch/x86/mm/tlb.c | 2 ++ > 3 files changed, 6 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/include/asm/hardirq.h b/arch/x86/include/asm/hardirq.h > index d3895db..e34b252 100644 > --- a/arch/x86/include/asm/hardirq.h > +++ b/arch/x86/include/asm/hardirq.h > @@ -18,6 +18,8 @@ typedef struct { > #ifdef CONFIG_SMP > unsigned int irq_resched_count; > unsigned int irq_call_count; > + /* irq_tlb_count is double-counted in irq_call_count, so it must be > + subtracted from irq_call_count when displaying irq_call_count */ > unsigned int irq_tlb_count; > #endif > #ifdef CONFIG_X86_THERMAL_VECTOR > diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c > index d44f782..e4595f1 100644 > --- a/arch/x86/kernel/irq.c > +++ b/arch/x86/kernel/irq.c > @@ -92,7 +92,8 @@ int arch_show_interrupts(struct seq_file *p, int prec) > seq_printf(p, " Rescheduling interrupts\n"); > seq_printf(p, "%*s: ", prec, "CAL"); > for_each_online_cpu(j) > - seq_printf(p, "%10u ", irq_stats(j)->irq_call_count); > + seq_printf(p, "%10u ", irq_stats(j)->irq_call_count - > + irq_stats(j)->irq_tlb_count); > seq_printf(p, " Function call interrupts\n"); > seq_printf(p, "%*s: ", prec, "TLB"); > for_each_online_cpu(j) > @@ -147,7 +148,6 @@ u64 arch_irq_stat_cpu(unsigned int cpu) > #ifdef CONFIG_SMP > sum += irq_stats(cpu)->irq_resched_count; > sum += irq_stats(cpu)->irq_call_count; > - sum += irq_stats(cpu)->irq_tlb_count; > #endif > #ifdef CONFIG_X86_THERMAL_VECTOR > sum += irq_stats(cpu)->irq_thermal_count; > diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c > index 613cd83..2d6d8ed 100644 > --- a/arch/x86/mm/tlb.c > +++ b/arch/x86/mm/tlb.c > @@ -98,6 +98,8 @@ static void flush_tlb_func(void *info) > { > struct flush_tlb_info *f = info; > > + inc_irq_stat(irq_tlb_count); > + > if (f->flush_mm != this_cpu_read(cpu_tlbstate.active_mm)) > return; > > > -- Thanks Alex ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] x86: Distinguish TLB shootdown interrupts from other functions call interrupts 2012-09-20 8:50 ` Tomoki Sekiyama 2012-09-21 8:08 ` Alex Shi @ 2012-09-24 1:37 ` Alex Shi 1 sibling, 0 replies; 11+ messages in thread From: Alex Shi @ 2012-09-24 1:37 UTC (permalink / raw) To: Tomoki Sekiyama Cc: x86, yrl.pp-manager.tt, linux-kernel, Ingo Molnar, H. Peter Anvin, Thomas Gleixner On 09/20/2012 04:50 PM, Tomoki Sekiyama wrote: > unsigned int irq_resched_count; > unsigned int irq_call_count; > + /* irq_tlb_count is double-counted in irq_call_count, so it must be > + subtracted from irq_call_count when displaying irq_call_count */ > unsigned int irq_tlb_count; Review again this patch, above comments is not kernel compatible format. Could you change it like standard comment format: /* * xxxxxxx * xxxx */ -- Thanks Alex ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2012-09-28 6:14 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <50611D82.4010702@intel.com>
2012-09-26 2:11 ` [PATCH] x86: Distinguish TLB shootdown interrupts from other functions call interrupts Tomoki Sekiyama
2012-09-26 2:17 ` Alex Shi
2012-09-27 7:02 ` Alex Shi
2012-09-28 5:49 ` H. Peter Anvin
2012-09-28 6:08 ` Alex Shi
2012-09-28 6:13 ` [tip:x86/mm] " tip-bot for Tomoki Sekiyama
2012-09-19 9:04 [PATCH] " Tomoki Sekiyama
2012-09-20 1:18 ` Alex Shi
2012-09-20 8:50 ` Tomoki Sekiyama
2012-09-21 8:08 ` Alex Shi
2012-09-24 1:37 ` Alex Shi
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).