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

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

[Ross Lagerwall]

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);
+		else
+			handle_irq_for_port(port);
+	}
 
 	q->head[priority] = head;
 }
 
-static void evtchn_fifo_handle_events(unsigned cpu)
+static void __evtchn_fifo_handle_events(unsigned cpu, bool drop)
 {
 	struct evtchn_fifo_control_block *control_block;
 	unsigned long ready;
@@ -331,11 +336,16 @@ static void evtchn_fifo_handle_events(unsigned cpu)
 
 	while (ready) {
 		q = find_first_bit(&ready, EVTCHN_FIFO_MAX_QUEUES);
-		consume_one_event(cpu, control_block, q, &ready);
+		consume_one_event(cpu, control_block, q, &ready, drop);
 		ready |= xchg(&control_block->ready, 0);
 	}
 }
 
+static void evtchn_fifo_handle_events(unsigned cpu)
+{
+	__evtchn_fifo_handle_events(cpu, false);
+}
+
 static void evtchn_fifo_resume(void)
 {
 	unsigned cpu;
@@ -420,6 +430,9 @@ static int evtchn_fifo_cpu_notification(struct notifier_block *self,
 		if (!per_cpu(cpu_control_block, cpu))
 			ret = evtchn_fifo_alloc_control_block(cpu);
 		break;
+	case CPU_DEAD:
+		__evtchn_fifo_handle_events(cpu, true);
+		break;
 	default:
 		break;
 	}
-- 
2.1.0

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

[Boris Ostrovsky]

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)?

Also, why not do this (testing for unprocessed events) in 
xen_evtchn_close()?

-boris

> +		else
> +			handle_irq_for_port(port);
> +	}
>   
>   	q->head[priority] = head;
>   }
>   
> -static void evtchn_fifo_handle_events(unsigned cpu)
> +static void __evtchn_fifo_handle_events(unsigned cpu, bool drop)
>   {
>   	struct evtchn_fifo_control_block *control_block;
>   	unsigned long ready;
> @@ -331,11 +336,16 @@ static void evtchn_fifo_handle_events(unsigned cpu)
>   
>   	while (ready) {
>   		q = find_first_bit(&ready, EVTCHN_FIFO_MAX_QUEUES);
> -		consume_one_event(cpu, control_block, q, &ready);
> +		consume_one_event(cpu, control_block, q, &ready, drop);
>   		ready |= xchg(&control_block->ready, 0);
>   	}
>   }
>   
> +static void evtchn_fifo_handle_events(unsigned cpu)
> +{
> +	__evtchn_fifo_handle_events(cpu, false);
> +}
> +
>   static void evtchn_fifo_resume(void)
>   {
>   	unsigned cpu;
> @@ -420,6 +430,9 @@ static int evtchn_fifo_cpu_notification(struct notifier_block *self,
>   		if (!per_cpu(cpu_control_block, cpu))
>   			ret = evtchn_fifo_alloc_control_block(cpu);
>   		break;
> +	case CPU_DEAD:
> +		__evtchn_fifo_handle_events(cpu, true);
> +		break;
>   	default:
>   		break;
>   	}

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

[David Vrabel]

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

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

[Ross Lagerwall]

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

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

[Boris Ostrovsky]

On 06/29/2015 06:19 AM, Ross Lagerwall wrote:
> 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.

(Sorry, I missed this)

Why can't (actually, why doesn't) the cpu that is being offlined drain 
its queue?

-boris

>>
>> David
>>
>
> Ping. Is this change OK?
>

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

[Ross Lagerwall]

On 06/29/2015 02:32 PM, Boris Ostrovsky wrote:
> On 06/29/2015 06:19 AM, Ross Lagerwall wrote:
>> 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.
>
> (Sorry, I missed this)
>
> Why can't (actually, why doesn't) the cpu that is being offlined drain
> its queue?
>

Where would this be done? I thought using CPU notifiers was the correct 
way to hook when a CPU goes down without having to stick fifo event 
channel code in the core Xen code.

-- 
Ross Lagerwall

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

[Boris Ostrovsky]

On 06/30/2015 05:51 AM, Ross Lagerwall wrote:
> On 06/29/2015 02:32 PM, Boris Ostrovsky wrote:
>> On 06/29/2015 06:19 AM, Ross Lagerwall wrote:
>>> 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.
>>
>> (Sorry, I missed this)
>>
>> Why can't (actually, why doesn't) the cpu that is being offlined drain
>> its queue?
>>
>
> Where would this be done? I thought using CPU notifiers was the 
> correct way to hook when a CPU goes down without having to stick fifo 
> event channel code in the core Xen code.

In xen_evtchn_close(). We should be getting there (roughly) as cpu_die() 
-> xen_cpu_die() -> xen_smp_intr_free() -> unbind_from_irqhandler(). In 
fact, this path is taken right before cpu_down() sends CPU_DEAD 
notifications.

I think cleaning up in xen_evtchn_close() is better because it is 
possible to close event channel for reasons other than CPU going away, 
in which case we also may need to deal with unprocessed events.

(BTW, I noticed that you are cleaning up fifo events only. Do we need to 
do the same for 2-level?)

-boris

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

[David Vrabel]

On 30/06/15 13:26, Boris Ostrovsky wrote:
> On 06/30/2015 05:51 AM, Ross Lagerwall wrote:
>> On 06/29/2015 02:32 PM, Boris Ostrovsky wrote:
>>> On 06/29/2015 06:19 AM, Ross Lagerwall wrote:
>>>> 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.
>>>
>>> (Sorry, I missed this)
>>>
>>> Why can't (actually, why doesn't) the cpu that is being offlined drain
>>> its queue?
>>>
>>
>> Where would this be done? I thought using CPU notifiers was the
>> correct way to hook when a CPU goes down without having to stick fifo
>> event channel code in the core Xen code.
> 
> In xen_evtchn_close(). We should be getting there (roughly) as cpu_die()
> -> xen_cpu_die() -> xen_smp_intr_free() -> unbind_from_irqhandler(). In
> fact, this path is taken right before cpu_down() sends CPU_DEAD
> notifications.
> 
> I think cleaning up in xen_evtchn_close() is better because it is
> possible to close event channel for reasons other than CPU going away,
> in which case we also may need to deal with unprocessed events.

Having looked at this further and attempted to do this, draining events
in close is difficult because

a) we can't wait for LINKED to clear when closing since we're holding
the desc spin lock; and deferring the close to a tasklet or work doesn't
work because:

b) rebinding a PIRQ will fail if the close is deferred and not yet
completed and there is no way to ensure the close happens promptly
without changes to core irq code.

c) Xen will be fixed to not reuse ports that are still LINKED.

I'm going to apply Ross's original patch and Cc stable.

David

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

[David Vrabel]

On 19/06/15 16:15, 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.

Applied to for-linus-4.4, thanks.

David