Re: [PATCH v2 4/8] xen/netback: fix spurious event detection for common event case
On Thu, Feb 11, 2021 at 11:16:12AM +0100, Juergen Gross wrote: > In case of a common event for rx and tx queue the event should be > regarded to be spurious if no rx and no tx requests are pending. > > Unfortunately the condition for testing that is wrong causing to > decide a event being spurious if no rx OR no tx requests are > pending. > > Fix that plus using local variables for rx/tx pending indicators in > order to split function calls and if condition. > > Fixes: 23025393dbeb3b ("xen/netback: use lateeoi irq binding") > Signed-off-by: Juergen Gross Reviewed-by: Wei Liu
RE: [PATCH v2 4/8] xen/netback: fix spurious event detection for common event case
> -Original Message- > From: Juergen Gross > Sent: 11 February 2021 10:16 > To: xen-de...@lists.xenproject.org; net...@vger.kernel.org; > linux-kernel@vger.kernel.org > Cc: Juergen Gross ; Wei Liu ; Paul > Durrant ; David > S. Miller ; Jakub Kicinski > Subject: [PATCH v2 4/8] xen/netback: fix spurious event detection for common > event case > > In case of a common event for rx and tx queue the event should be > regarded to be spurious if no rx and no tx requests are pending. > > Unfortunately the condition for testing that is wrong causing to > decide a event being spurious if no rx OR no tx requests are > pending. > > Fix that plus using local variables for rx/tx pending indicators in > order to split function calls and if condition. > Definitely neater. > Fixes: 23025393dbeb3b ("xen/netback: use lateeoi irq binding") > Signed-off-by: Juergen Gross Reviewed-by: Paul Durrant
Re: [PATCH v2 4/8] xen/netback: fix spurious event detection for common event case
On 11.02.2021 11:16, Juergen Gross wrote: > --- a/drivers/net/xen-netback/interface.c > +++ b/drivers/net/xen-netback/interface.c > @@ -162,13 +162,15 @@ irqreturn_t xenvif_interrupt(int irq, void *dev_id) > { > struct xenvif_queue *queue = dev_id; > int old; > + bool has_rx, has_tx; > > old = atomic_fetch_or(NETBK_COMMON_EOI, &queue->eoi_pending); > WARN(old, "Interrupt while EOI pending\n"); > > - /* Use bitwise or as we need to call both functions. */ > - if ((!xenvif_handle_tx_interrupt(queue) | > - !xenvif_handle_rx_interrupt(queue))) { > + has_tx = xenvif_handle_tx_interrupt(queue); > + has_rx = xenvif_handle_rx_interrupt(queue); > + > + if (!has_rx && !has_tx) { > atomic_andnot(NETBK_COMMON_EOI, &queue->eoi_pending); > xen_irq_lateeoi(irq, XEN_EOI_FLAG_SPURIOUS); > } > Ah yes, what was originally meant really was if (!(xenvif_handle_tx_interrupt(queue) | xenvif_handle_rx_interrupt(queue))) { (also hinted at by the otherwise pointless inner parentheses), which you simply write in an alternative way. Reviewed-by: Jan Beulich Jan