Hi, > From: Zheng, Lv > Sent: Wednesday, April 29, 2015 8:25 AM > Subject: RE: [RFC PATCH 5/5] GHES: Make NMI handler have a single reader > > Hi, > > > From: Borislav Petkov [mailto:bp@xxxxxxxxx] > > 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. If you man the LOCK prefix, I understand now. Thanks and best regards -Lv > > 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. > > -- ��.n��������+%������w��{.n�����{�����ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
