| Message ID | 20220427092743.925563-3-Penny.Zheng@arm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | populate/unpopulate memory when domain on static | expand |
Hi, On 27/04/2022 10:27, Penny Zheng wrote: > There is a slim chance that free_heap_pages() may decide to merge a chunk > from the static region(PGC_reserved) with the about-to-be-free chunk. This sentence tells me that the merge can happen but it doesn't tell me the cases. I think the second part is more important to know. The code in free_heap_Pages() will try to merge with the successor/predecessor if they are suitably aligned. So the issue can only happen if the pages reserved are right next to the pages given to the heap allocator. > > So in order to avoid the above scenario, this commit updates free_heap_pages() > to check whether the predecessor and/or successor has PGC_reserved set, > when trying to merge the about-to-be-freed chunk with the predecessor > and/or successor. > > Signed-off-by: Penny Zheng <penny.zheng@arm.com> > --- > v3 changes: > - no changes > --- > v2 changes: > - new commit > --- > 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 be501582a3..1f3ad4bd28 100644 > --- a/xen/common/page_alloc.c > +++ b/xen/common/page_alloc.c > @@ -1483,6 +1483,7 @@ static void free_heap_pages( > /* Merge with predecessor block? */ > if ( !mfn_valid(page_to_mfn(predecessor)) || > !page_state_is(predecessor, free) || > + (predecessor->count_info & PGC_reserved) || > (PFN_ORDER(predecessor) != order) || > (phys_to_nid(page_to_maddr(predecessor)) != node) ) > break; > @@ -1506,6 +1507,7 @@ static void free_heap_pages( > /* Merge with successor block? */ > if ( !mfn_valid(page_to_mfn(successor)) || > !page_state_is(successor, free) || > + (successor->count_info & PGC_reserved) || > (PFN_ORDER(successor) != order) || > (phys_to_nid(page_to_maddr(successor)) != node) ) > break; Cheers,
On 27.04.2022 11:27, Penny Zheng wrote: > There is a slim chance that free_heap_pages() may decide to merge a chunk > from the static region(PGC_reserved) with the about-to-be-free chunk. > > So in order to avoid the above scenario, this commit updates free_heap_pages() > to check whether the predecessor and/or successor has PGC_reserved set, > when trying to merge the about-to-be-freed chunk with the predecessor > and/or successor. > > Signed-off-by: Penny Zheng <penny.zheng@arm.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> I think this also wants a Suggested-by or Reported-by (iirc) Julien? Jan
Hi, > -----Original Message----- > From: Jan Beulich <jbeulich@suse.com> > Sent: Wednesday, May 4, 2022 9:30 PM > To: Penny Zheng <Penny.Zheng@arm.com> > Cc: Wei Chen <Wei.Chen@arm.com>; Henry Wang <Henry.Wang@arm.com>; > Andrew Cooper <andrew.cooper3@citrix.com>; George Dunlap > <george.dunlap@citrix.com>; Julien Grall <julien@xen.org>; Stefano > Stabellini <sstabellini@kernel.org>; Wei Liu <wl@xen.org>; xen- > devel@lists.xenproject.org > Subject: Re: [PATCH v3 2/6] xen: do not merge reserved pages in > free_heap_pages() > > On 27.04.2022 11:27, Penny Zheng wrote: > > There is a slim chance that free_heap_pages() may decide to merge a > > chunk from the static region(PGC_reserved) with the about-to-be-free > chunk. > > > > So in order to avoid the above scenario, this commit updates > > free_heap_pages() to check whether the predecessor and/or successor > > has PGC_reserved set, when trying to merge the about-to-be-freed chunk > > with the predecessor and/or successor. > > > > Signed-off-by: Penny Zheng <penny.zheng@arm.com> > > Reviewed-by: Jan Beulich <jbeulich@suse.com> > > I think this also wants a Suggested-by or Reported-by (iirc) Julien? > Sure, I'll definitely add-in Suggested-by: Julien Grall <jgrall@amazon.com> > Jan
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c index be501582a3..1f3ad4bd28 100644 --- a/xen/common/page_alloc.c +++ b/xen/common/page_alloc.c @@ -1483,6 +1483,7 @@ static void free_heap_pages( /* Merge with predecessor block? */ if ( !mfn_valid(page_to_mfn(predecessor)) || !page_state_is(predecessor, free) || + (predecessor->count_info & PGC_reserved) || (PFN_ORDER(predecessor) != order) || (phys_to_nid(page_to_maddr(predecessor)) != node) ) break; @@ -1506,6 +1507,7 @@ static void free_heap_pages( /* Merge with successor block? */ if ( !mfn_valid(page_to_mfn(successor)) || !page_state_is(successor, free) || + (successor->count_info & PGC_reserved) || (PFN_ORDER(successor) != order) || (phys_to_nid(page_to_maddr(successor)) != node) ) break;
There is a slim chance that free_heap_pages() may decide to merge a chunk from the static region(PGC_reserved) with the about-to-be-free chunk. So in order to avoid the above scenario, this commit updates free_heap_pages() to check whether the predecessor and/or successor has PGC_reserved set, when trying to merge the about-to-be-freed chunk with the predecessor and/or successor. Signed-off-by: Penny Zheng <penny.zheng@arm.com> --- v3 changes: - no changes --- v2 changes: - new commit --- xen/common/page_alloc.c | 2 ++ 1 file changed, 2 insertions(+)