Re: [Xen-devel] [PATCH] xenpaging: check p2mt in p2m_mem_paging functions

[Tim Deegan <tim@xxxxxxx>]

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@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel