Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1657678

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

Show all headers | View raw

On 06/05/2017 10:49 AM, Anoob Soman wrote:
> 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 believe we do need to take affinity into consideration even if the
chance that it is non-default is small.

I am not opposed to having bind_last_selected_cpu percpu, I just wanted
to understand the reason better. Additional locking would be a downside
with a global so if you feel that percpu is worth it then I won't object.

>
>>>
>>> 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);

(BTW, I just noticed --- you don't need to initialize desc)

-boris

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH] xen-evtchn: Bind dyn evtchn:qemu-dm interrupt to next online VCPU Anoob Soman <anoob.soman@citrix.com> - 2017-06-02 17:00 +0200
  Re: [PATCH] xen-evtchn: Bind dyn evtchn:qemu-dm interrupt to next  online VCPU Juergen Gross <jgross@suse.com> - 2017-06-02 17:20 +0200
    Re: [PATCH] xen-evtchn: Bind dyn evtchn:qemu-dm interrupt to next  online VCPU Anoob Soman <anoob.soman@citrix.com> - 2017-06-05 12:00 +0200
  Re: [PATCH] xen-evtchn: Bind dyn evtchn:qemu-dm interrupt to next  online VCPU Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2017-06-02 18:30 +0200
    Re: [PATCH] xen-evtchn: Bind dyn evtchn:qemu-dm interrupt to next  online VCPU Anoob Soman <anoob.soman@citrix.com> - 2017-06-05 12:20 +0200
      Re: [PATCH] xen-evtchn: Bind dyn evtchn:qemu-dm interrupt to next  online VCPU Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2017-06-05 16:20 +0200
        Re: [Xen-devel] [PATCH] xen-evtchn: Bind dyn evtchn:qemu-dm interrupt  to next online VCPU Anoob Soman <anoob.soman@citrix.com> - 2017-06-05 16:50 +0200
          Re: [Xen-devel] [PATCH] xen-evtchn: Bind dyn evtchn:qemu-dm interrupt  to next online VCPU Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2017-06-05 17:40 +0200
            Re: [Xen-devel] [PATCH] xen-evtchn: Bind dyn evtchn:qemu-dm interrupt  to next online VCPU Anoob Soman <anoob.soman@citrix.com> - 2017-06-05 18:20 +0200
              Re: [Xen-devel] [PATCH] xen-evtchn: Bind dyn evtchn:qemu-dm interrupt  to next online VCPU Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2017-06-05 18:50 +0200
                Re: [Xen-devel] [PATCH] xen-evtchn: Bind dyn evtchn:qemu-dm interrupt  to next online VCPU Anoob Soman <anoob.soman@citrix.com> - 2017-06-05 19:20 +0200

csiph-web