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

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

[Sander Eikelenboom]

L.S.,

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.

--
Sander


(XEN) [2019-12-04 17:03:25.062] grant_table.c:1808:d7v0 Expanding d7 grant table from 3 to 4 frames
(XEN) [2019-12-04 17:03:25.365] Assertion '!preempt_count()' failed at preempt.c:36
(XEN) [2019-12-04 17:03:25.365] ----[ Xen-4.14-unstable  x86_64  debug=y   Not tainted ]----
(XEN) [2019-12-04 17:03:25.365] CPU:    0
(XEN) [2019-12-04 17:03:25.365] RIP:    e008:[<ffff82d080224b2d>] ASSERT_NOT_IN_ATOMIC+0x46/0x4c
(XEN) [2019-12-04 17:03:25.365] RFLAGS: 0000000000010202   CONTEXT: hypervisor (d0v5)
(XEN) [2019-12-04 17:03:25.365] rax: ffff82d080597020   rbx: ffff83069fd26000   rcx: 00000000000000a2
(XEN) [2019-12-04 17:03:25.365] rdx: 0000000000000000   rsi: ffff8306b20d38a0   rdi: ffff83069fd55b38
(XEN) [2019-12-04 17:03:25.365] rbp: ffff8300c7c8fee8   rsp: ffff8300c7c8fee8   r8:  deadbeefdeadf00d
(XEN) [2019-12-04 17:03:25.365] r9:  deadbeefdeadf00d   r10: 0000000000000000   r11: 0000000000000000
(XEN) [2019-12-04 17:03:25.365] r12: 0000000000000000   r13: 0000000000000000   r14: 0000000000000000
(XEN) [2019-12-04 17:03:25.365] r15: 0000000000000000   cr0: 0000000080050033   cr4: 00000000000006e0
(XEN) [2019-12-04 17:03:25.365] cr3: 00000004630a3000   cr2: 00007f602bd4b4f0
(XEN) [2019-12-04 17:03:25.365] fsb: 00007f602b08cbc0   gsb: ffff88807d540000   gss: 0000000000000000
(XEN) [2019-12-04 17:03:25.365] ds: 0000   es: 0000   fs: 0000   gs: 0000   ss: e010   cs: e008
(XEN) [2019-12-04 17:03:25.365] Xen code around <ffff82d080224b2d> (ASSERT_NOT_IN_ATOMIC+0x46/0x4c):
(XEN) [2019-12-04 17:03:25.365]  58 f6 c4 02 74 06 5d c3 <0f> 0b 0f 0b 0f 0b 55 48 89 e5 48 8d 05 e6 24 37
(XEN) [2019-12-04 17:03:25.365] Xen stack trace from rsp=ffff8300c7c8fee8:
(XEN) [2019-12-04 17:03:25.365]    00007cff383700e7 ffff82d080385065 0000000000000000 00007ffe22bf6d90
(XEN) [2019-12-04 17:03:25.365]    0000000000305000 00007ffe22bf6d90 ffff88805a2d8700 ffff8880703495f0
(XEN) [2019-12-04 17:03:25.365]    0000000000000282 0000000000000000 ffff888078435600 0000000000000000
(XEN) [2019-12-04 17:03:25.365]    0000000000000000 ffffffff8100148a 0000000000000000 0000000000000000
(XEN) [2019-12-04 17:03:25.365]    deadbeefdeadf00d 0000010000000000 ffffffff8100148a 000000000000e033
(XEN) [2019-12-04 17:03:25.365]    0000000000000282 ffffc90004dafd88 000000000000e02b 003e6be50048ffe0
(XEN) [2019-12-04 17:03:25.365]    003e6d0f00094f03 003e6e0300000000 003e699d0048ffe0 0000e01000000000
(XEN) [2019-12-04 17:03:25.365]    ffff83069fd26000 0000000000000000 00000000000006e0 0000000000000000
(XEN) [2019-12-04 17:03:25.365]    0000000000000000 003e040000000000 003e78f000094ea0
(XEN) [2019-12-04 17:03:25.365] Xen call trace:
(XEN) [2019-12-04 17:03:25.365]    [<ffff82d080224b2d>] R ASSERT_NOT_IN_ATOMIC+0x46/0x4c
(XEN) [2019-12-04 17:03:25.365]    [<ffff82d080385065>] F x86_64/entry.S#test_all_events+0x6/0x3d
(XEN) [2019-12-04 17:03:25.365] 
(XEN) [2019-12-04 17:03:26.089] 
(XEN) [2019-12-04 17:03:26.098] ****************************************
(XEN) [2019-12-04 17:03:26.117] Panic on CPU 0:
(XEN) [2019-12-04 17:03:26.130] Assertion '!preempt_count()' failed at preempt.c:36
(XEN) [2019-12-04 17:03:26.152] ****************************************
(XEN) [2019-12-04 17:03:26.171] 
(XEN) [2019-12-04 17:03:26.180] Reboot in five seconds...

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

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

[Jan Beulich]

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

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

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

[Sander Eikelenboom]

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@citrix.com>
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@citrix.com>
    Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>
    Acked-by: Jan Beulich <jbeulich@suse.com>



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

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

[Durrant, Paul]

> -----Original Message-----
> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of
> Sander Eikelenboom
> Sent: 04 December 2019 21:04
> To: Jan Beulich <jbeulich@suse.com>
> Cc: xen-devel@lists.xenproject.org; Igor Druzhinin
> <igor.druzhinin@citrix.com>; Paul Durrant <paul@xen.org>
> 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@citrix.com>
> 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@citrix.com>
>     Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com>
>     Acked-by: Jan Beulich <jbeulich@suse.com>
> 

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@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

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

[Jan Beulich]

On 05.12.2019 09:35, Durrant, Paul wrote:
> --- 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;
>          }

But this seems wrong - you'd end up calling assign_device() even
for the XEN_DOMCTL_test_assign_device case, when ret is 0. All we
want is to delete the break statement afaict.

Jan

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

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

[Durrant, Paul]

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: 05 December 2019 08:44
> To: Durrant, Paul <pdurrant@amazon.com>
> Cc: Sander Eikelenboom <linux@eikelenboom.it>; xen-
> devel@lists.xenproject.org; Igor Druzhinin <igor.druzhinin@citrix.com>;
> Paul Durrant <paul@xen.org>
> Subject: Re: xen-unstable (4.14 to be): Assertion '!preempt_count()'
> failed at preempt.c:36
> 
> On 05.12.2019 09:35, Durrant, Paul wrote:
> > --- 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;
> >          }
> 
> But this seems wrong - you'd end up calling assign_device() even
> for the XEN_DOMCTL_test_assign_device case, when ret is 0. All we
> want is to delete the break statement afaict.
> 

Ah, yes; that logic is quite confusing. The patch should indeed be:

---8<---
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index ced0c28e4f..c07a63981a 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -1705,7 +1705,6 @@ int iommu_do_pci_domctl(
                        seg, bus, PCI_SLOT(devfn), PCI_FUNC(devfn));
                 ret = -EINVAL;
             }
-            break;
         }
         else if ( !ret )
             ret = assign_device(d, seg, bus, devfn, flags);
---8<---

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