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

[Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>]

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@xxxxxxxxxx>
> > > > > ---
> > > > >    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

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