| Message ID | 1424795655-16952-1-git-send-email-marc.mari.barcelo@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 02/24/2015 11:34 AM, Marc Marí wrote: > The MSIX interrupt was always acked without checking its value, which caused a > race condition. If the ISR was raised between the read and the acking, the ISR > was never detected and it timed out. > > Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com> > --- > tests/libqos/virtio-pci.c | 16 ++++++++++++---- > 1 file changed, 12 insertions(+), 4 deletions(-) > > diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c > index 788ebaf..c74a669 100644 > --- a/tests/libqos/virtio-pci.c > +++ b/tests/libqos/virtio-pci.c > @@ -140,8 +140,12 @@ static bool qvirtio_pci_get_queue_isr_status(QVirtioDevice *d, QVirtQueue *vq) > return qpci_msix_pending(dev->pdev, vqpci->msix_entry); > } else { > data = readl(vqpci->msix_addr); > - writel(vqpci->msix_addr, 0); > - return data == vqpci->msix_data; > + if (data == vqpci->msix_data) { > + writel(vqpci->msix_addr, 0); > + return true; > + } else { > + return false; > + } > } > } else { > return qpci_io_readb(dev->pdev, dev->addr + QVIRTIO_ISR_STATUS) & 1; > @@ -160,8 +164,12 @@ static bool qvirtio_pci_get_config_isr_status(QVirtioDevice *d) > return qpci_msix_pending(dev->pdev, dev->config_msix_entry); > } else { > data = readl(dev->config_msix_addr); > - writel(dev->config_msix_addr, 0); > - return data == dev->config_msix_data; > + if (data == dev->config_msix_data) { > + writel(dev->config_msix_addr, 0); > + return true; > + } else { > + return false; > + } > } > } else { > return qpci_io_readb(dev->pdev, dev->addr + QVIRTIO_ISR_STATUS) & 2; > 1,600+ runs and no hang, thanks :) Tested-by: John Snow <jsnow@redhat.com> Reviewed-by: John Snow <jsnow@redhat.com>
On 02/24/2015 01:09 PM, John Snow wrote: > > > On 02/24/2015 11:34 AM, Marc Marí wrote: >> The MSIX interrupt was always acked without checking its value, which >> caused a >> race condition. If the ISR was raised between the read and the acking, >> the ISR >> was never detected and it timed out. >> >> Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com> >> --- >> tests/libqos/virtio-pci.c | 16 ++++++++++++---- >> 1 file changed, 12 insertions(+), 4 deletions(-) >> >> diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c >> index 788ebaf..c74a669 100644 >> --- a/tests/libqos/virtio-pci.c >> +++ b/tests/libqos/virtio-pci.c >> @@ -140,8 +140,12 @@ static bool >> qvirtio_pci_get_queue_isr_status(QVirtioDevice *d, QVirtQueue *vq) >> return qpci_msix_pending(dev->pdev, vqpci->msix_entry); >> } else { >> data = readl(vqpci->msix_addr); >> - writel(vqpci->msix_addr, 0); >> - return data == vqpci->msix_data; >> + if (data == vqpci->msix_data) { >> + writel(vqpci->msix_addr, 0); >> + return true; >> + } else { >> + return false; >> + } >> } >> } else { >> return qpci_io_readb(dev->pdev, dev->addr + >> QVIRTIO_ISR_STATUS) & 1; >> @@ -160,8 +164,12 @@ static bool >> qvirtio_pci_get_config_isr_status(QVirtioDevice *d) >> return qpci_msix_pending(dev->pdev, >> dev->config_msix_entry); >> } else { >> data = readl(dev->config_msix_addr); >> - writel(dev->config_msix_addr, 0); >> - return data == dev->config_msix_data; >> + if (data == dev->config_msix_data) { >> + writel(dev->config_msix_addr, 0); >> + return true; >> + } else { >> + return false; >> + } >> } >> } else { >> return qpci_io_readb(dev->pdev, dev->addr + >> QVIRTIO_ISR_STATUS) & 2; >> > > 1,600+ runs and no hang, thanks :) > > Tested-by: John Snow <jsnow@redhat.com> > Reviewed-by: John Snow <jsnow@redhat.com> > Ping? Still hitting this failure upstream.
El Tue, 10 Mar 2015 16:50:48 -0400 John Snow <jsnow@redhat.com> escribió: > > > On 02/24/2015 01:09 PM, John Snow wrote: > > > > > > On 02/24/2015 11:34 AM, Marc Marí wrote: > >> The MSIX interrupt was always acked without checking its value, > >> which caused a > >> race condition. If the ISR was raised between the read and the > >> acking, the ISR > >> was never detected and it timed out. > >> > >> Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com> > >> --- > >> tests/libqos/virtio-pci.c | 16 ++++++++++++---- > >> 1 file changed, 12 insertions(+), 4 deletions(-) > >> > >> diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c > >> index 788ebaf..c74a669 100644 > >> --- a/tests/libqos/virtio-pci.c > >> +++ b/tests/libqos/virtio-pci.c > >> @@ -140,8 +140,12 @@ static bool > >> qvirtio_pci_get_queue_isr_status(QVirtioDevice *d, QVirtQueue *vq) > >> return qpci_msix_pending(dev->pdev, > >> vqpci->msix_entry); } else { > >> data = readl(vqpci->msix_addr); > >> - writel(vqpci->msix_addr, 0); > >> - return data == vqpci->msix_data; > >> + if (data == vqpci->msix_data) { > >> + writel(vqpci->msix_addr, 0); > >> + return true; > >> + } else { > >> + return false; > >> + } > >> } > >> } else { > >> return qpci_io_readb(dev->pdev, dev->addr + > >> QVIRTIO_ISR_STATUS) & 1; > >> @@ -160,8 +164,12 @@ static bool > >> qvirtio_pci_get_config_isr_status(QVirtioDevice *d) > >> return qpci_msix_pending(dev->pdev, > >> dev->config_msix_entry); > >> } else { > >> data = readl(dev->config_msix_addr); > >> - writel(dev->config_msix_addr, 0); > >> - return data == dev->config_msix_data; > >> + if (data == dev->config_msix_data) { > >> + writel(dev->config_msix_addr, 0); > >> + return true; > >> + } else { > >> + return false; > >> + } > >> } > >> } else { > >> return qpci_io_readb(dev->pdev, dev->addr + > >> QVIRTIO_ISR_STATUS) & 2; > >> > > > > 1,600+ runs and no hang, thanks :) > > > > Tested-by: John Snow <jsnow@redhat.com> > > Reviewed-by: John Snow <jsnow@redhat.com> > > > > Ping? > > Still hitting this failure upstream. I think this is in Stefan's pull request, which has issues merging. Marc
Am 10.03.2015 um 21:50 schrieb John Snow: > On 02/24/2015 01:09 PM, John Snow wrote: >> On 02/24/2015 11:34 AM, Marc Marí wrote: >>> The MSIX interrupt was always acked without checking its value, which >>> caused a >>> race condition. If the ISR was raised between the read and the acking, >>> the ISR >>> was never detected and it timed out. >>> >>> Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com> >>> --- >>> tests/libqos/virtio-pci.c | 16 ++++++++++++---- >>> 1 file changed, 12 insertions(+), 4 deletions(-) >>> >>> diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c >>> index 788ebaf..c74a669 100644 >>> --- a/tests/libqos/virtio-pci.c >>> +++ b/tests/libqos/virtio-pci.c >>> @@ -140,8 +140,12 @@ static bool >>> qvirtio_pci_get_queue_isr_status(QVirtioDevice *d, QVirtQueue *vq) >>> return qpci_msix_pending(dev->pdev, vqpci->msix_entry); >>> } else { >>> data = readl(vqpci->msix_addr); >>> - writel(vqpci->msix_addr, 0); >>> - return data == vqpci->msix_data; >>> + if (data == vqpci->msix_data) { >>> + writel(vqpci->msix_addr, 0); >>> + return true; >>> + } else { >>> + return false; >>> + } >>> } >>> } else { >>> return qpci_io_readb(dev->pdev, dev->addr + >>> QVIRTIO_ISR_STATUS) & 1; >>> @@ -160,8 +164,12 @@ static bool >>> qvirtio_pci_get_config_isr_status(QVirtioDevice *d) >>> return qpci_msix_pending(dev->pdev, >>> dev->config_msix_entry); >>> } else { >>> data = readl(dev->config_msix_addr); >>> - writel(dev->config_msix_addr, 0); >>> - return data == dev->config_msix_data; >>> + if (data == dev->config_msix_data) { >>> + writel(dev->config_msix_addr, 0); >>> + return true; >>> + } else { >>> + return false; >>> + } >>> } >>> } else { >>> return qpci_io_readb(dev->pdev, dev->addr + >>> QVIRTIO_ISR_STATUS) & 2; >>> >> >> 1,600+ runs and no hang, thanks :) >> >> Tested-by: John Snow <jsnow@redhat.com> >> Reviewed-by: John Snow <jsnow@redhat.com> >> > > Ping? > > Still hitting this failure upstream. CC'ing the PCI/virtio maintainer might help. ;) Cheers, Andreas
diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c index 788ebaf..c74a669 100644 --- a/tests/libqos/virtio-pci.c +++ b/tests/libqos/virtio-pci.c @@ -140,8 +140,12 @@ static bool qvirtio_pci_get_queue_isr_status(QVirtioDevice *d, QVirtQueue *vq) return qpci_msix_pending(dev->pdev, vqpci->msix_entry); } else { data = readl(vqpci->msix_addr); - writel(vqpci->msix_addr, 0); - return data == vqpci->msix_data; + if (data == vqpci->msix_data) { + writel(vqpci->msix_addr, 0); + return true; + } else { + return false; + } } } else { return qpci_io_readb(dev->pdev, dev->addr + QVIRTIO_ISR_STATUS) & 1; @@ -160,8 +164,12 @@ static bool qvirtio_pci_get_config_isr_status(QVirtioDevice *d) return qpci_msix_pending(dev->pdev, dev->config_msix_entry); } else { data = readl(dev->config_msix_addr); - writel(dev->config_msix_addr, 0); - return data == dev->config_msix_data; + if (data == dev->config_msix_data) { + writel(dev->config_msix_addr, 0); + return true; + } else { + return false; + } } } else { return qpci_io_readb(dev->pdev, dev->addr + QVIRTIO_ISR_STATUS) & 2;
The MSIX interrupt was always acked without checking its value, which caused a race condition. If the ISR was raised between the read and the acking, the ISR was never detected and it timed out. Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com> --- tests/libqos/virtio-pci.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-)