[PATCH] pci_remove_device: fix linked list discipline

[PATCH] pci_remove_device: fix linked list discipline

[Tim Deegan]

# HG changeset patch
# User Tim Deegan <Tim.Deegan@citrix.com>
# Date 1305708740 -3600
# Node ID 7b12c46b18777655c8a5f8290286f5699c77c335
# Parent  f531ed84b0661aa6863dc86d5e5638642bc47301
pci_remove_device: fix linked list discipline

Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com>

diff -r f531ed84b066 -r 7b12c46b1877 xen/drivers/passthrough/pci.c
--- a/xen/drivers/passthrough/pci.c	Tue May 17 17:32:19 2011 +0100
+++ b/xen/drivers/passthrough/pci.c	Wed May 18 09:52:20 2011 +0100
@@ -173,11 +173,11 @@ out:
 
 int pci_remove_device(u8 bus, u8 devfn)
 {
-    struct pci_dev *pdev;
+    struct pci_dev *pdev, *tmp;
     int ret = -ENODEV;
 
     spin_lock(&pcidevs_lock);
-    list_for_each_entry ( pdev, &alldevs_list, alldevs_list )
+    list_for_each_entry_safe ( pdev, tmp, &alldevs_list, alldevs_list )
         if ( pdev->bus == bus && pdev->devfn == devfn )
         {
             ret = iommu_remove_device(pdev);

Re: [PATCH] pci_remove_device: fix linked list discipline

[Jan Beulich]

>>> On 18.05.11 at 10:53, Tim Deegan <Tim.Deegan@citrix.com> wrote:
> # HG changeset patch
> # User Tim Deegan <Tim.Deegan@citrix.com>
> # Date 1305708740 -3600
> # Node ID 7b12c46b18777655c8a5f8290286f5699c77c335
> # Parent  f531ed84b0661aa6863dc86d5e5638642bc47301
> pci_remove_device: fix linked list discipline
> 
> Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com>
> 
> diff -r f531ed84b066 -r 7b12c46b1877 xen/drivers/passthrough/pci.c
> --- a/xen/drivers/passthrough/pci.c	Tue May 17 17:32:19 2011 +0100
> +++ b/xen/drivers/passthrough/pci.c	Wed May 18 09:52:20 2011 +0100
> @@ -173,11 +173,11 @@ out:
>  
>  int pci_remove_device(u8 bus, u8 devfn)
>  {
> -    struct pci_dev *pdev;
> +    struct pci_dev *pdev, *tmp;
>      int ret = -ENODEV;
>  
>      spin_lock(&pcidevs_lock);
> -    list_for_each_entry ( pdev, &alldevs_list, alldevs_list )
> +    list_for_each_entry_safe ( pdev, tmp, &alldevs_list, alldevs_list )

Somehow I overlooked this patch when it was sent - looking at the
code it modifies I can't see why the ..._safe() variant is necessary
here, as there's a break statement following the list deletion.

Jan

>          if ( pdev->bus == bus && pdev->devfn == devfn )
>          {
>              ret = iommu_remove_device(pdev);
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com 
> http://lists.xensource.com/xen-devel 

Re: [PATCH] pci_remove_device: fix linked list discipline

[Tim Deegan]

At 16:41 +0100 on 20 May (1305909715), Jan Beulich wrote:
> > diff -r f531ed84b066 -r 7b12c46b1877 xen/drivers/passthrough/pci.c
> > --- a/xen/drivers/passthrough/pci.c	Tue May 17 17:32:19 2011 +0100
> > +++ b/xen/drivers/passthrough/pci.c	Wed May 18 09:52:20 2011 +0100
> > @@ -173,11 +173,11 @@ out:
> >  
> >  int pci_remove_device(u8 bus, u8 devfn)
> >  {
> > -    struct pci_dev *pdev;
> > +    struct pci_dev *pdev, *tmp;
> >      int ret = -ENODEV;
> >  
> >      spin_lock(&pcidevs_lock);
> > -    list_for_each_entry ( pdev, &alldevs_list, alldevs_list )
> > +    list_for_each_entry_safe ( pdev, tmp, &alldevs_list, alldevs_list )
> 
> Somehow I overlooked this patch when it was sent - looking at the
> code it modifies I can't see why the ..._safe() variant is necessary
> here, as there's a break statement following the list deletion.

Ah - good point.  I'll go back to the crash report I thought this fixed
and take another look. :)

Cheers,

Tim.

-- 
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

[PATCH] Re: pci_remove_device: fix linked list discipline

[Tim Deegan]

[-- Attachment #1: Type: text/plain, Size: 726 bytes --]

At 10:55 +0100 on 23 May (1306148154), Tim Deegan wrote:
> > Somehow I overlooked this patch when it was sent - looking at the
> > code it modifies I can't see why the ..._safe() variant is necessary
> > here, as there's a break statement following the list deletion.
> 
> Ah - good point.  I'll go back to the crash report I thought this fixed
> and take another look. :)

The attached patch is a more likely candidate.  That will teach me to
debug crash reports without getting hold of proper debug symbols. :)

Keir, can you apply this and revert 23352:ea48976517af ?

Cheers,

Tim.

-- 
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

[-- Attachment #2: x --]
[-- Type: text/plain, Size: 1390 bytes --]

# HG changeset patch
# User Tim Deegan <Tim.Deegan@citrix.com>
# Date 1306154800 -3600
# Node ID d585903eb8bce83d7d365a4c94bbf0176283c3e9
# Parent  0f670f5146c858ffdc743176d4e22aef4bfe12da
drivers/passthrough: fix error paths in pci_add_device*()

When a device can't be allocated to dom0 by the IOMMU, don't leave
dom0 in the "domain" field.  It causes pci_remove_device()
to crash trying to remove the dev from the domain's list of devices
(and was probably the wrong thing to do anyway).

Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com>

diff -r 0f670f5146c8 -r d585903eb8bc xen/drivers/passthrough/pci.c
--- a/xen/drivers/passthrough/pci.c	Sat May 21 07:55:46 2011 +0100
+++ b/xen/drivers/passthrough/pci.c	Mon May 23 13:46:40 2011 +0100
@@ -158,7 +158,10 @@ int pci_add_device(u8 bus, u8 devfn)
         pdev->domain = dom0;
         ret = iommu_add_device(pdev);
         if ( ret )
+        {
+            pdev->domain = NULL;
             goto out;
+        }
 
         list_add(&pdev->domain_list, &dom0->arch.pdev_list);
         pci_enable_acs(pdev);
@@ -222,7 +225,10 @@ int pci_add_device_ext(u8 bus, u8 devfn,
         pdev->domain = dom0;
         ret = iommu_add_device(pdev);
         if ( ret )
+        {
+            pdev->domain = NULL;
             goto out;
+        }
 
         list_add(&pdev->domain_list, &dom0->arch.pdev_list);
         pci_enable_acs(pdev);

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel