Re: [PATCH] evtchn/fifo: don't corrupt queues if an old tail is linked

[David Vrabel <david.vrabel@citrix.com>]

On 09/12/13 09:32, Jan Beulich wrote:
>>>> On 06.12.13 at 18:38, David Vrabel <david.vrabel@citrix.com> wrote:
>> --- a/xen/include/xen/sched.h
>> +++ b/xen/include/xen/sched.h
>> @@ -98,6 +98,8 @@ struct evtchn
>>      } u;
>>      u8 priority;
>>      u8 pending:1;
>> +    u16 last_vcpu_id;
>> +    u8 last_priority;
> 
> Is it really correct for these two new fields to remain uninitialized
> until evtchn_fifo_set_pending() would get run the first time (and
> hence thinking there was a move this first time through)?

They're initialized to zero and I think this is fine. The code as-is is
simpler than having to special case events that have never been on a queue.

> Which also gets me to ask whether it's really correct to only set
> the priority to EVTCHN_FIFO_PRIORITY_DEFAULT in setup_ports(),
> but not on any subsequently allocated/bound ones?

This patch fixes this but would you prefer a new evtchn_port_op hook for
the init?

--- a/xen/common/event_channel.c
+++ b/xen/common/event_channel.c
@@ -141,6 +141,7 @@ static struct evtchn *alloc_evtchn_bucket(struct
domain *d, unsigned int port)
             return NULL;
         }
         chn[i].port = port + i;
+        chn[i].priority = EVTCHN_FIFO_PRIORITY_DEFAULT;
     }
     return chn;
 }
diff --git a/xen/common/event_fifo.c b/xen/common/event_fifo.c
index b29297f..394879e 100644
--- a/xen/common/event_fifo.c
+++ b/xen/common/event_fifo.c
@@ -444,7 +444,6 @@ static void setup_ports(struct domain *d)
      * For each port that is already bound:
      *
      * - save its pending state.
-     * - set default priority.
      */
     for ( port = 1; port < d->max_evtchns; port++ )
     {
@@ -457,8 +456,6 @@ static void setup_ports(struct domain *d)

         if ( test_bit(port, &shared_info(d, evtchn_pending)) )
             evtchn->pending = 1;
-
-        evtchn_fifo_set_priority(d, evtchn, EVTCHN_FIFO_PRIORITY_DEFAULT);
     }
 }

David