Re: [Xen-devel] [RFC PATCH] dpci: Put the dpci back on the list if running on another CPU.

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 12.01.15 at 17:45, <konrad.wilk@xxxxxxxxxx> wrote:
> There is race when we clear the STATE_SCHED in the softirq
> - which allows the 'raise_softirq_for' to progress and
> schedule another dpci. During that time the other CPU could
> receive an interrupt and calls 'raise_softirq_for' and put
> the dpci on its per-cpu list. There would be two 'dpci_softirq'
> running at the same time (on different CPUs) where the
> dpci state is STATE_RUN (and STATE_SCHED is cleared). This
> ends up hitting:
> 
>  if ( test_and_set_bit(STATE_RUN, &pirq_dpci->state) )
>       BUG()
> 
> Instead of that put the dpci back on the per-cpu list to deal
> with later.
> 
> The reason we can get his with this is when an interrupt
> affinity is set over multiple CPUs.
> 
> Another potential fix would be to add a guard in the raise_softirq_for
> to check for 'STATE_RUN' bit being set and not schedule the
> dpci until that bit has been cleared.

I indeed think this should be investigated, because it would make
explicit what ...

> --- a/xen/drivers/passthrough/io.c
> +++ b/xen/drivers/passthrough/io.c
> @@ -804,7 +804,17 @@ static void dpci_softirq(void)
>          d = pirq_dpci->dom;
>          smp_mb(); /* 'd' MUST be saved before we set/clear the bits. */
>          if ( test_and_set_bit(STATE_RUN, &pirq_dpci->state) )
> -            BUG();
> +        {
> +            unsigned long flags;
> +
> +            /* Put back on the list and retry. */
> +            local_irq_save(flags);
> +            list_add_tail(&pirq_dpci->softirq_list, &this_cpu(dpci_list));
> +            local_irq_restore(flags);
> +
> +            raise_softirq(HVM_DPCI_SOFTIRQ);
> +            continue;
> +        }

... this does implicitly - spin until the bad condition cleared.

Additionally I think it should be considered whether the bitmap
approach of interpreting ->state is the right one, and we don't
instead want a clean 3-state (idle, sched, run) model.

Jan


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