Re: [PATCH] xenpaging: check p2mt in p2m_mem_paging functions

[Olaf Hering <olaf@aepfle.de>]

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