From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Vrabel Subject: Re: [PATCH] Xen backend support for paged out grant targets. Date: Fri, 31 Aug 2012 15:32:10 +0100 Message-ID: <5040CAEA.7000600@citrix.com> References: <1346086287-17674-1-git-send-email-andres@lagarcavilla.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1346086287-17674-1-git-send-email-andres@lagarcavilla.org> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: andres@lagarcavilla.org Cc: Andres Lagar-Cavilla , Konrad Rzeszutek Wilk , xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 27/08/12 17:51, andres@lagarcavilla.org wrote: > From: Andres Lagar-Cavilla > > Since Xen-4.2, hvm domains may have portions of their memory paged out. When a > foreign domain (such as dom0) attempts to map these frames, the map will > initially fail. The hypervisor returns a suitable errno, and kicks an > asynchronous page-in operation carried out by a helper. The foreign domain is > expected to retry the mapping operation until it eventually succeeds. The > foreign domain is not put to sleep because itself could be the one running the > pager assist (typical scenario for dom0). > > This patch adds support for this mechanism for backend drivers using grant > mapping and copying operations. Specifically, this covers the blkback and > gntdev drivers (which map foregin grants), and the netback driver (which copies > foreign grants). > > * Add GNTST_eagain, already exposed by Xen, to the grant interface. > * Add a retry method for grants that fail with GNTST_eagain (i.e. because the > target foregin frame is paged out). > * Insert hooks with appropriate macro decorators in the aforementioned drivers. I think you should implement wrappers around HYPERVISOR_grant_table_op() have have the wrapper do the retries instead of every backend having to check for EAGAIN and issue the retries itself. Similar to the gnttab_map_grant_no_eagain() function you've already added. Why do some operations not retry anyway? > +void > +gnttab_retry_eagain_gop(unsigned int cmd, void *gop, int16_t *status, > + const char *func) > +{ > + u8 delay = 1; > + > + do { > + BUG_ON(HYPERVISOR_grant_table_op(cmd, gop, 1)); > + if (*status == GNTST_eagain) > + msleep(delay++); > + } while ((*status == GNTST_eagain) && delay); Terminating the loop when delay wraps is a bit subtle. Why not make delay unsigned and check delay <= MAX_DELAY? Would it be sensible to ramp the delay faster? Perhaps double each iteration with a maximum possible delay of e.g., 256 ms. > +#define gnttab_map_grant_no_eagain(_gop) \ > +do { \ > + if ( HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, (_gop), 1)) \ > + BUG(); \ > + if ((_gop)->status == GNTST_eagain) \ > + gnttab_retry_eagain_map((_gop)); \ > +} while(0) Inline functions, please. David