On Thu, Feb 23, Tim Deegan wrote:
> This is based on an earlier RFC patch by Olaf Hering, but heavily
> simplified (removing a per-gfn queue of waiting vcpus in favour of
> a scan of all vcpus on page-in).
Tim, thanks for that work. From just reading the change it looks ok.
A few comments below:
> +++ b/xen/arch/x86/mm/p2m.c Thu Feb 23 16:18:09 2012 +0000
> @@ -160,13 +160,49 @@ mfn_t __get_gfn_type_access(struct p2m_d
> }
>
> /* For now only perform locking on hap domains */
> - if ( locked && (hap_enabled(p2m->domain)) )
> + locked = locked && hap_enabled(p2m->domain);
> +
> +#ifdef __x86_64__
> +again:
> +#endif
> + if ( locked )
> /* Grab the lock here, don't release until put_gfn */
> gfn_lock(p2m, gfn, 0);
>
> mfn = p2m->get_entry(p2m, gfn, t, a, q, page_order);
>
> #ifdef __x86_64__
> + if ( p2m_is_paging(*t) && (q & P2M_ALLOC)
> + && p2m->domain == current->domain )
> + {
> + if ( locked )
> + gfn_unlock(p2m, gfn, 0);
> +
> + /* Ping the pager */
> + if ( *t == p2m_ram_paging_out || *t == p2m_ram_paged )
> + p2m_mem_paging_populate(p2m->domain, gfn);
> +
> + /* Wait until the pager finishes paging it in */
> + current->arch.mem_paging_gfn = gfn;
> + wait_event(current->arch.mem_paging_wq, ({
> + int done;
> + mfn = p2m->get_entry(p2m, gfn, t, a, 0, page_order);
> + done = (*t != p2m_ram_paging_in);
I assume p2m_mem_paging_populate() will not return until the state is
forwarded to p2m_ram_paging_in. Maybe p2m_is_paging(*t) would make it
more obvious what this check is supposed to do.
> + /* Safety catch: it _should_ be safe to wait here
> + * but if it's not, crash the VM, not the host */
> + if ( in_atomic() )
> + {
> + WARN();
> + domain_crash(p2m->domain);
> + done = 1;
> + }
> + done;
> + }));
> + goto again;
> + }
> +#endif
> +
> +#ifdef __x86_64__
> if ( (q & P2M_UNSHARE) && p2m_is_shared(*t) )
> {
> ASSERT(!p2m_is_nestedp2m(p2m));
> @@ -946,17 +982,17 @@ void p2m_mem_paging_drop_page(struct dom
> * This function needs to be called whenever gfn_to_mfn() returns any of the p2m
> * paging types because the gfn may not be backed by a mfn.
> *
> - * The gfn can be in any of the paging states, but the pager needs only be
> - * notified when the gfn is in the paging-out path (paging_out or paged). This
> - * function may be called more than once from several vcpus. If the vcpu belongs
> - * to the guest, the vcpu must be stopped and the pager notified that the vcpu
> - * was stopped. The pager needs to handle several requests for the same gfn.
> + * The gfn can be in any of the paging states, but the pager needs only
> + * be notified when the gfn is in the paging-out path (paging_out or
> + * paged). This function may be called more than once from several
> + * vcpus. The pager needs to handle several requests for the same gfn.
> *
> - * If the gfn is not in the paging-out path and the vcpu does not belong to the
> - * guest, nothing needs to be done and the function assumes that a request was
> - * already sent to the pager. In this case the caller has to try again until the
> - * gfn is fully paged in again.
> + * If the gfn is not in the paging-out path nothing needs to be done and
> + * the function assumes that a request was already sent to the pager.
> + * In this case the caller has to try again until the gfn is fully paged
> + * in again.
> */
> +
> void p2m_mem_paging_populate(struct domain *d, unsigned long gfn)
> {
> struct vcpu *v = current;
> @@ -965,6 +1001,7 @@ void p2m_mem_paging_populate(struct doma
> p2m_access_t a;
> mfn_t mfn;
> struct p2m_domain *p2m = p2m_get_hostp2m(d);
> + int send_request = 0;
Is that variable supposed to be used?
Perhaps the feature to fast-forward (or rollback) from
p2m_ram_paging_out to p2m_ram_rw could be a separate patch. My initial
version of this patch did not have a strict requirement for this
feature, if I remember correctly.
> /* We're paging. There should be a ring */
> int rc = mem_event_claim_slot(d, &d->mem_event->paging);
> @@ -987,19 +1024,22 @@ void p2m_mem_paging_populate(struct doma
> /* Evict will fail now, tag this request for pager */
> if ( p2mt == p2m_ram_paging_out )
> req.flags |= MEM_EVENT_FLAG_EVICT_FAIL;
> -
> - set_p2m_entry(p2m, gfn, mfn, PAGE_ORDER_4K, p2m_ram_paging_in, a);
> + if ( p2mt == p2m_ram_paging_out && mfn_valid(mfn) && v->domain == d )
> + /* Short-cut back to paged-in state (but not for foreign
> + * mappings, or the pager couldn't map it to page it out) */
> + set_p2m_entry(p2m, gfn, mfn, PAGE_ORDER_4K,
> + paging_mode_log_dirty(d)
> + ? p2m_ram_logdirty : p2m_ram_rw, a);
> + else
> + {
> + set_p2m_entry(p2m, gfn, mfn, PAGE_ORDER_4K, p2m_ram_paging_in, a);
> + send_request = 1;
> + }
> }
> gfn_unlock(p2m, gfn, 0);
>
> - /* Pause domain if request came from guest and gfn has paging type */
> - if ( p2m_is_paging(p2mt) && v->domain == d )
> - {
> - vcpu_pause_nosync(v);
> - req.flags |= MEM_EVENT_FLAG_VCPU_PAUSED;
> - }
> /* No need to inform pager if the gfn is not in the page-out path */
> - else if ( p2mt != p2m_ram_paging_out && p2mt != p2m_ram_paged )
> + if ( p2mt != p2m_ram_paging_out && p2mt != p2m_ram_paged )
> {
> /* gfn is already on its way back and vcpu is not paused */
> mem_event_cancel_slot(d, &d->mem_event->paging);
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel