Re: [PATCH v2 02/14] KVM: x86: simplify atomics in kvm_pit_ack_irq

[Paolo Bonzini]

On 17/02/2016 20:14, Radim KrÄmÃÅ wrote:
> -	value = 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 = 1;
	spin_unlock(...)
	
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