[PATCH] x86/PoD: fix (un)locking after 24772:28edc2b31a9b

[PATCH] x86/PoD: fix (un)locking after 24772:28edc2b31a9b

[Jan Beulich]

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

That c/s introduced a double unlock on the out-of-memory error path of
p2m_pod_demand_populate().

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/mm/p2m-pod.c
+++ b/xen/arch/x86/mm/p2m-pod.c
@@ -1075,6 +1075,7 @@ out_of_memory:
     printk("%s: Out of populate-on-demand memory! tot_pages %" PRIu32 " pod_entries %" PRIi32 "\n",
            __func__, d->tot_pages, p2m->pod.entry_count);
     domain_crash(d);
+    return -1;
 out_fail:
     pod_unlock(p2m);
     return -1;




[-- Attachment #2: x86-PoD-locking-fix-24772.patch --]
[-- Type: text/plain, Size: 563 bytes --]

x86/PoD: fix (un)locking after 24772:28edc2b31a9b

That c/s introduced a double unlock on the out-of-memory error path of
p2m_pod_demand_populate().

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/mm/p2m-pod.c
+++ b/xen/arch/x86/mm/p2m-pod.c
@@ -1075,6 +1075,7 @@ out_of_memory:
     printk("%s: Out of populate-on-demand memory! tot_pages %" PRIu32 " pod_entries %" PRIi32 "\n",
            __func__, d->tot_pages, p2m->pod.entry_count);
     domain_crash(d);
+    return -1;
 out_fail:
     pod_unlock(p2m);
     return -1;

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

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

Re: [PATCH] x86/PoD: fix (un)locking after 24772:28edc2b31a9b

[George Dunlap]

On 13/08/12 13:11, Jan Beulich wrote:
> That c/s introduced a double unlock on the out-of-memory error path of
> p2m_pod_demand_populate().
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Good catch.

Acked-by: George Dunlap <george.dunlap@eu.citrix.com>
>
> --- a/xen/arch/x86/mm/p2m-pod.c
> +++ b/xen/arch/x86/mm/p2m-pod.c
> @@ -1075,6 +1075,7 @@ out_of_memory:
>       printk("%s: Out of populate-on-demand memory! tot_pages %" PRIu32 " pod_entries %" PRIi32 "\n",
>              __func__, d->tot_pages, p2m->pod.entry_count);
>       domain_crash(d);
> +    return -1;
>   out_fail:
>       pod_unlock(p2m);
>       return -1;
>
>
>

Re: [PATCH] x86/PoD: fix (un)locking after 24772:28edc2b31a9b

[Andres Lagar-Cavilla]

> That c/s introduced a double unlock on the out-of-memory error path of
> p2m_pod_demand_populate().
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Andres Lagar-Cavilla <andres@lagarcavilla.org>

Thanks
Andres
>
> --- a/xen/arch/x86/mm/p2m-pod.c
> +++ b/xen/arch/x86/mm/p2m-pod.c
> @@ -1075,6 +1075,7 @@ out_of_memory:
>      printk("%s: Out of populate-on-demand memory! tot_pages %" PRIu32 "
> pod_entries %" PRIi32 "\n",
>             __func__, d->tot_pages, p2m->pod.entry_count);
>      domain_crash(d);
> +    return -1;
>  out_fail:
>      pod_unlock(p2m);
>      return -1;
>
>
>
>

Re: [PATCH] x86/PoD: fix (un)locking after 24772:28edc2b31a9b

[Jan Beulich]

>>> On 13.08.12 at 14:11, "Jan Beulich" <JBeulich@suse.com> wrote:
> That c/s introduced a double unlock on the out-of-memory error path of
> p2m_pod_demand_populate().

I also wonder how correct that changeset's elimination of the page
alloc lock in a number of places here is - p2m_pod_set_mem_target()'s
calculations, for example, involve d->tot_pages, which with that lock
not held can change under its feet.

Jan

Re: [PATCH] x86/PoD: fix (un)locking after 24772:28edc2b31a9b

[Andres Lagar-Cavilla]

>>>> On 13.08.12 at 14:11, "Jan Beulich" <JBeulich@suse.com> wrote:
>> That c/s introduced a double unlock on the out-of-memory error path of
>> p2m_pod_demand_populate().
>
> I also wonder how correct that changeset's elimination of the page
> alloc lock in a number of places here is - p2m_pod_set_mem_target()'s
> calculations, for example, involve d->tot_pages, which with that lock
> not held can change under its feet.

afaict, access to d->tot_pages was not protected by the page_alloc lock
even prior to 24772.

Back when, I thought those unprotected tot_pages accesses should either be
locked or atomic_read(). Slipped through the cracks.

Andres

>
> Jan
>
>