On Fri, 2006-06-16 at 11:00 +0900, Isaku Yamahata wrote:
> 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).
Argh. Yes, you are correct.
>
> > > @@ -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 for the clarification. I'll have to think about this a bit.
Guaranteeing the loop terminates could be a lot more expensive than
the risk of not terminating. Like you say, the scenario is unlikely.
--
Ciao,
al
----------------------------------------------------------------------
Al Stone Alter Ego:
Open Source and Linux R&D Debian Developer
Hewlett-Packard Company http://www.debian.org
E-mail: ahs3@xxxxxxxxx ahs3@xxxxxxxxxx
----------------------------------------------------------------------
_______________________________________________
Xen-ia64-devel mailing list
Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ia64-devel
|
Home