Re: [Xen-devel] [for-xen-4.5 PATCH v2 2/2] dpci: Add ZOMBIE state to allow the softirq to finish with the dpci_pirq.

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 21.11.14 at 16:14, <konrad.wilk@xxxxxxxxxx> wrote:
> On Fri, Nov 21, 2014 at 01:03:43PM +0100, Jan Beulich wrote:
>> >>> On 21.11.14 at 12:50, <konrad.wilk@xxxxxxxxxx> wrote:
>> > On November 21, 2014 2:51:33 AM EST, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>> >>>>> On 20.11.14 at 20:51, <konrad.wilk@xxxxxxxxxx> wrote:
>> >>> @@ -669,7 +670,7 @@ static void hvm_dirq_assist(struct domain *d,
>> >>struct hvm_pirq_dpci *pirq_dpci)
>> >>>      ASSERT(d->arch.hvm_domain.irq.dpci);
>> >>>  
>> >>>      spin_lock(&d->event_lock);
>> >>> -    if ( pirq_dpci->state )
>> >>> +    if ( test_and_clear_bool(pirq_dpci->masked) )
>> >>>      {
>> >>>          struct pirq *pirq = dpci_pirq(pirq_dpci);
>> >>>          const struct dev_intx_gsi_link *digl;
>> >>
>> >>So this now guards solely against the timeout enforced EOI? Why do
>> >>you no longer need to guard against cancellation (i.e. why isn't this
>> >>looking at both ->state and ->masked)?
>> >>
>> > 
>> > The previous state check was superfluous as the dpci_softirq would check 
>> > for 
>> > the valid STATE_ before calling hvm_dirq_assist (and deal with 
>> > cancellation).
>> 
>> I thought so first too, but then how is the guarding against
>> cancellation working now?
> 
> Since there are two forms of cancellation, lets consider each one seperatly:
> 
> 1). pt_irq_create_bind and pt_irq_destroy_bind. Each of them calling 
>     pt_pirq_softirq_reset in the 'error' case or in the normal path of 
> destruction.
>     When that happens the action handler for the IRQ is removed the moment we 
> call
>     pirq_guest_unbind. As such we only have to deal with the potential 
> outstanding
>     pirq_dpci waiting to be serviced. Since we did the 'pt_pirq_softirq_reset'
>     we have changed the state from STATE_SCHED to zero. That means 
> dpci_softirq will
>     NOT go in:
>       if ( test_and_clear_bit(STATE_SCHED, &pirq_dpci->state) )               

Unless the flag got set again in the meantime. Since the event lock
is being acquired right before the line quoted above, _that_ is
what needs closely looking at (and why I was asking about the
deletion of the [at the first glance unnecessary] checking of ->state
in hvm_dirq_assist()).

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel