[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 3/3] evtchn/fifo: don't spin indefinitely when setting LINK

>>> On 04.11.13 at 17:30, David Vrabel <david.vrabel@xxxxxxxxxx> wrote:
> On 04/11/13 14:57, Jan Beulich wrote:
>>>>> On 04.11.13 at 15:52, David Vrabel <david.vrabel@xxxxxxxxxx> wrote:
>>> On 04/11/13 14:39, Jan Beulich wrote:
>>>>>>> On 31.10.13 at 16:03, David Vrabel <david.vrabel@xxxxxxxxxx> wrote:
>>>>>          event_word_t *tail_word;
>>>>>          bool_t linked = 0;
>>>>> +        evtchn->q = q;
>>>>> +
>>>>>          spin_lock_irqsave(&q->lock, flags);
>>>>>          /*
>>>> I fail to see how this change is related to the rest of the patch.
>>> This is needed so the correct queue lock is used in evtchn_fifo_unmask().
>> I guessed that, but I wasn't able to immediately see why that would
>> not have been a requirement before.
> An event now has two queues associated with it.
> 1. The queue it is supposed to be on.  This is found with
> evtchn->notify_vcpu_id and evtchn->priority when an event is being linked.
> 2. The queue the event is on right now (the new evtchn->q), until this
> patch we didn't care which queue an event was on, only that it was on
> some queue.
> These are not necessarily the same since an event may be moved between
> VCPUs or have its priority changed while it is already on a queue.
> Some additional notes on the locking:
> The locking in the FIFO-based ABI is designed on the assumption that
> every hypervisor operation on an event channel (set pending, unmask) is
> done while holding a per-event channel spin lock.
> Currently, the per-event locking is done with the coarser per-domain
> event_lock.
> The queue lock serializes adding events and clearing MASKED on tail events.
> The event lock also protects evtchn->q and the evtchn->q->tail ==
> evtchn->port test.

But aren't you setting evtchn->q too late then?
evtchn_fifo_unmask() racing evtchn_fifo_set_pending() could
then lead to the latter seeing ->q still clear, the former setting it
and doing its linking work while in parallel the latter might clear
MASKED at any time.

Plus - shouldn't you clear ->q at some point again?


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.