Re: [PATCH] x86/mm: Improve ring management for memory events. Do not lose guest events

[Olaf Hering <olaf@aepfle.de>]

On Wed, Jan 11, Andres Lagar-Cavilla wrote:

A few comments:

> -static int mem_event_disable(struct mem_event_domain *med)
> +static int mem_event_ring_available(struct mem_event_domain *med)
>  {
> -    unmap_domain_page(med->ring_page);
> -    med->ring_page = NULL;
> +    int avail_req = RING_FREE_REQUESTS(&med->front_ring);
> +    avail_req -= med->target_producers;
> +    avail_req -= med->foreign_producers;
>  
> -    unmap_domain_page(med->shared_page);
> -    med->shared_page = NULL;
> +    BUG_ON(avail_req < 0);
> +
> +    return avail_req;
> +}
> +

mem_event_ring_available() should return unsigned since the values it
provides can only be positive. The function itself enforces this.

> -void p2m_mem_paging_drop_page(struct domain *d, unsigned long gfn)
> +int p2m_mem_paging_drop_page(struct domain *d, unsigned long gfn)
>  {
> -    struct vcpu *v = current;
>      mem_event_request_t req;
>  
> -    /* Check that there's space on the ring for this request */
> -    if ( mem_event_check_ring(d, &d->mem_event->paging) == 0)
> -    {
> -        /* Send release notification to pager */
> -        memset(&req, 0, sizeof(req));
> -        req.flags |= MEM_EVENT_FLAG_DROP_PAGE;
> -        req.gfn = gfn;
> -        req.vcpu_id = v->vcpu_id;
> +    /* We allow no ring in this unique case, because it won't affect
> +     * correctness of the guest execution at this point.  If this is the only
> +     * page that happens to be paged-out, we'll be okay..  but it's likely the
> +     * guest will crash shortly anyways. */
> +    int rc = mem_event_claim_slot(d, &d->mem_event->paging);
> +    if ( rc < 0 )
> +        return rc;
>  
> -        mem_event_put_request(d, &d->mem_event->paging, &req);
> -    }
> +    /* Send release notification to pager */
> +    memset(&req, 0, sizeof(req));
> +    req.type = MEM_EVENT_TYPE_PAGING;
> +    req.gfn = gfn;
> +    req.flags = MEM_EVENT_FLAG_DROP_PAGE;
> +
> +    mem_event_put_request(d, &d->mem_event->paging, &req);
> +    return 0;
>  }

p2m_mem_paging_drop_page() should remain void because the caller has
already done its work, making it not restartable. Also it is only called
when a gfn is in paging state, which I'm sure can not happen without a
ring.

And quilt says:
Warning: trailing whitespace in lines 167,254 of xen/arch/x86/mm/mem_event.c
Warning: trailing whitespace in line 168 of xen/common/memory.c
Warning: trailing whitespace in line 1127 of xen/arch/x86/mm/p2m.c


Olaf