Xen devel - Aug 2012 - [PATCH] 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;





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

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;
>
>
>

> 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;
>
>
>
>

>>> 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

>>>> 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
>
>