page-pipe: do not allow pipe sharing between different PPB types

Submitted by Mike Rapoport on Feb. 11, 2018, 1:53 p.m.

Details

Message ID 1518357195-23077-1-git-send-email-rppt@linux.vnet.ibm.com
State Accepted
Series "page-pipe: do not allow pipe sharing between different PPB types"
Commit e78f0a5f39eeb3c70bfe93a97ee44d3833ea47bd
Headers show

Commit Message

Mike Rapoport Feb. 11, 2018, 1:53 p.m.
Currently, if pipe is shared between lazy and non-lazy PPBs lazy migration
fails because data that should be transfered on demand is spliced into the
images. Preventing pipe sharing between PPBs of different type resolves
this issue.
In order to still minimize pipe fragmentation, we track the last pipe that
was used for certain PPB type and re-use it for the PPB of the same type.

Signed-off-by: Mike Rapoport <rppt@linux.vnet.ibm.com>
---
 criu/include/page-pipe.h | 11 +++++++++++
 criu/page-pipe.c         | 43 +++++++++++++++++++++++++++++++++++++------
 2 files changed, 48 insertions(+), 6 deletions(-)

Patch hide | download patch | download mbox

diff --git a/criu/include/page-pipe.h b/criu/include/page-pipe.h
index 053ae39..8fa1bfa 100644
--- a/criu/include/page-pipe.h
+++ b/criu/include/page-pipe.h
@@ -102,12 +102,23 @@  struct page_pipe_buf {
 	struct list_head l;	/* links into page_pipe->bufs */
 };
 
+/*
+ * Page pipe buffers with different flags cannot share the same pipe.
+ * We track the last ppb that was used for each type separately in the
+ * prev[] array in the struct page_pipe (below).
+ * Currently we have 2 types: the buffers that are always stored in
+ * the images and the buffers that are lazily migrated
+ */
+#define PP_PIPE_TYPES	2
+
 #define PP_HOLE_PARENT (1 << 0)
 
 struct page_pipe {
 	unsigned int nr_pipes;	/* how many page_pipe_bufs in there */
 	struct list_head bufs;	/* list of bufs */
 	struct list_head free_bufs;	/* list of bufs */
+	struct page_pipe_buf *prev[PP_PIPE_TYPES];	/* last ppb of each type
+							   for pipe sharing */
 	unsigned int nr_iovs;	/* number of iovs */
 	unsigned int free_iov;	/* first free iov */
 	struct iovec *iovs;	/* iovs. They are provided into create_page_pipe
diff --git a/criu/page-pipe.c b/criu/page-pipe.c
index f33e86a..f790a6f 100644
--- a/criu/page-pipe.c
+++ b/criu/page-pipe.c
@@ -10,6 +10,7 @@ 
 #include "page-pipe.h"
 #include "fcntl.h"
 #include "stats.h"
+#include "cr_options.h"
 
 /* can existing iov accumulate the page? */
 static inline bool iov_grow_page(struct iovec *iov, unsigned long addr)
@@ -63,8 +64,39 @@  static inline int ppb_resize_pipe(struct page_pipe_buf *ppb)
 	return 0;
 }
 
-static struct page_pipe_buf *ppb_alloc(struct page_pipe *pp, struct page_pipe_buf *prev)
+static struct page_pipe_buf *pp_prev_ppb(struct page_pipe *pp,
+					 unsigned int ppb_flags)
 {
+	int type = 0;
+
+	/* don't allow to reuse a pipe in the PP_CHUNK_MODE mode */
+	if (pp->flags & PP_CHUNK_MODE)
+		return NULL;
+
+	if (list_empty(&pp->bufs))
+		return NULL;
+
+	if (ppb_flags & PPB_LAZY && opts.lazy_pages)
+		type = 1;
+
+	return pp->prev[type];
+}
+
+static void pp_update_prev_ppb(struct page_pipe *pp, struct page_pipe_buf *ppb,
+			       unsigned int ppb_flags)
+{
+	int type = 0;
+
+	if (ppb_flags & PPB_LAZY && opts.lazy_pages)
+		type = 1;
+
+	pp->prev[type] = ppb;
+}
+
+static struct page_pipe_buf *ppb_alloc(struct page_pipe *pp,
+				       unsigned int ppb_flags)
+{
+	struct page_pipe_buf *prev = pp_prev_ppb(pp, ppb_flags);
 	struct page_pipe_buf *ppb;
 
 	ppb = xmalloc(sizeof(*ppb));
@@ -94,6 +126,8 @@  static struct page_pipe_buf *ppb_alloc(struct page_pipe *pp, struct page_pipe_bu
 
 	list_add_tail(&ppb->l, &pp->bufs);
 
+	pp_update_prev_ppb(pp, ppb, ppb_flags);
+
 	return ppb;
 }
 
@@ -119,7 +153,7 @@  static void ppb_init(struct page_pipe_buf *ppb, unsigned int pages_in,
 
 static int page_pipe_grow(struct page_pipe *pp, unsigned int flags)
 {
-	struct page_pipe_buf *ppb, *prev = NULL;
+	struct page_pipe_buf *ppb;
 	struct iovec *free_iov;
 
 	pr_debug("Will grow page pipe (iov off is %u)\n", pp->free_iov);
@@ -133,10 +167,7 @@  static int page_pipe_grow(struct page_pipe *pp, unsigned int flags)
 	if ((pp->flags & PP_CHUNK_MODE) && (pp->nr_pipes == NR_PIPES_PER_CHUNK))
 		return -EAGAIN;
 
-	/* don't allow to reuse a pipe in the PP_CHUNK_MODE mode */
-	if (!(pp->flags & PP_CHUNK_MODE) && !list_empty(&pp->bufs))
-		prev = list_entry(pp->bufs.prev, struct page_pipe_buf, l);
-	ppb = ppb_alloc(pp, prev);
+	ppb = ppb_alloc(pp, flags);
 	if (!ppb)
 		return -1;
 

Comments

Andrey Vagin Feb. 16, 2018, 12:25 a.m.
Applied, thanks

On Sun, Feb 11, 2018 at 03:53:15PM +0200, Mike Rapoport wrote:
> Currently, if pipe is shared between lazy and non-lazy PPBs lazy migration
> fails because data that should be transfered on demand is spliced into the
> images. Preventing pipe sharing between PPBs of different type resolves
> this issue.
> In order to still minimize pipe fragmentation, we track the last pipe that
> was used for certain PPB type and re-use it for the PPB of the same type.
> 
> Signed-off-by: Mike Rapoport <rppt@linux.vnet.ibm.com>
> ---
>  criu/include/page-pipe.h | 11 +++++++++++
>  criu/page-pipe.c         | 43 +++++++++++++++++++++++++++++++++++++------
>  2 files changed, 48 insertions(+), 6 deletions(-)
> 
> diff --git a/criu/include/page-pipe.h b/criu/include/page-pipe.h
> index 053ae39..8fa1bfa 100644
> --- a/criu/include/page-pipe.h
> +++ b/criu/include/page-pipe.h
> @@ -102,12 +102,23 @@ struct page_pipe_buf {
>  	struct list_head l;	/* links into page_pipe->bufs */
>  };
>  
> +/*
> + * Page pipe buffers with different flags cannot share the same pipe.
> + * We track the last ppb that was used for each type separately in the
> + * prev[] array in the struct page_pipe (below).
> + * Currently we have 2 types: the buffers that are always stored in
> + * the images and the buffers that are lazily migrated
> + */
> +#define PP_PIPE_TYPES	2
> +
>  #define PP_HOLE_PARENT (1 << 0)
>  
>  struct page_pipe {
>  	unsigned int nr_pipes;	/* how many page_pipe_bufs in there */
>  	struct list_head bufs;	/* list of bufs */
>  	struct list_head free_bufs;	/* list of bufs */
> +	struct page_pipe_buf *prev[PP_PIPE_TYPES];	/* last ppb of each type
> +							   for pipe sharing */
>  	unsigned int nr_iovs;	/* number of iovs */
>  	unsigned int free_iov;	/* first free iov */
>  	struct iovec *iovs;	/* iovs. They are provided into create_page_pipe
> diff --git a/criu/page-pipe.c b/criu/page-pipe.c
> index f33e86a..f790a6f 100644
> --- a/criu/page-pipe.c
> +++ b/criu/page-pipe.c
> @@ -10,6 +10,7 @@
>  #include "page-pipe.h"
>  #include "fcntl.h"
>  #include "stats.h"
> +#include "cr_options.h"
>  
>  /* can existing iov accumulate the page? */
>  static inline bool iov_grow_page(struct iovec *iov, unsigned long addr)
> @@ -63,8 +64,39 @@ static inline int ppb_resize_pipe(struct page_pipe_buf *ppb)
>  	return 0;
>  }
>  
> -static struct page_pipe_buf *ppb_alloc(struct page_pipe *pp, struct page_pipe_buf *prev)
> +static struct page_pipe_buf *pp_prev_ppb(struct page_pipe *pp,
> +					 unsigned int ppb_flags)
>  {
> +	int type = 0;
> +
> +	/* don't allow to reuse a pipe in the PP_CHUNK_MODE mode */
> +	if (pp->flags & PP_CHUNK_MODE)
> +		return NULL;
> +
> +	if (list_empty(&pp->bufs))
> +		return NULL;
> +
> +	if (ppb_flags & PPB_LAZY && opts.lazy_pages)
> +		type = 1;
> +
> +	return pp->prev[type];
> +}
> +
> +static void pp_update_prev_ppb(struct page_pipe *pp, struct page_pipe_buf *ppb,
> +			       unsigned int ppb_flags)
> +{
> +	int type = 0;
> +
> +	if (ppb_flags & PPB_LAZY && opts.lazy_pages)
> +		type = 1;
> +
> +	pp->prev[type] = ppb;
> +}
> +
> +static struct page_pipe_buf *ppb_alloc(struct page_pipe *pp,
> +				       unsigned int ppb_flags)
> +{
> +	struct page_pipe_buf *prev = pp_prev_ppb(pp, ppb_flags);
>  	struct page_pipe_buf *ppb;
>  
>  	ppb = xmalloc(sizeof(*ppb));
> @@ -94,6 +126,8 @@ static struct page_pipe_buf *ppb_alloc(struct page_pipe *pp, struct page_pipe_bu
>  
>  	list_add_tail(&ppb->l, &pp->bufs);
>  
> +	pp_update_prev_ppb(pp, ppb, ppb_flags);
> +
>  	return ppb;
>  }
>  
> @@ -119,7 +153,7 @@ static void ppb_init(struct page_pipe_buf *ppb, unsigned int pages_in,
>  
>  static int page_pipe_grow(struct page_pipe *pp, unsigned int flags)
>  {
> -	struct page_pipe_buf *ppb, *prev = NULL;
> +	struct page_pipe_buf *ppb;
>  	struct iovec *free_iov;
>  
>  	pr_debug("Will grow page pipe (iov off is %u)\n", pp->free_iov);
> @@ -133,10 +167,7 @@ static int page_pipe_grow(struct page_pipe *pp, unsigned int flags)
>  	if ((pp->flags & PP_CHUNK_MODE) && (pp->nr_pipes == NR_PIPES_PER_CHUNK))
>  		return -EAGAIN;
>  
> -	/* don't allow to reuse a pipe in the PP_CHUNK_MODE mode */
> -	if (!(pp->flags & PP_CHUNK_MODE) && !list_empty(&pp->bufs))
> -		prev = list_entry(pp->bufs.prev, struct page_pipe_buf, l);
> -	ppb = ppb_alloc(pp, prev);
> +	ppb = ppb_alloc(pp, flags);
>  	if (!ppb)
>  		return -1;
>  
> -- 
> 2.7.4
>