All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stewart Hildebrand <Stewart.Hildebrand@dornerworks.com>
To: Jan Beulich <jbeulich@suse.com>, Julien Grall <julien.grall@arm.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>, Wei Liu <wl@xen.org>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	George Dunlap <George.Dunlap@eu.citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Ian Jackson <ian.jackson@eu.citrix.com>,
	Jeff Kubascik <Jeff.Kubascik@dornerworks.com>,
	Tim Deegan <tim@xen.org>,
	"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
	Jarvis Roach <Jarvis.Roach@dornerworks.com>
Subject: Re: [Xen-devel] [PATCH] xen/page_alloc: Keep away MFN 0 from the buddy allocator
Date: Fri, 9 Aug 2019 18:15:14 +0000	[thread overview]
Message-ID: <62e082b025d8483d8b577c283fce1873@dornerworks.com> (raw)
In-Reply-To: <e9d6353c-fd4d-116e-2531-7db46abf2b23@suse.com>

On Friday, August 9, 2019 9:39 AM, Jan Beulich <jbeulich@suse.com> wrote:
>On 09.08.2019 14:14, Julien Grall wrote:
>> Combining of buddies happens only such that the resulting larger buddy
>> is still order-aligned. To cross a zone boundary while merging, the
>> implication is that both the buddy [0, 2^n-1] and the buddy
>> [2^n, 2^(n+1)] are free.
>
>[2^n, 2^(n+1)-1]
>
>You may want to add that merging across zone boundaries is what we
>need to prevent.
>
>> Ideally we want to fix the allocator, but for now we can just prevent
>> adding the MFN 0 in the allocator.
>>
>> On x86, the MFN 0 is already kept away from the buddy allocator. So the
>> bug can only happen on Arm platform where the first memory bank is
>> starting at 0.
>>
>> As this is a specific to the allocator, the MFN 0 is removed in the common code
>> to cater all the architectures (current and future).
>>
>> Reported-by: Jeff Kubascik <jeff.kubascik@dornerworks.com>
>> Signed-off-by: Julien Grall <julien.grall@arm.com>
>
>Reviewed-by: Jan Beulich <jbeulich@suse.com>

Here is Jeff's initial patch for the issue.

From: Jeff Kubascik <jeff.kubascik@dornerworks.com>
Date: Mon, 4 Mar 2019 14:14:05 -0500
Subject: [PATCH] Check zone before merging adjacent blocks in heap

The Xen heap is split up into nodes and zones. Each node + zone is
managed as a separate pool of memory.

When returning pages to the heap, free_heap_pages will check adjacent
blocks to see if they can be combined into a larger block. However, the
zone of the adjacent block is not checked. This results in blocks that
migrate from one zone to another.

When a block migrates to the adjacent zone, the avail counters for the
old and new node + zone is not updated accordingly. The avail counter
is used when allocating pages to determine whether to skip over a zone.
With this behavior, it is possible for free pages to collect in a zone
with the avail counter smaller than the actual page count, resulting
in free pages that are not allocable.

This commit adds a check to compare the adjacent block's zone with the
current zone before merging them.

Signed-off-by: Jeff Kubascik <Jeff.Kubascik@dornerworks.com>
Tested-by: Stewart Hildebrand <stewart.hildebrand@dornerworks.com>
---
 xen/common/page_alloc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 482f0988f7..a92268cc67 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -1419,6 +1419,7 @@ static void free_heap_pages(
             if ( !mfn_valid(page_to_mfn(predecessor)) ||
                  !page_state_is(predecessor, free) ||
                  (PFN_ORDER(predecessor) != order) ||
+                 (page_to_zone(pg-mask) != zone) ||
                  (phys_to_nid(page_to_maddr(predecessor)) != node) )
                 break;
 
@@ -1442,6 +1443,7 @@ static void free_heap_pages(
             if ( !mfn_valid(page_to_mfn(successor)) ||
                  !page_state_is(successor, free) ||
                  (PFN_ORDER(successor) != order) ||
+                 (page_to_zone(pg+mask) != zone) ||
                  (phys_to_nid(page_to_maddr(successor)) != node) )
                 break;
 
-- 
2.22.0

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

  reply	other threads:[~2019-08-09 18:14 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-09 12:14 [Xen-devel] [PATCH] xen/page_alloc: Keep away MFN 0 from the buddy allocator Julien Grall
2019-08-09 13:38 ` Jan Beulich
2019-08-09 18:15   ` Stewart Hildebrand [this message]
2019-08-09 18:23     ` Stefano Stabellini
2019-08-09 18:34       ` Stewart Hildebrand
2019-08-09 18:35         ` Julien Grall
2019-08-12  6:16         ` Jan Beulich
2019-08-12  6:01     ` Jan Beulich
2019-08-09 17:55 ` Stefano Stabellini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=62e082b025d8483d8b577c283fce1873@dornerworks.com \
    --to=stewart.hildebrand@dornerworks.com \
    --cc=George.Dunlap@eu.citrix.com \
    --cc=Jarvis.Roach@dornerworks.com \
    --cc=Jeff.Kubascik@dornerworks.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=ian.jackson@eu.citrix.com \
    --cc=jbeulich@suse.com \
    --cc=julien.grall@arm.com \
    --cc=konrad.wilk@oracle.com \
    --cc=sstabellini@kernel.org \
    --cc=tim@xen.org \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.