Re: [Xen-devel] xen-unstable (4.14 to be): Assertion '!preempt_count()' failed at preempt.c:36

["Durrant, Paul" <pdurrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Xen-devel <xen-devel-bounces@xxxxxxxxxxxxxxxxxxxx> On Behalf Of
> Sander Eikelenboom
> Sent: 04 December 2019 21:04
> To: Jan Beulich <jbeulich@xxxxxxxx>
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; Igor Druzhinin
> <igor.druzhinin@xxxxxxxxxx>; Paul Durrant <paul@xxxxxxx>
> Subject: Re: [Xen-devel] xen-unstable (4.14 to be): Assertion
> '!preempt_count()' failed at preempt.c:36
> 
> On 04/12/2019 18:30, Jan Beulich wrote:
> > On 04.12.2019 18:21, Sander Eikelenboom wrote:
> >> On current xen-unstable (4.14 to be) and AMD cpu:
> >>
> >> After rebooting the host, while the guests are starting, I hit the
> assertion below.
> >> xen-staging-4.13 seems fine on the same machine.
> >
> > Nothing between 4.13 RC4 and the tip of staging stands out,
> > so I wonder if you could bisect over this range? Or perhaps
> > someone else sees something I don't see (right now).
> >
> > Jan
> 
> Bisection came up with:
> 
> commit cd7dedad8209753e0fc8a97e61d04b74912b53dc
> Author: Paul Durrant <paul.durrant@xxxxxxxxxx>
> Date:   Fri Nov 15 18:59:30 2019 +0000
> 
>     passthrough: simplify locking and logging
> 
>     Dropping the pcidevs lock between calling device_assigned() and
>     assign_device() means that the latter has to do the same check as the
>     former for no obvious gain. Also, since long running operations under
>     pcidevs lock already drop the lock and return -ERESTART periodically
> there
>     is little point in immediately failing an assignment operation with
>     -ERESTART just because the pcidevs lock could not be acquired (for the
>     second time, having already blocked on acquiring the lock in
>     device_assigned()).
> 
>     This patch instead acquires the lock once for assignment (or test
> assign)
>     operations directly in iommu_do_pci_domctl() and thus can remove the
>     duplicate domain ownership check in assign_device(). Whilst in the
>     neighbourhood, the patch also removes some debug logging from
>     assign_device() and deassign_device() and replaces it with proper
> error
>     logging, which allows error logging in iommu_do_pci_domctl() to be
>     removed.
> 
>     Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
>     Signed-off-by: Igor Druzhinin <igor.druzhinin@xxxxxxxxxx>
>     Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
> 

Going through the code, I notice a missing pcidevs_unlock() in the case of a 
device already assigned. I fixed it with a bit of re-structuring. Could you try 
the following patch?

---8<---
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index ced0c28e4f..c7207998a5 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -1696,16 +1696,12 @@ int iommu_do_pci_domctl(

         pcidevs_lock();
         ret = device_assigned(seg, bus, devfn);
-        if ( domctl->cmd == XEN_DOMCTL_test_assign_device )
+        if ( ret && domctl->cmd == XEN_DOMCTL_test_assign_device )
         {
-            if ( ret )
-            {
-                printk(XENLOG_G_INFO
-                       "%04x:%02x:%02x.%u already assigned, or non-existent\n",
-                       seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
-                ret = -EINVAL;
-            }
-            break;
+            printk(XENLOG_G_INFO
+                   "%04x:%02x:%02x.%u already assigned, or non-existent\n",
+                   seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
+            ret = -EINVAL;
         }
---8<---

Thanks,

  Paul


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel