lkml.org 
[lkml]   [2017]   [Jun]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [Xen-devel] [PATCH] xen-evtchn: Bind dyn evtchn:qemu-dm interrupt to next online VCPU
From
Date
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);

\
 
 \ /
  Last update: 2017-06-12 00:23    [W:0.388 / U:0.016 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site