* [PATCH] xenpaging: check p2mt in p2m_mem_paging functions
@ 2011-10-11 12:42 Olaf Hering
2011-10-13 11:27 ` Tim Deegan
0 siblings, 1 reply; 4+ messages in thread
From: Olaf Hering @ 2011-10-11 12:42 UTC (permalink / raw)
To: xen-devel
# 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);
}
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] xenpaging: check p2mt in p2m_mem_paging functions
2011-10-11 12:42 [PATCH] xenpaging: check p2mt in p2m_mem_paging functions Olaf Hering
@ 2011-10-13 11:27 ` Tim Deegan
2011-10-13 12:14 ` Olaf Hering
0 siblings, 1 reply; 4+ messages in thread
From: Tim Deegan @ 2011-10-13 11:27 UTC (permalink / raw)
To: Olaf Hering; +Cc: xen-devel
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.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] xenpaging: check p2mt in p2m_mem_paging functions
2011-10-13 11:27 ` Tim Deegan
@ 2011-10-13 12:14 ` Olaf Hering
2011-10-13 14:27 ` Tim Deegan
0 siblings, 1 reply; 4+ messages in thread
From: Olaf Hering @ 2011-10-13 12:14 UTC (permalink / raw)
To: Tim Deegan; +Cc: xen-devel
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] xenpaging: check p2mt in p2m_mem_paging functions
2011-10-13 12:14 ` Olaf Hering
@ 2011-10-13 14:27 ` Tim Deegan
0 siblings, 0 replies; 4+ messages in thread
From: Tim Deegan @ 2011-10-13 14:27 UTC (permalink / raw)
To: Olaf Hering; +Cc: xen-devel
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.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-10-13 14:27 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-10-11 12:42 [PATCH] xenpaging: check p2mt in p2m_mem_paging functions Olaf Hering
2011-10-13 11:27 ` Tim Deegan
2011-10-13 12:14 ` Olaf Hering
2011-10-13 14:27 ` Tim Deegan
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.