mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Aaron Lu <aaron.lu@intel.com>
To: x86@kernel.org, linux-kernel@vger.kernel.org
Cc: Alex Shi <alex.shi@linaro.org>,
	Tomoki Sekiyama <tomoki.sekiyama.qu@hitachi.com>,
	Davidlohr Bueso <dave@stgolabs.net>,
	Huang Ying <ying.huang@intel.com>, Ingo Molnar <mingo@redhat.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	"\"H. Peter Anvin\"" <hpa@zytor.com>
Subject: [PATCH] x86/irq: do not substract irq_tlb_count from irq_call_count
Date: Thu, 11 Aug 2016 15:44:30 +0800	[thread overview]
Message-ID: <20160811074430.GA18163@aaronlu.sh.intel.com> (raw)

Since commit 52aec3308db8 ("x86/tlb: replace INVALIDATE_TLB_VECTOR by
CALL_FUNCTION_VECTOR"), the tlb remote shootdown is done through call
function vector. That commit didn't take care of irq_tlb_count so later
commit fd0f5869724f ("x86: Distinguish TLB shootdown interrupts from
other functions call interrupts") tried to fix it.

The fix assumes every increase of irq_tlb_count has a corresponding
increase of irq_call_count. So the irq_call_count is always bigger than
irq_tlb_count and we could substract irq_tlb_count from irq_call_count.

Unfortunately this is not true for the smp_call_function_single case.
The IPI is only sent if the target CPU's call_single_queue is empty when
adding a csd into it in generic_exec_single. That means if two threads
are both adding flush tlb csds to the same CPU's call_single_queue, only
one IPI is sent. In other words, the irq_call_count is incremented by 1
but irq_tlb_count is incremented by 2. Over time, irq_tlb_count will be
bigger than irq_call_count and the substract will produce a very large
irq_call_count value due to overflow.

Considering that:
1 it's not worth to send more IPIs for the sake of accurate counting of
  irq_call_count in generic_exec_single;
2 it's not easy to tell if the call function interrupt is for TLB
  shootdown in __smp_call_function_single_interrupt.
Not to exclude TLB shootdown from call function count seems to be the
simplest fix and this patch just did that.

This is found by LKP's cyclic performance regression tracking recently
with the vm-scalability test suite. I have bisected to commit
0a7ce4b5a632 ("mm/rmap: share the i_mmap_rwsem"). This commit didn't do
anything wrong but revealed the irq_call_count problem. IIUC, the commit
makes rwc->remap_one in rmap_walk_file concurrent with multiple threads.
When remap_one is try_to_unmap_one, then multiple threads could queue
flush tlb to the same CPU but only one IPI will be sent.

Since the commit enter Linux v3.19, the counting problem only shows up
from v3.19. Considering this is a behaviour change, I'm not sure if I
should add the stable tag here.

Signed-off-by: Aaron Lu <aaron.lu@intel.com>
---
 arch/x86/include/asm/hardirq.h | 4 ----
 arch/x86/kernel/irq.c          | 3 +--
 2 files changed, 1 insertion(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/hardirq.h b/arch/x86/include/asm/hardirq.h
index 7178043b0e1d..59405a248fc2 100644
--- a/arch/x86/include/asm/hardirq.h
+++ b/arch/x86/include/asm/hardirq.h
@@ -22,10 +22,6 @@ 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 61521dc19c10..9f669fdd2010 100644
--- a/arch/x86/kernel/irq.c
+++ b/arch/x86/kernel/irq.c
@@ -102,8 +102,7 @@ int arch_show_interrupts(struct seq_file *p, int prec)
 	seq_puts(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 -
-					irq_stats(j)->irq_tlb_count);
+		seq_printf(p, "%10u ", irq_stats(j)->irq_call_count);
 	seq_puts(p, "  Function call interrupts\n");
 	seq_printf(p, "%*s: ", prec, "TLB");
 	for_each_online_cpu(j)
-- 
2.5.5

             reply	other threads:[~2016-08-11  7:44 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-11  7:44 Aaron Lu [this message]
2016-08-11  9:13 ` Ingo Molnar
2016-08-11  9:16   ` Aaron Lu
2016-08-11 11:57 ` [tip:x86/urgent] x86/irq: Do " tip-bot for Aaron Lu
2016-08-11 15:13 ` [PATCH] x86/irq: do " Huang, Ying
2016-08-12  2:16   ` Aaron Lu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160811074430.GA18163@aaronlu.sh.intel.com \
    --to=aaron.lu@intel.com \
    --cc=alex.shi@linaro.org \
    --cc=dave@stgolabs.net \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=tomoki.sekiyama.qu@hitachi.com \
    --cc=x86@kernel.org \
    --cc=ying.huang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome