> Rest of the code looks fine.
Except for a couple of bugs that took awhile to track down :-(
First (stupid one), get_page returns 1 for success and 0 for failure
and I was testing for failure by <= 0 (the Linux convention, oops).
Second, and much more subtle, apparently get_page_and_type
must always be bracketed with a put_page_and_type, and get_page
must always be bracketed with a put_page, i.e. these can't
be mixed. It appears that the "and_type" versions take
two references, not one so mixing them results in bad
non-obvious BUGs when pages are cycled through the heap.
> -----Original Message-----
> From: Keir Fraser [mailto:keir.fraser@xxxxxxxxxxxxx]
> Sent: Tuesday, September 21, 2010 1:44 AM
> To: Dan Magenheimer; Jan Beulich
> Cc: Xen-devel
> Subject: Re: [Xen-devel] Re: xen crash in tmem: checking a xen pfn for
> domain ownership
>
> On 21/09/2010 00:03, "Dan Magenheimer" <dan.magenheimer@xxxxxxxxxx>
> wrote:
>
> > if ( is_hvm_domain(current->domain) )
> > {
> > xmfn = mfn_x(gfn_to_mfn_unshare(current->domain, cmfn, &t,
> 2));
> > if (t != p2m_ram_rw || xmfn == INVALID_MFN)
> > return NULL;
> > }
> > else
> > {
> > xmfn = cmfn;
> > if (!mfn_valid(xmfn))
> > return NULL;
> > }
>
> This is needlessly cumbersome. You can do it without the if(is_hvm):
>
> xmfn = mfn_x(gfn_to_mfn(current->domain, cmfn, &t);
> if ((t != p2m_ram_rw) || !mfn_valid(xmfn))
> return NULL;
>
> (Didn't use hfn_to_mfn_unshare as you have decided against it.)
>
> Rest of the code looks fine.
>
> -- Keir
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home