On Wed, Oct 22, 2008 at 01:56:05PM +0800, Xu, Anthony wrote:
> >> diff -r e1ed3e5cd001 xen/arch/ia64/xen/mm.c
> >> --- a/xen/arch/ia64/xen/mm.c Tue Oct 21 18:55:22 2008 +0800
> >> +++ b/xen/arch/ia64/xen/mm.c Tue Oct 21 19:13:45 2008 +0800 @@
> >> -189,6 +189,10 @@
> >>
> >> static void __xencomm_mark_dirty(struct domain *d,
> >> unsigned long addr, unsigned int
> >> len); + +static void
> >> +zap_domain_page_one(struct domain *d, unsigned long mpaddr,
> >> + int clear_PGC_allocate, unsigned long mfn);
> >>
> >> extern unsigned long ia64_iobase;
> >>
> >> @@ -908,20 +912,21 @@
> >> unsigned long flags)
> >> {
> >> volatile pte_t *pte;
> >> - pte_t old_pte;
> >> pte_t new_pte;
> >> pte_t ret_pte;
> >> unsigned long prot = flags_to_prot(flags);
> >>
> >> pte = lookup_alloc_domain_pte(d, mpaddr);
> >> + new_pte = pfn_pte(physaddr >> PAGE_SHIFT, __pgprot(prot));
> >> + if(pte_val(new_pte) == pte_val(*pte))
> >> + return 0;
> >>
> >> - old_pte = __pte(0);
> >> - new_pte = pfn_pte(physaddr >> PAGE_SHIFT, __pgprot(prot));
> >> - ret_pte = ptep_cmpxchg_rel(&d->arch.mm, mpaddr, pte, old_pte,
> >> new_pte);
> >> - if (pte_val(ret_pte) == pte_val(old_pte)) {
> >> - smp_mb();
> >> - return 0;
> >> - }
> >> + /* for assigned MMIO, the old pte may be set to _PAGE_IO
> >> attribute, + * so zap it first, then set up it.
> >> + */
> >> +
> >> + zap_domain_page_one(d, mpaddr, 1, INVALID_MFN);
> >> + ret_pte = ptep_xchg(&d->arch.mm, mpaddr, pte, new_pte);
> >>
> >> // dom0 tries to map real machine's I/O region, but failed.
> >> // It is very likely that dom0 doesn't boot correctly because
> >
> > Hmmm, are you really sure that the above is SMP-safe?
> > We are touching p2m table locklessly so we must be extremely
> > careful. The above hunk split the atomic operation into two phase
> > and makes the following logic not make sense.
> >
> >
>
> Yes, it is not SMP-safe there is lock for p2m.
> Modifying p2m is not a frequent operation, why not add a lock for it?
It is frequent to read p2m table. So lockless approach was adopted
for scalability.
It doesn't make sense to lock around only writer side.
thanks,
--
yamahata
_______________________________________________
Xen-ia64-devel mailing list
Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ia64-devel
|