lkml.org 
[lkml]   [2015]   [Apr]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [RFC PATCH 5/5] GHES: Make NMI handler have a single reader
Date
Hi,

> From: Borislav Petkov [mailto:bp@alien8.de]
> Sent: Tuesday, April 28, 2015 9:59 PM
> Subject: Re: [RFC PATCH 5/5] GHES: Make NMI handler have a single reader
>
> On Tue, Apr 28, 2015 at 01:38:41PM +0000, Zheng, Lv wrote:
> > > - raw_spin_lock(&ghes_nmi_lock);
> > > + if (!atomic_add_unless(&ghes_in_nmi, 1, 1))
> > > + return ret;
> > > +
> >
> > if (atomic_cmpxchg(&ghes_in_nmi, 0, 1))
> > return ret;
>
> Ok, now I understand what you mean.
>
> We absolutely want to use atomic_add_unless() because we get to save us
> the expensive
>
> LOCK; CMPXCHG
>
> if the value was already 1. Which is exactly what this patch is trying
> to avoid - a thundering herd of cores CMPXCHGing a global variable.

IMO, on most architectures, the "cmp" part should work just like what you've done with "if".
And on some architectures, if the "xchg" doesn't happen, the "cmp" part even won't cause a pipe line hazard.

Thanks and best regards
-Lv


>
> I.e.,
>
> movl ghes_in_nmi(%rip), %ecx # MEM[(const int *)&ghes_in_nmi], c
> cmpl $1, %ecx #, c
> je .L311 #, <--- exit here if ghes_in_nmi == 1.
> leal 1(%rcx), %edx #, D.37163
> movl %ecx, %eax # c, c
> #APP
> # 177 "./arch/x86/include/asm/atomic.h" 1
> .pushsection .smp_locks,"a"
> .balign 4
> .long 671f - .
> .popsection
> 671:
> lock; cmpxchgl %edx,ghes_in_nmi(%rip) # D.37163, MEM[(volatile u32 *)&ghes_in_nmi]
> # 0 "" 2
> #NO_APP
>
> --
> Regards/Gruss,
> Boris.
>
> ECO tip #101: Trim your mails when you reply.
> --
\
 
 \ /
  Last update: 2015-04-29 02:41    [W:0.193 / U:0.032 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site