Re: [PATCH] xen/events/fifo: Consume unprocessed events when a CPU dies

[Ross Lagerwall <ross.lagerwall@citrix.com>]

On 06/19/2015 05:06 PM, David Vrabel wrote:
> On 19/06/15 17:02, Boris Ostrovsky wrote:
>> On 06/19/2015 11:15 AM, Ross Lagerwall wrote:
>>> When a CPU is offlined, there may be unprocessed events on a port for
>>> that CPU.  If the port is subsequently reused on a different CPU, it
>>> could be in an unexpected state with the link bit set, resulting in
>>> interrupts being missed. Fix this by consuming any unprocessed events
>>> for a particular CPU when that CPU dies.
>>>
>>> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
>>> ---
>>>    drivers/xen/events/events_fifo.c | 23 ++++++++++++++++++-----
>>>    1 file changed, 18 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/xen/events/events_fifo.c
>>> b/drivers/xen/events/events_fifo.c
>>> index 417415d..1dd0ba12 100644
>>> --- a/drivers/xen/events/events_fifo.c
>>> +++ b/drivers/xen/events/events_fifo.c
>>> @@ -281,7 +281,8 @@ static void handle_irq_for_port(unsigned port)
>>>      static void consume_one_event(unsigned cpu,
>>>                      struct evtchn_fifo_control_block *control_block,
>>> -                  unsigned priority, unsigned long *ready)
>>> +                  unsigned priority, unsigned long *ready,
>>> +                  bool drop)
>>>    {
>>>        struct evtchn_fifo_queue *q = &per_cpu(cpu_queue, cpu);
>>>        uint32_t head;
>>> @@ -313,13 +314,17 @@ static void consume_one_event(unsigned cpu,
>>>        if (head == 0)
>>>            clear_bit(priority, ready);
>>>    -    if (evtchn_fifo_is_pending(port) && !evtchn_fifo_is_masked(port))
>>> -        handle_irq_for_port(port);
>>> +    if (evtchn_fifo_is_pending(port) && !evtchn_fifo_is_masked(port)) {
>>> +        if (unlikely(drop))
>>> +            pr_warn("Dropping pending event for port %u\n", port);
>>
>> Maybe pr_info (or pr_notice)?
>
> We want a warning here because we think this shouldn't happen -- if it
> does we actually need to retrigger the event on its new CPU.
>
>> Also, why not do this (testing for unprocessed events) in
>> xen_evtchn_close()?
>
> We can't do anything about them when closing because they may be in the
> middle of a queue.
>
> David
>

Ping. Is this change OK?

-- 
Ross Lagerwall