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 19.11.14 at 23:21, <konrad.wilk@xxxxxxxxxx> wrote:

Leaving aside the question of whether this is the right approach, in
case it is a couple of comments:

> @@ -85,7 +91,7 @@ static void raise_softirq_for(struct hvm_pirq_dpci 
> *pirq_dpci)
>   */
>  bool_t pt_pirq_softirq_active(struct hvm_pirq_dpci *pirq_dpci)
>  {
> -    if ( pirq_dpci->state & ((1 << STATE_RUN) | (1 << STATE_SCHED)) )
> +    if ( pirq_dpci->state & ((1 << STATE_RUN) | (1 << STATE_SCHED) | (1 << 
> STATE_ZOMBIE) ) )

This is becoming unwieldy - perhaps better just "if ( pirq_dpci->state )"?

> @@ -122,6 +128,7 @@ static void pt_pirq_softirq_cancel(struct hvm_pirq_dpci 
> *pirq_dpci,
>          /* fallthrough. */
>      case (1 << STATE_RUN):
>      case (1 << STATE_RUN) | (1 << STATE_SCHED):
> +    case (1 << STATE_RUN) | (1 << STATE_SCHED) | (1 << STATE_ZOMBIE):

How would we get into this state? Afaict STATE_ZOMBIE can't be set
at the same time as STATE_SCHED.

> @@ -786,6 +793,7 @@ unlock:
>  static void dpci_softirq(void)
>  {
>      unsigned int cpu = smp_processor_id();
> +    unsigned int reset = 0;

bool_t (and would better be inside the loop, avoiding the pointless
re-init on the last iteration).

But taking it as a whole - aren't we now at risk of losing an interrupt
instance the guest expects, due to raise_softirq_for() bailing on
zombie entries, and dpci_softirq() being the only place where the
zombie state gets cleared?

Jan


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