From mboxrd@z Thu Jan 1 00:00:00 1970 From: Olaf Hering Subject: Re: [PATCH] x86/mm: Improve ring management for memory events. Do not lose guest events Date: Fri, 13 Jan 2012 10:50:22 +0100 Message-ID: <20120113095022.GA18130@aepfle.de> References: Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Andres Lagar-Cavilla Cc: andres@gridcentric.ca, xen-devel@lists.xensource.com, tim@xen.org, adin@gridcentric.ca List-Id: xen-devel@lists.xenproject.org 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