On Thu, Jun 15, 2006 at 01:14:06PM -0600, Al Stone wrote:
> In all of the functions above, it appears that the return value of
> a function (pte_offset_map()) is being returned as a volatile result
> from each of the functions. Is that really needed? I'm not sure
> it helps in this case, but I could be wrong.
It seems that you are confusing
volatile pte_t* (a pointer to volatile pte_t) with
pte_t* volatile (a volatile pointer to pte_t).
> > @@ -986,21 +1034,42 @@ destroy_grant_host_mapping(unsigned long
> > }
> >
> > pte = lookup_noalloc_domain_pte(d, gpaddr);
> > - if (pte == NULL || !pte_present(*pte) || pte_pfn(*pte) != mfn)
> > + if (pte == NULL) {
> > + DPRINTK("%s: gpaddr 0x%lx mfn 0x%lx\n", __func__, gpaddr, mfn);
> > return GNTST_general_error;
> > -
> > - // update pte
> > - old_pte = ptep_get_and_clear(&d->arch.mm, gpaddr, pte);
> > - if (pte_present(old_pte)) {
> > - old_mfn = pte_pfn(old_pte);
> > - } else {
> > + }
> > +
> > + again:
> > + cur_arflags = pte_val(*pte) & ~_PAGE_PPN_MASK;
> > + cur_pte = pfn_pte(mfn, __pgprot(cur_arflags));
> > + if (!pte_present(cur_pte)) {
> > + DPRINTK("%s: gpaddr 0x%lx mfn 0x%lx cur_pte 0x%lx\n",
> > + __func__, gpaddr, mfn, pte_val(cur_pte));
> > return GNTST_general_error;
> > }
> > - domain_page_flush(d, gpaddr, old_mfn, INVALID_MFN);
> > -
> > - old_page = mfn_to_page(old_mfn);
> > - BUG_ON(page_get_owner(old_page) == d);//try_to_clear_PGC_allocate(d,
> > page) is not needed.
> > - put_page(old_page);
> > + new_pte = __pte(0);
> > +
> > + old_pte = ptep_cmpxchg_rel(&d->arch.mm, gpaddr, pte, cur_pte,
> > new_pte);
> > + if (unlikely(!pte_present(old_pte))) {
> > + DPRINTK("%s: gpaddr 0x%lx mfn 0x%lx cur_pte 0x%lx old_pte
> > 0x%lx\n",
> > + __func__, gpaddr, mfn, pte_val(cur_pte),
> > pte_val(old_pte));
> > + return GNTST_general_error;
> > + }
> > + if (unlikely(pte_val(cur_pte) != pte_val(old_pte))) {
> > + if (pte_pfn(old_pte) == mfn) {
> > + goto again;
>
> Maybe I'm just being paranoid, but is there *any* chance this goto loop
> will not terminate?
Yes there is.
If there are more than two vcpus and enough physical cpus, and
other vcpus keep chainging the entry, the goto loop won't terminate.
I think it is very unlikey in practice.
Thanks.
--
yamahata
_______________________________________________
Xen-ia64-devel mailing list
Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ia64-devel
|