From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Bonzini Subject: Re: [PATCH v2 02/14] KVM: x86: simplify atomics in kvm_pit_ack_irq Date: Thu, 18 Feb 2016 19:04:43 +0100 Message-ID: <56C607BB.3000103@redhat.com> References: <1455736496-374-1-git-send-email-rkrcmar@redhat.com> <1455736496-374-3-git-send-email-rkrcmar@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: kvm@vger.kernel.org, Yuki Shibuya To: =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , linux-kernel@vger.kernel.org Return-path: In-Reply-To: <1455736496-374-3-git-send-email-rkrcmar@redhat.com> Sender: linux-kernel-owner@vger.kernel.org List-Id: kvm.vger.kernel.org On 17/02/2016 20:14, Radim Kr=C4=8Dm=C3=A1=C5=99 wrote: > - value =3D atomic_dec_return(&ps->pending); > - if (value < 0) > - /* spurious acks can be generated if, for example, the > - * PIC is being reset. Handle it gracefully here > - */ > - atomic_inc(&ps->pending); > - else if (value > 0 && ps->reinject) > - /* in this case, we had multiple outstanding pit interrupts > - * that we needed to inject. Reinject > - */ > + if (atomic_dec_if_positive(&ps->pending) > 0 && ps->reinject) > queue_kthread_work(&ps->pit->worker, &ps->pit->expired); Here it would have made sense to do already if (!ps->reinject) { WARN_ON_ONCE(ps->pending || !ps->irq_ack); return; } spin_lock(...) if (atomic_dec_if_positive(&ps->pending) > 0) queue_kthread_work(...); ps->irq_ack =3D 1; spin_unlock(...) =09 because ps->pending is only ever nonzero, and irq_ack is only ever zero= , if ps->reinject. Not a big deal since the ack notifier is going to disappear altogether for the discard policy, but the nice thing is that it lets you remove the ack notifier earlier and disentangle a bit more discard mode. So if you want for v3 you can reorder the patches like this: - patch 1, same - patch 2, what is outlined above - patch 3, remove ack notifier for discard - patch 4..14 the rest Paolo