Olaf Hering
2011-Oct-11 12:42 UTC
[Xen-devel] [PATCH] xenpaging: check p2mt in p2m_mem_paging functions
# HG changeset patch
# User Olaf Hering <olaf@aepfle.de>
# Date 1318336928 -7200
# Node ID bc64a435d572680efb221445c28583d03d4eb175
# Parent bdd49540f1e1c803d01c88adb67c6ce01e2d00d8
xenpaging: check p2mt in p2m_mem_paging functions
Add checks to forward the p2m_ram_paging* state properly during page-in.
Resume can be called several times if several vcpus called populate for
the gfn. Finish resume only once and print some debug for other cases.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
diff -r bdd49540f1e1 -r bc64a435d572 xen/arch/x86/mm/p2m.c
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -852,16 +852,22 @@ int p2m_mem_paging_prep(struct domain *d
p2m_access_t a;
mfn_t mfn;
struct p2m_domain *p2m = p2m_get_hostp2m(d);
- int ret = -ENOMEM;
+ int ret;
p2m_lock(p2m);
mfn = p2m->get_entry(p2m, gfn, &p2mt, &a, p2m_query, NULL);
+ ret = -ENOENT;
+ /* Allow only missing pages */
+ if ( p2mt != p2m_ram_paging_in_start )
+ goto out;
+
/* Allocate a page if the gfn does not have one yet */
if ( !mfn_valid(mfn) )
{
/* Get a free page */
+ ret = -ENOMEM;
page = alloc_domheap_page(p2m->domain, 0);
if ( unlikely(page == NULL) )
goto out;
@@ -897,9 +903,16 @@ void p2m_mem_paging_resume(struct domain
{
p2m_lock(p2m);
mfn = p2m->get_entry(p2m, rsp.gfn, &p2mt, &a, p2m_query,
NULL);
- set_p2m_entry(p2m, rsp.gfn, mfn, 0, p2m_ram_rw, a);
- set_gpfn_from_mfn(mfn_x(mfn), rsp.gfn);
- audit_p2m(p2m, 1);
+ /* Allow only pages which were prepared properly or pages which were
nominated but not evicted */
+ if ( mfn_valid(mfn) && ( p2mt == p2m_ram_paging_in || p2mt ==
p2m_ram_paging_in_start ) )
+ {
+ set_p2m_entry(p2m, rsp.gfn, mfn, 0, p2m_ram_rw, a);
+ set_gpfn_from_mfn(mfn_x(mfn), rsp.gfn);
+ audit_p2m(p2m, 1);
+ /* May be called more than once if the gfn was populate from different
vcpus */
+ } else if ( p2mt != p2m_ram_rw ) {
+ printk("resume: %d %lx %x %lx\n", d->domain_id,
rsp.gfn, p2mt, mfn_x(mfn));
+ }
p2m_unlock(p2m);
}
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Tim Deegan
2011-Oct-13 11:27 UTC
Re: [Xen-devel] [PATCH] xenpaging: check p2mt in p2m_mem_paging functions
Hi, At 14:42 +0200 on 11 Oct (1318344151), Olaf Hering wrote:> @@ -897,9 +903,16 @@ void p2m_mem_paging_resume(struct domain > { > p2m_lock(p2m); > mfn = p2m->get_entry(p2m, rsp.gfn, &p2mt, &a, p2m_query, NULL); > - set_p2m_entry(p2m, rsp.gfn, mfn, 0, p2m_ram_rw, a); > - set_gpfn_from_mfn(mfn_x(mfn), rsp.gfn); > - audit_p2m(p2m, 1); > + /* Allow only pages which were prepared properly or pages which were nominated but not evicted */ > + if ( mfn_valid(mfn) && ( p2mt == p2m_ram_paging_in || p2mt == p2m_ram_paging_in_start ) )Wouldn''t a nominated-but-not-evicted page have type p2m_ram_paging_out?> + { > + set_p2m_entry(p2m, rsp.gfn, mfn, 0, p2m_ram_rw, a); > + set_gpfn_from_mfn(mfn_x(mfn), rsp.gfn); > + audit_p2m(p2m, 1); > + /* May be called more than once if the gfn was populate from different vcpus */ > + } else if ( p2mt != p2m_ram_rw ) { > + printk("resume: %d %lx %x %lx\n", d->domain_id, rsp.gfn, p2mt, mfn_x(mfn));This should be a gdprintk of some kind, probably XENLOG_WARNING unless it happens a lot. Cheers, Tim. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Olaf Hering
2011-Oct-13 12:14 UTC
Re: [Xen-devel] [PATCH] xenpaging: check p2mt in p2m_mem_paging functions
On Thu, Oct 13, Tim Deegan wrote:> Hi, > > At 14:42 +0200 on 11 Oct (1318344151), Olaf Hering wrote: > > @@ -897,9 +903,16 @@ void p2m_mem_paging_resume(struct domain > > { > > p2m_lock(p2m); > > mfn = p2m->get_entry(p2m, rsp.gfn, &p2mt, &a, p2m_query, NULL); > > - set_p2m_entry(p2m, rsp.gfn, mfn, 0, p2m_ram_rw, a); > > - set_gpfn_from_mfn(mfn_x(mfn), rsp.gfn); > > - audit_p2m(p2m, 1); > > + /* Allow only pages which were prepared properly or pages which were nominated but not evicted */ > > + if ( mfn_valid(mfn) && ( p2mt == p2m_ram_paging_in || p2mt == p2m_ram_paging_in_start ) ) > > Wouldn''t a nominated-but-not-evicted page have type p2m_ram_paging_out?Yes, but in the page-in path it will be p2m_ram_paging_in_start with a valid mfn.> > + { > > + set_p2m_entry(p2m, rsp.gfn, mfn, 0, p2m_ram_rw, a); > > + set_gpfn_from_mfn(mfn_x(mfn), rsp.gfn); > > + audit_p2m(p2m, 1); > > + /* May be called more than once if the gfn was populate from different vcpus */ > > + } else if ( p2mt != p2m_ram_rw ) { > > + printk("resume: %d %lx %x %lx\n", d->domain_id, rsp.gfn, p2mt, mfn_x(mfn)); > > This should be a gdprintk of some kind, probably XENLOG_WARNING unless > it happens a lot.Its just debug, perhaps that gfn was already use for something else while the pager processed multiple page-in requests from different vcpus. Do you want me to resend this patch? Olaf _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Tim Deegan
2011-Oct-13 14:27 UTC
Re: [Xen-devel] [PATCH] xenpaging: check p2mt in p2m_mem_paging functions
At 14:14 +0200 on 13 Oct (1318515242), Olaf Hering wrote:> On Thu, Oct 13, Tim Deegan wrote: > > > Hi, > > > > At 14:42 +0200 on 11 Oct (1318344151), Olaf Hering wrote: > > > @@ -897,9 +903,16 @@ void p2m_mem_paging_resume(struct domain > > > { > > > p2m_lock(p2m); > > > mfn = p2m->get_entry(p2m, rsp.gfn, &p2mt, &a, p2m_query, NULL); > > > - set_p2m_entry(p2m, rsp.gfn, mfn, 0, p2m_ram_rw, a); > > > - set_gpfn_from_mfn(mfn_x(mfn), rsp.gfn); > > > - audit_p2m(p2m, 1); > > > + /* Allow only pages which were prepared properly or pages which were nominated but not evicted */ > > > + if ( mfn_valid(mfn) && ( p2mt == p2m_ram_paging_in || p2mt == p2m_ram_paging_in_start ) ) > > > > Wouldn''t a nominated-but-not-evicted page have type p2m_ram_paging_out? > > Yes, but in the page-in path it will be p2m_ram_paging_in_start with a > valid mfn.Eurgh. OK, in that case this comment should probably explain why. :)> > > + { > > > + set_p2m_entry(p2m, rsp.gfn, mfn, 0, p2m_ram_rw, a); > > > + set_gpfn_from_mfn(mfn_x(mfn), rsp.gfn); > > > + audit_p2m(p2m, 1); > > > + /* May be called more than once if the gfn was populate from different vcpus */ > > > + } else if ( p2mt != p2m_ram_rw ) { > > > + printk("resume: %d %lx %x %lx\n", d->domain_id, rsp.gfn, p2mt, mfn_x(mfn)); > > > > This should be a gdprintk of some kind, probably XENLOG_WARNING unless > > it happens a lot. > > Its just debug, perhaps that gfn was already use for something else > while the pager processed multiple page-in requests from different > vcpus.OK, so should it be removed entirely?> Do you want me to resend this patch?Yes, please. Tim. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel