Wei Liu
2013-Sep-05 10:00 UTC
[PATCH RFC] xen/balloon: check whether a page is pointed to scratch page MFN
Now that balloon pages might have two kind of P2M entries, a)
INVALID_P2M_ENTRY when that page is never used, b) mfn of one of the
many ballooned scratch pages. Refelct this in increase_reservation to
avoid hitting BUG_ON.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
drivers/xen/balloon.c | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
index 3101cf6..63a7e5e 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -305,6 +305,24 @@ static enum bp_state reserve_additional_memory(long credit)
}
#endif /* CONFIG_XEN_BALLOON_MEMORY_HOTPLUG */
+static bool p2m_is_ballooned_scratch_page(unsigned long pfn)
+{
+ int cpu;
+ unsigned long bpfn;
+ struct page *bpage;
+
+ for_each_possible_cpu(cpu) {
+ bpage = per_cpu(balloon_scratch_page, cpu);
+ if (!bpage)
+ continue;
+ bpfn = page_to_pfn(bpage);
+ if (pfn_to_mfn(pfn) == pfn_to_mfn(bpfn))
+ return true;
+ }
+
+ return false;
+}
+
static enum bp_state increase_reservation(unsigned long nr_pages)
{
int rc;
@@ -350,7 +368,8 @@ static enum bp_state increase_reservation(unsigned long
nr_pages)
pfn = page_to_pfn(page);
BUG_ON(!xen_feature(XENFEAT_auto_translated_physmap) &&
- phys_to_machine_mapping_valid(pfn));
+ phys_to_machine_mapping_valid(pfn) &&
+ !p2m_is_ballooned_scratch_page(pfn));
set_phys_to_machine(pfn, frame_list[i]);
--
1.7.10.4
Stefano Stabellini
2013-Sep-05 14:21 UTC
Re: [PATCH RFC] xen/balloon: check whether a page is pointed to scratch page MFN
On Thu, 5 Sep 2013, Wei Liu wrote:> Now that balloon pages might have two kind of P2M entries, a) > INVALID_P2M_ENTRY when that page is never used, b) mfn of one of the > many ballooned scratch pages. Refelct this in increase_reservation to > avoid hitting BUG_ON. > > Signed-off-by: Wei Liu <wei.liu2@citrix.com> > --- > drivers/xen/balloon.c | 21 ++++++++++++++++++++- > 1 file changed, 20 insertions(+), 1 deletion(-) > > diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c > index 3101cf6..63a7e5e 100644 > --- a/drivers/xen/balloon.c > +++ b/drivers/xen/balloon.c > @@ -305,6 +305,24 @@ static enum bp_state reserve_additional_memory(long credit) > } > #endif /* CONFIG_XEN_BALLOON_MEMORY_HOTPLUG */ > > +static bool p2m_is_ballooned_scratch_page(unsigned long pfn) > +{ > + int cpu; > + unsigned long bpfn; > + struct page *bpage; > + > + for_each_possible_cpu(cpu) { > + bpage = per_cpu(balloon_scratch_page, cpu); > + if (!bpage) > + continue; > + bpfn = page_to_pfn(bpage); > + if (pfn_to_mfn(pfn) == pfn_to_mfn(bpfn))Even though it is true that this function doesn''t need to be very fast, we might as well try to write in an efficient way. For example it might be worth calculating pfn_to_mfn(pfn) just once outside the loop?> + return true; > + } > + > + return false; > +} > + > static enum bp_state increase_reservation(unsigned long nr_pages) > { > int rc; > @@ -350,7 +368,8 @@ static enum bp_state increase_reservation(unsigned long nr_pages) > > pfn = page_to_pfn(page); > BUG_ON(!xen_feature(XENFEAT_auto_translated_physmap) && > - phys_to_machine_mapping_valid(pfn)); > + phys_to_machine_mapping_valid(pfn) && > + !p2m_is_ballooned_scratch_page(pfn)); > > set_phys_to_machine(pfn, frame_list[i]); > > -- > 1.7.10.4 >
David Vrabel
2013-Sep-05 14:25 UTC
Re: [PATCH RFC] xen/balloon: check whether a page is pointed to scratch page MFN
On 05/09/13 11:00, Wei Liu wrote:> Now that balloon pages might have two kind of P2M entries, a) > INVALID_P2M_ENTRY when that page is never used, b) mfn of one of the > many ballooned scratch pages. Refelct this in increase_reservation to > avoid hitting BUG_ON.This BUG_ON() isn''t useful, please just remove it. David
Wei Liu
2013-Sep-05 14:27 UTC
Re: [PATCH RFC] xen/balloon: check whether a page is pointed to scratch page MFN
On Thu, Sep 05, 2013 at 03:21:32PM +0100, Stefano Stabellini wrote:> On Thu, 5 Sep 2013, Wei Liu wrote: > > Now that balloon pages might have two kind of P2M entries, a) > > INVALID_P2M_ENTRY when that page is never used, b) mfn of one of the > > many ballooned scratch pages. Refelct this in increase_reservation to > > avoid hitting BUG_ON. > > > > Signed-off-by: Wei Liu <wei.liu2@citrix.com> > > --- > > drivers/xen/balloon.c | 21 ++++++++++++++++++++- > > 1 file changed, 20 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c > > index 3101cf6..63a7e5e 100644 > > --- a/drivers/xen/balloon.c > > +++ b/drivers/xen/balloon.c > > @@ -305,6 +305,24 @@ static enum bp_state reserve_additional_memory(long credit) > > } > > #endif /* CONFIG_XEN_BALLOON_MEMORY_HOTPLUG */ > > > > +static bool p2m_is_ballooned_scratch_page(unsigned long pfn) > > +{ > > + int cpu; > > + unsigned long bpfn; > > + struct page *bpage; > > + > > + for_each_possible_cpu(cpu) { > > + bpage = per_cpu(balloon_scratch_page, cpu); > > + if (!bpage) > > + continue; > > + bpfn = page_to_pfn(bpage); > > + if (pfn_to_mfn(pfn) == pfn_to_mfn(bpfn)) > > Even though it is true that this function doesn''t need to be very fast, > we might as well try to write in an efficient way. > For example it might be worth calculating pfn_to_mfn(pfn) just once > outside the loop? >Good point.
Wei Liu
2013-Sep-05 14:29 UTC
Re: [PATCH RFC] xen/balloon: check whether a page is pointed to scratch page MFN
On Thu, Sep 05, 2013 at 03:25:55PM +0100, David Vrabel wrote:> On 05/09/13 11:00, Wei Liu wrote: > > Now that balloon pages might have two kind of P2M entries, a) > > INVALID_P2M_ENTRY when that page is never used, b) mfn of one of the > > many ballooned scratch pages. Refelct this in increase_reservation to > > avoid hitting BUG_ON. > > This BUG_ON() isn''t useful, please just remove it. >I''m fine with that. I''m just less confident to do so. :-) Now that you and Stefano both voted for this option, haha.> David
The BUG_ON in increase_reservation is wrong as we have P2M entry ballooned out page set to balloon scratch page, so it might have a valid P2M entry at that point. Signed-off-by: Wei Liu <wei.liu2@citrix.com> --- drivers/xen/balloon.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c index 3101cf6..b52df76 100644 --- a/drivers/xen/balloon.c +++ b/drivers/xen/balloon.c @@ -349,8 +349,6 @@ static enum bp_state increase_reservation(unsigned long nr_pages) BUG_ON(page == NULL); pfn = page_to_pfn(page); - BUG_ON(!xen_feature(XENFEAT_auto_translated_physmap) && - phys_to_machine_mapping_valid(pfn)); set_phys_to_machine(pfn, frame_list[i]); -- 1.7.10.4
Stefano Stabellini
2013-Sep-05 15:43 UTC
Re: [PATCH] xen/balloon: remove BUG_ON in increase_reservation
On Thu, 5 Sep 2013, Wei Liu wrote:> The BUG_ON in increase_reservation is wrong as we have P2M entry > ballooned out page set to balloon scratch page, so it might have a valid > P2M entry at that point. > > Signed-off-by: Wei Liu <wei.liu2@citrix.com>Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>> drivers/xen/balloon.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c > index 3101cf6..b52df76 100644 > --- a/drivers/xen/balloon.c > +++ b/drivers/xen/balloon.c > @@ -349,8 +349,6 @@ static enum bp_state increase_reservation(unsigned long nr_pages) > BUG_ON(page == NULL); > > pfn = page_to_pfn(page); > - BUG_ON(!xen_feature(XENFEAT_auto_translated_physmap) && > - phys_to_machine_mapping_valid(pfn)); > > set_phys_to_machine(pfn, frame_list[i]); > > -- > 1.7.10.4 > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel >