lkml.org 
[lkml]   [2020]   [Aug]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [patch V9 21/39] x86/irq: Convey vector as argument and not in ptregs
From
Date
Hi Thomas,

On 25.08.20 12:28, Thomas Gleixner wrote:
>
> Alex,
>
> On Mon, Aug 24 2020 at 19:29, Alexander Graf wrote:
>> I'm currently trying to understand a performance regression with
>> ScyllaDB on i3en.3xlarge (KVM based VM on Skylake) which we reliably
>> bisected down to this commit:
>>
>> https://github.com/scylladb/scylla/issues/7036
>>
>> What we're seeing is that syscalls such as membarrier() take forever
>> (0-10 µs would be normal):
> ...
>> That again seems to stem from a vastly slowed down
>> smp_call_function_many_cond():
>>
>> Samples: 218K of event 'cpu-clock', 4000 Hz
>> Overhead Shared Object Symbol
>> 94.51% [kernel] [k] smp_call_function_many_cond
>> 0.76% [kernel] [k] __do_softirq
>> 0.32% [kernel] [k] native_queued_spin_lock_slowpath
>> [...]
>>
>> which is stuck in
>>
>> │ csd_lock_wait():
>> │ smp_cond_load_acquire(&csd->flags, !(VAL &
>> 0.00 │ mov 0x8(%rcx),%edx
>> 0.00 │ and $0x1,%edx
>> │ ↓ je 2b9
>> │ rep_nop():
>> 0.70 │2af: pause
>> │ csd_lock_wait():
>> 92.82 │ mov 0x8(%rcx),%edx
>> 6.48 │ and $0x1,%edx
>> 0.00 │ ↑ jne 2af
>> 0.00 │2b9: ↑ jmp 282
>>
>>
>> Given the patch at hand I was expecting lost IPIs, but I can't quite see
>> anything getting lost.
>
> I have no idea how that patch should be related to IPI and smp function
> calls. It's changing the way how regular device interrupts and their
> spurious counterpart are handled and not the way how IPIs are
> handled. They are handled by direct vectors and do not go through
> do_IRQ() at all.
>
> Aside of that the commit just changes the way how the interrupt vector
> of a regular device interrupt is stored and conveyed. The extra read and
> write on the cache hot stack is hardly related to anything IPI.

I am as puzzled as you are, but the bisect is very clear: 79b9c183021e
works fast and 633260fa1 (as well as mainline) shows the weird behavior
above.

It gets even better. This small (demonstrative only, mangled) patch on
top of 633260fa1 also resolves the performance issue:

diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c
index c766936..7e91e9a 100644
--- a/arch/x86/kernel/irq.c
+++ b/arch/x86/kernel/irq.c
@@ -239,6 +239,7 @@ __visible void __irq_entry do_IRQ(struct pt_regs
*regs, unsigned long vector)
* lower 8 bits.
*/
vector &= 0xFF;
+ regs->orig_ax = ~vector;

/* entering_irq() tells RCU that we're not quiescent. Check it. */
RCU_LOCKDEP_WARN(!rcu_is_watching(), "IRQ failed to wake up RCU");

To me that sounds like some irq exit code somewhere must still be
looking at orig_ax to decide on something - and that something is wrong
now that we removed the negation of the vector. It also seems to have an
impact on remote function calls.

I'll have a deeper look tomorrow again if I can find any such place, but
I wouldn't mind if anyone could point me into the right direction
earlier :).


Alex



Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879


\
 
 \ /
  Last update: 2020-08-26 01:19    [W:0.115 / U:0.012 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site