Re: [Xen-devel] [PATCH] xen-evtchn: Bind dyn evtchn:qemu-dm interrupt to next online VCPU

From: Anoob Soman
Date: Mon Jun 05 2017 - 10:49:53 EST


On 05/06/17 15:10, Boris Ostrovsky wrote:
The reason for percpu instead of global, was to avoid locking. We can
have a global variable (last_cpu) without locking, but value of
last_cpu wont be consistent, without locks. Moreover, since
irq_affinity is also used in the calculation of cpu to bind, having a
percpu or global wouldn't really matter, as the result (selected_cpu)
is more likely to be random (because different irqs can have different
affinity). What do you guys suggest.
Doesn't initial affinity (which is what we expect here since irqbalance
has not run yet) typically cover all guest VCPUs?

Yes, initial affinity covers all online VCPUs. But there is a small chance that initial affinity might change, before evtch_bind_interdom_next_vcpu is called. For example, I could run a script to change irq affinity, just when irq sysfs entry appears. This is the reason that I thought it would be sensible (based on your suggestion) to include irq_affinity to calculate the next VCPU. If you think, changing irq_affinity between request_irq() and evtch_bind_interdom_next_vcpu is virtually impossible, then we can drop affinity and just use cpu_online_mask.


I think we would still require spin_lock(). spin_lock is for irq_desc.
If you are trying to protect affinity then it may well change after you
drop the lock.

In fact, don't you have a race here? If we offline a VCPU we will (by
way of cpu_disable_common()->fixup_irqs()) update affinity to reflect
that a CPU is gone and there is a chance that xen_rebind_evtchn_to_cpu()
will happen after that.

So, contrary to what I said earlier ;-) not only do you need the lock,
but you should hold it across xen_rebind_evtchn_to_cpu() call. Does this
make sense?

Yes, you are correct. .irq_set_affinity pretty much does the same thing.

The code will now looks like this.
raw_spin_lock_irqsave(lock, flags);
percpu read
select_cpu
percpu write
xen_rebind_evtchn_to_cpu(evtchn, selected_cpu)
raw_spin_unlock_irqsave(lock, flags);