[3/7] Merging splice and read modes of pre-dump

Submitted by abhishek dubey on Aug. 20, 2019, 11:06 p.m.

Details

Message ID 1566342422-18459-4-git-send-email-dubeyabhishek777@gmail.com
State New
Series "GSoC 19: Optimizing the Pre-dump Algorithm"
Headers show

Commit Message

abhishek dubey Aug. 20, 2019, 11:06 p.m.
Invoke appropriate pre-dump algorithm,
as per --pre-dump-mode option supplied.

Default pre-dump mode is: splice

Signed-off-by: Abhishek Dubey <dubeyabhishek777@gmail.com>
---
 criu/cr-dump.c            | 13 +++++++++++--
 criu/include/cr_options.h |  5 +++++
 criu/include/mem.h        |  2 +-
 3 files changed, 17 insertions(+), 3 deletions(-)

Patch hide | download patch | download mbox

diff --git a/criu/cr-dump.c b/criu/cr-dump.c
index e070b8b..9baf7cf 100644
--- a/criu/cr-dump.c
+++ b/criu/cr-dump.c
@@ -1189,7 +1189,9 @@  static int pre_dump_one_task(struct pstree_item *item, InventoryEntry *parent_ie
 
 	item->pid->ns[0].virt = misc.pid;
 
-	mdc.pre_dump = true;
+	mdc.pre_dump = (opts.pre_dump_mode == PRE_DUMP_READ) ?
+						  PRE_DUMP_READ
+						: PRE_DUMP_SPLICE;
 	mdc.lazy = false;
 	mdc.stat = NULL;
 	mdc.parent_ie = parent_ie;
@@ -1513,7 +1515,14 @@  static int cr_pre_dump_finish(int status)
 			goto err;
 
 		mem_pp = dmpi(item)->mem_pp;
-		ret = page_xfer_dump_pages(&xfer, mem_pp);
+
+		if (opts.pre_dump_mode == PRE_DUMP_READ) {
+			timing_stop(TIME_MEMWRITE);
+			ret = page_xfer_predump_pages(item->pid->real,
+							&xfer, mem_pp);
+		}
+		else
+			ret = page_xfer_dump_pages(&xfer, mem_pp);
 
 		xfer.close(&xfer);
 
diff --git a/criu/include/cr_options.h b/criu/include/cr_options.h
index c519c74..6b84e31 100644
--- a/criu/include/cr_options.h
+++ b/criu/include/cr_options.h
@@ -39,6 +39,11 @@  struct cg_root_opt {
 };
 
 /*
+ * Pre-dump variants
+ */
+#define PRE_DUMP_SPLICE	1		/* Pre-dump using parasite */
+#define PRE_DUMP_READ		2		/* Pre-dump using process_vm_readv syscall */
+/*
  * Cgroup management options.
  */
 #define CG_MODE_IGNORE		(0u << 0)	/* Zero is important here */
diff --git a/criu/include/mem.h b/criu/include/mem.h
index 251cb1a..74c2803 100644
--- a/criu/include/mem.h
+++ b/criu/include/mem.h
@@ -15,7 +15,7 @@  struct pstree_item;
 struct vma_area;
 
 struct mem_dump_ctl {
-	bool			pre_dump;
+	int			pre_dump;
 	bool			lazy;
 	struct proc_pid_stat	*stat;
 	InventoryEntry		*parent_ie;

Comments

Andrei Vagin Aug. 22, 2019, 3:12 p.m.
On Wed, Aug 21, 2019 at 04:36:58AM +0530, Abhishek Dubey wrote:
> Invoke appropriate pre-dump algorithm,
> as per --pre-dump-mode option supplied.
> 
> Default pre-dump mode is: splice
> 
> Signed-off-by: Abhishek Dubey <dubeyabhishek777@gmail.com>
> ---
>  criu/cr-dump.c            | 13 +++++++++++--
>  criu/include/cr_options.h |  5 +++++
>  criu/include/mem.h        |  2 +-
>  3 files changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/criu/cr-dump.c b/criu/cr-dump.c
> index e070b8b..9baf7cf 100644
> --- a/criu/cr-dump.c
> +++ b/criu/cr-dump.c
> @@ -1189,7 +1189,9 @@ static int pre_dump_one_task(struct pstree_item *item, InventoryEntry *parent_ie
>  
>  	item->pid->ns[0].virt = misc.pid;
>  
> -	mdc.pre_dump = true;
> +	mdc.pre_dump = (opts.pre_dump_mode == PRE_DUMP_READ) ?
> +						  PRE_DUMP_READ
> +						: PRE_DUMP_SPLICE;

Could you explain what is going on here? Should we return an error if
opts.pre_dump_mode isn't equal to PRE_DUMP_READ or PRE_DUMP_SPLICE?

>  	mdc.lazy = false;
>  	mdc.stat = NULL;
>  	mdc.parent_ie = parent_ie;
> @@ -1513,7 +1515,14 @@ static int cr_pre_dump_finish(int status)
>  			goto err;
>  
>  		mem_pp = dmpi(item)->mem_pp;
> -		ret = page_xfer_dump_pages(&xfer, mem_pp);
> +
> +		if (opts.pre_dump_mode == PRE_DUMP_READ) {
> +			timing_stop(TIME_MEMWRITE);
> +			ret = page_xfer_predump_pages(item->pid->real,
> +							&xfer, mem_pp);
> +		}
> +		else
> +			ret = page_xfer_dump_pages(&xfer, mem_pp);
>  
>  		xfer.close(&xfer);
>  
> diff --git a/criu/include/cr_options.h b/criu/include/cr_options.h
> index c519c74..6b84e31 100644
> --- a/criu/include/cr_options.h
> +++ b/criu/include/cr_options.h
> @@ -39,6 +39,11 @@ struct cg_root_opt {
>  };
>  
>  /*
> + * Pre-dump variants
> + */
> +#define PRE_DUMP_SPLICE	1		/* Pre-dump using parasite */
> +#define PRE_DUMP_READ		2		/* Pre-dump using process_vm_readv syscall */
> +/*
>   * Cgroup management options.
>   */
>  #define CG_MODE_IGNORE		(0u << 0)	/* Zero is important here */
> diff --git a/criu/include/mem.h b/criu/include/mem.h
> index 251cb1a..74c2803 100644
> --- a/criu/include/mem.h
> +++ b/criu/include/mem.h
> @@ -15,7 +15,7 @@ struct pstree_item;
>  struct vma_area;
>  
>  struct mem_dump_ctl {
> -	bool			pre_dump;
> +	int			pre_dump;
>  	bool			lazy;
>  	struct proc_pid_stat	*stat;
>  	InventoryEntry		*parent_ie;
> -- 
> 2.7.4
>
abhishek dubey Aug. 22, 2019, 9:25 p.m.
On 22/08/19 8:42 PM, Andrei Vagin wrote:
> On Wed, Aug 21, 2019 at 04:36:58AM +0530, Abhishek Dubey wrote:
>> Invoke appropriate pre-dump algorithm,
>> as per --pre-dump-mode option supplied.
>>
>> Default pre-dump mode is: splice
>>
>> Signed-off-by: Abhishek Dubey <dubeyabhishek777@gmail.com>
>> ---
>>   criu/cr-dump.c            | 13 +++++++++++--
>>   criu/include/cr_options.h |  5 +++++
>>   criu/include/mem.h        |  2 +-
>>   3 files changed, 17 insertions(+), 3 deletions(-)
>>
>> diff --git a/criu/cr-dump.c b/criu/cr-dump.c
>> index e070b8b..9baf7cf 100644
>> --- a/criu/cr-dump.c
>> +++ b/criu/cr-dump.c
>> @@ -1189,7 +1189,9 @@ static int pre_dump_one_task(struct pstree_item *item, InventoryEntry *parent_ie
>>   
>>   	item->pid->ns[0].virt = misc.pid;
>>   
>> -	mdc.pre_dump = true;
>> +	mdc.pre_dump = (opts.pre_dump_mode == PRE_DUMP_READ) ?
>> +						  PRE_DUMP_READ
>> +						: PRE_DUMP_SPLICE;

This patch seem unnecessary to me now. I will rollback these changes. 
Rather than modifying mdc.pre_dump to store mode of pre-dump,

I would use opts.pre_dump_mode for branching.

Yes, I must throw error, if some other value except splice/read is 
passed from command line.

I will handle it now.

> Could you explain what is going on here? Should we return an error if
> opts.pre_dump_mode isn't equal to PRE_DUMP_READ or PRE_DUMP_SPLICE?
>
>>   	mdc.lazy = false;
>>   	mdc.stat = NULL;
>>   	mdc.parent_ie = parent_ie;
>> @@ -1513,7 +1515,14 @@ static int cr_pre_dump_finish(int status)
>>   			goto err;
>>   
>>   		mem_pp = dmpi(item)->mem_pp;
>> -		ret = page_xfer_dump_pages(&xfer, mem_pp);
>> +
>> +		if (opts.pre_dump_mode == PRE_DUMP_READ) {
>> +			timing_stop(TIME_MEMWRITE);
>> +			ret = page_xfer_predump_pages(item->pid->real,
>> +							&xfer, mem_pp);
>> +		}
>> +		else
>> +			ret = page_xfer_dump_pages(&xfer, mem_pp);
>>   
>>   		xfer.close(&xfer);
>>   
>> diff --git a/criu/include/cr_options.h b/criu/include/cr_options.h
>> index c519c74..6b84e31 100644
>> --- a/criu/include/cr_options.h
>> +++ b/criu/include/cr_options.h
>> @@ -39,6 +39,11 @@ struct cg_root_opt {
>>   };
>>   
>>   /*
>> + * Pre-dump variants
>> + */
>> +#define PRE_DUMP_SPLICE	1		/* Pre-dump using parasite */
>> +#define PRE_DUMP_READ		2		/* Pre-dump using process_vm_readv syscall */
>> +/*
>>    * Cgroup management options.
>>    */
>>   #define CG_MODE_IGNORE		(0u << 0)	/* Zero is important here */
>> diff --git a/criu/include/mem.h b/criu/include/mem.h
>> index 251cb1a..74c2803 100644
>> --- a/criu/include/mem.h
>> +++ b/criu/include/mem.h
>> @@ -15,7 +15,7 @@ struct pstree_item;
>>   struct vma_area;
>>   
>>   struct mem_dump_ctl {
>> -	bool			pre_dump;
>> +	int			pre_dump;
>>   	bool			lazy;
>>   	struct proc_pid_stat	*stat;
>>   	InventoryEntry		*parent_ie;
>> -- 
>> 2.7.4
>>
-Abhishek