[1/7] Adding --pre-dump-mode option

Submitted by Abhishek Dubey on Aug. 25, 2019, 1:58 a.m.

Details

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

Commit Message

Abhishek Dubey Aug. 25, 2019, 1:58 a.m.
There exists two modes of pre-dump algorithm:
    1) splicing memory by parasite
        --pre-dump-mode=splice (default)
    2) using process_vm_readv syscall
        --pre-dump-mode=read

Signed-off-by: Abhishek Dubey <dubeyabhishek777@gmail.com>
---
 Documentation/criu.txt    |  6 ++++++
 criu/config.c             | 10 ++++++++++
 criu/cr-dump.c            |  6 +++++-
 criu/crtools.c            |  2 ++
 criu/include/cr_options.h |  7 +++++++
 criu/mem.c                | 13 ++++++++++++-
 test/zdtm.py              |  9 ++++++++-
 7 files changed, 50 insertions(+), 3 deletions(-)

Patch hide | download patch | download mbox

diff --git a/Documentation/criu.txt b/Documentation/criu.txt
index 94fc542..674967f 100644
--- a/Documentation/criu.txt
+++ b/Documentation/criu.txt
@@ -156,6 +156,12 @@  In addition, *page-server* options may be specified.
     Turn on memory changes tracker in the kernel. If the option is
     not passed the memory tracker get turned on implicitly.
 
+*--pre-dump-mode*='mode'::
+    There are two 'mode' to operate pre-dump algorithm. The 'splice' mode
+    is parasite based, whereas 'read' mode is based on process_vm_readv
+    syscall. The 'read' mode incurs reduced frozen time and reduced
+    memory pressure as compared to 'splice' mode. Default is 'splice' mode.
+
 *dump*
 ~~~~~~
 Performs a checkpoint procedure.
diff --git a/criu/config.c b/criu/config.c
index 3a54afd..3c65dc0 100644
--- a/criu/config.c
+++ b/criu/config.c
@@ -276,6 +276,7 @@  void init_opts(void)
 	opts.empty_ns = 0;
 	opts.status_fd = -1;
 	opts.log_level = DEFAULT_LOGLEVEL;
+	opts.pre_dump_mode = PRE_DUMP_SPLICE;
 }
 
 bool deprecated_ok(char *what)
@@ -517,6 +518,7 @@  int parse_options(int argc, char **argv, bool *usage_error,
 		{ "tls-key",			required_argument,	0, 1095},
 		BOOL_OPT("tls", &opts.tls),
 		{"tls-no-cn-verify",		no_argument,		&opts.tls_no_cn_verify, true},
+		{ "pre-dump-mode",		required_argument,	0, 1097},
 		{ },
 	};
 
@@ -815,6 +817,14 @@  int parse_options(int argc, char **argv, bool *usage_error,
 		case 1095:
 			SET_CHAR_OPTS(tls_key, optarg);
 			break;
+		case 1097:
+			if (!strcmp("read", optarg)) {
+				opts.pre_dump_mode = PRE_DUMP_READ;
+			} else if (strcmp("splice", optarg)) {
+				pr_err("Unable to parse value of --pre-dump-mode\n");
+				return 1;
+			}
+			break;
 		case 'V':
 			pr_msg("Version: %s\n", CRIU_VERSION);
 			if (strcmp(CRIU_GITID, "0"))
diff --git a/criu/cr-dump.c b/criu/cr-dump.c
index e070b8b..a7ce1ce 100644
--- a/criu/cr-dump.c
+++ b/criu/cr-dump.c
@@ -1513,7 +1513,11 @@  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)
+			ret = 0;  /* Call optimized pre-dump here */
+		else
+			ret = page_xfer_dump_pages(&xfer, mem_pp);
 
 		xfer.close(&xfer);
 
diff --git a/criu/crtools.c b/criu/crtools.c
index 97a6d6d..fc17d83 100644
--- a/criu/crtools.c
+++ b/criu/crtools.c
@@ -442,6 +442,8 @@  usage:
 "                        pages images of previous dump\n"
 "                        when used on restore, as soon as page is restored, it\n"
 "                        will be punched from the image\n"
+"  --pre-dump-mode       splice - parasite based pre-dumping (default)\n"
+"                        read   - process_vm_readv syscall based pre-dumping\n"
 "\n"
 "Page/Service server options:\n"
 "  --address ADDR        address of server or service\n"
diff --git a/criu/include/cr_options.h b/criu/include/cr_options.h
index c519c74..e50989f 100644
--- a/criu/include/cr_options.h
+++ b/criu/include/cr_options.h
@@ -39,6 +39,12 @@  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 */
@@ -81,6 +87,7 @@  struct cr_options {
 	int			evasive_devices;
 	int			link_remap_ok;
 	int			log_file_per_pid;
+	int			pre_dump_mode;
 	bool			swrk_restore;
 	char			*output;
 	char			*root;
diff --git a/criu/mem.c b/criu/mem.c
index de66a62..911b9d2 100644
--- a/criu/mem.c
+++ b/criu/mem.c
@@ -482,7 +482,18 @@  static int __parasite_dump_pages_seized(struct pstree_item *item,
 	if (mdc->lazy)
 		memcpy(pargs_iovs(args), pp->iovs,
 		       sizeof(struct iovec) * pp->nr_iovs);
-	ret = drain_pages(pp, ctl, args);
+
+	/*
+	 * Faking drain_pages for pre-dump here. Actual drain_pages for pre-dump
+	 * will happen after task unfreezing in cr_pre_dump_finish(). This is
+	 * actual optimization which reduces time for which process was frozen
+	 * during pre-dump.
+	 */
+	if (mdc->pre_dump && opts.pre_dump_mode == PRE_DUMP_READ)
+		ret = 0;
+	else
+		ret = drain_pages(pp, ctl, args);
+
 	if (!ret && !mdc->pre_dump)
 		ret = xfer_pages(pp, &xfer);
 	if (ret)
diff --git a/test/zdtm.py b/test/zdtm.py
index 9b93a51..ca8b165 100755
--- a/test/zdtm.py
+++ b/test/zdtm.py
@@ -1020,6 +1020,7 @@  class criu:
         self.__tls = self.__tls_options() if opts['tls'] else []
         self.__criu_bin = opts['criu_bin']
         self.__crit_bin = opts['crit_bin']
+        self.__pre_dump_mode = opts['pre_dump_mode']
 
     def fini(self):
         if self.__lazy_migrate:
@@ -1276,6 +1277,8 @@  class criu:
             a_opts += ['--leave-stopped']
         if self.__empty_ns:
             a_opts += ['--empty-ns', 'net']
+        if self.__pre_dump_mode:
+            a_opts += ["--pre-dump-mode", "%s" % self.__pre_dump_mode]
 
         nowait = False
         if self.__lazy_migrate and action == "dump":
@@ -1865,7 +1868,7 @@  class Launcher:
               'sat', 'script', 'rpc', 'lazy_pages', 'join_ns', 'dedup', 'sbs',
               'freezecg', 'user', 'dry_run', 'noauto_dedup',
               'remote_lazy_pages', 'show_stats', 'lazy_migrate', 'remote',
-              'tls', 'criu_bin', 'crit_bin')
+              'tls', 'criu_bin', 'crit_bin', 'pre_dump_mode')
         arg = repr((name, desc, flavor, {d: self.__opts[d] for d in nd}))
 
         if self.__use_log:
@@ -2512,6 +2515,10 @@  rp.add_argument("--criu-bin",
 rp.add_argument("--crit-bin",
                 help="Path to crit binary",
                 default='../crit/crit')
+rp.add_argument("--pre-dump-mode",
+                help="Use splice or read mode of pre-dumping",
+                choices=['splice', 'read'],
+                default='splice')
 
 lp = sp.add_parser("list", help="List tests")
 lp.set_defaults(action=list_tests)

Comments

Radostin Stoyanov Aug. 25, 2019, 9:31 a.m.
On 25/08/2019 02:58, Abhishek Dubey wrote:
> There exists two modes of pre-dump algorithm:
>      1) splicing memory by parasite
>          --pre-dump-mode=splice (default)
>      2) using process_vm_readv syscall
>          --pre-dump-mode=read
>
> Signed-off-by: Abhishek Dubey <dubeyabhishek777@gmail.com>
> ---
>   Documentation/criu.txt    |  6 ++++++
>   criu/config.c             | 10 ++++++++++
>   criu/cr-dump.c            |  6 +++++-
>   criu/crtools.c            |  2 ++
>   criu/include/cr_options.h |  7 +++++++
>   criu/mem.c                | 13 ++++++++++++-
>   test/zdtm.py              |  9 ++++++++-
>   7 files changed, 50 insertions(+), 3 deletions(-)
Please add pre_dump_mode in images/rpc.proto and lib/c/criu.c

This will enable the option to be used with RPC and libcriu.
> diff --git a/Documentation/criu.txt b/Documentation/criu.txt
> index 94fc542..674967f 100644
> --- a/Documentation/criu.txt
> +++ b/Documentation/criu.txt
> @@ -156,6 +156,12 @@ In addition, *page-server* options may be specified.
>       Turn on memory changes tracker in the kernel. If the option is
>       not passed the memory tracker get turned on implicitly.
>   
> +*--pre-dump-mode*='mode'::
> +    There are two 'mode' to operate pre-dump algorithm. The 'splice' mode
> +    is parasite based, whereas 'read' mode is based on process_vm_readv
> +    syscall. The 'read' mode incurs reduced frozen time and reduced
> +    memory pressure as compared to 'splice' mode. Default is 'splice' mode.
> +
>   *dump*
>   ~~~~~~
>   Performs a checkpoint procedure.
> diff --git a/criu/config.c b/criu/config.c
> index 3a54afd..3c65dc0 100644
> --- a/criu/config.c
> +++ b/criu/config.c
> @@ -276,6 +276,7 @@ void init_opts(void)
>   	opts.empty_ns = 0;
>   	opts.status_fd = -1;
>   	opts.log_level = DEFAULT_LOGLEVEL;
> +	opts.pre_dump_mode = PRE_DUMP_SPLICE;
>   }
>   
>   bool deprecated_ok(char *what)
> @@ -517,6 +518,7 @@ int parse_options(int argc, char **argv, bool *usage_error,
>   		{ "tls-key",			required_argument,	0, 1095},
>   		BOOL_OPT("tls", &opts.tls),
>   		{"tls-no-cn-verify",		no_argument,		&opts.tls_no_cn_verify, true},
> +		{ "pre-dump-mode",		required_argument,	0, 1097},
>   		{ },
>   	};
>   
> @@ -815,6 +817,14 @@ int parse_options(int argc, char **argv, bool *usage_error,
>   		case 1095:
>   			SET_CHAR_OPTS(tls_key, optarg);
>   			break;
> +		case 1097:
> +			if (!strcmp("read", optarg)) {
> +				opts.pre_dump_mode = PRE_DUMP_READ;
> +			} else if (strcmp("splice", optarg)) {
> +				pr_err("Unable to parse value of --pre-dump-mode\n");
> +				return 1;
> +			}
> +			break;
>   		case 'V':
>   			pr_msg("Version: %s\n", CRIU_VERSION);
>   			if (strcmp(CRIU_GITID, "0"))
> diff --git a/criu/cr-dump.c b/criu/cr-dump.c
> index e070b8b..a7ce1ce 100644
> --- a/criu/cr-dump.c
> +++ b/criu/cr-dump.c
> @@ -1513,7 +1513,11 @@ 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)
> +			ret = 0;  /* Call optimized pre-dump here */
> +		else
> +			ret = page_xfer_dump_pages(&xfer, mem_pp);
>   
>   		xfer.close(&xfer);
>   
> diff --git a/criu/crtools.c b/criu/crtools.c
> index 97a6d6d..fc17d83 100644
> --- a/criu/crtools.c
> +++ b/criu/crtools.c
> @@ -442,6 +442,8 @@ usage:
>   "                        pages images of previous dump\n"
>   "                        when used on restore, as soon as page is restored, it\n"
>   "                        will be punched from the image\n"
> +"  --pre-dump-mode       splice - parasite based pre-dumping (default)\n"
> +"                        read   - process_vm_readv syscall based pre-dumping\n"
>   "\n"
>   "Page/Service server options:\n"
>   "  --address ADDR        address of server or service\n"
> diff --git a/criu/include/cr_options.h b/criu/include/cr_options.h
> index c519c74..e50989f 100644
> --- a/criu/include/cr_options.h
> +++ b/criu/include/cr_options.h
> @@ -39,6 +39,12 @@ 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 */
> @@ -81,6 +87,7 @@ struct cr_options {
>   	int			evasive_devices;
>   	int			link_remap_ok;
>   	int			log_file_per_pid;
> +	int			pre_dump_mode;
>   	bool			swrk_restore;
>   	char			*output;
>   	char			*root;
> diff --git a/criu/mem.c b/criu/mem.c
> index de66a62..911b9d2 100644
> --- a/criu/mem.c
> +++ b/criu/mem.c
> @@ -482,7 +482,18 @@ static int __parasite_dump_pages_seized(struct pstree_item *item,
>   	if (mdc->lazy)
>   		memcpy(pargs_iovs(args), pp->iovs,
>   		       sizeof(struct iovec) * pp->nr_iovs);
> -	ret = drain_pages(pp, ctl, args);
> +
> +	/*
> +	 * Faking drain_pages for pre-dump here. Actual drain_pages for pre-dump
> +	 * will happen after task unfreezing in cr_pre_dump_finish(). This is
> +	 * actual optimization which reduces time for which process was frozen
> +	 * during pre-dump.
> +	 */
> +	if (mdc->pre_dump && opts.pre_dump_mode == PRE_DUMP_READ)
> +		ret = 0;
> +	else
> +		ret = drain_pages(pp, ctl, args);
> +
>   	if (!ret && !mdc->pre_dump)
>   		ret = xfer_pages(pp, &xfer);
>   	if (ret)
> diff --git a/test/zdtm.py b/test/zdtm.py
> index 9b93a51..ca8b165 100755
> --- a/test/zdtm.py
> +++ b/test/zdtm.py
> @@ -1020,6 +1020,7 @@ class criu:
>           self.__tls = self.__tls_options() if opts['tls'] else []
>           self.__criu_bin = opts['criu_bin']
>           self.__crit_bin = opts['crit_bin']
> +        self.__pre_dump_mode = opts['pre_dump_mode']
>   
>       def fini(self):
>           if self.__lazy_migrate:
> @@ -1276,6 +1277,8 @@ class criu:
>               a_opts += ['--leave-stopped']
>           if self.__empty_ns:
>               a_opts += ['--empty-ns', 'net']
> +        if self.__pre_dump_mode:
> +            a_opts += ["--pre-dump-mode", "%s" % self.__pre_dump_mode]
>   
>           nowait = False
>           if self.__lazy_migrate and action == "dump":
> @@ -1865,7 +1868,7 @@ class Launcher:
>                 'sat', 'script', 'rpc', 'lazy_pages', 'join_ns', 'dedup', 'sbs',
>                 'freezecg', 'user', 'dry_run', 'noauto_dedup',
>                 'remote_lazy_pages', 'show_stats', 'lazy_migrate', 'remote',
> -              'tls', 'criu_bin', 'crit_bin')
> +              'tls', 'criu_bin', 'crit_bin', 'pre_dump_mode')
>           arg = repr((name, desc, flavor, {d: self.__opts[d] for d in nd}))
>   
>           if self.__use_log:
> @@ -2512,6 +2515,10 @@ rp.add_argument("--criu-bin",
>   rp.add_argument("--crit-bin",
>                   help="Path to crit binary",
>                   default='../crit/crit')
> +rp.add_argument("--pre-dump-mode",
> +                help="Use splice or read mode of pre-dumping",
> +                choices=['splice', 'read'],
> +                default='splice')
>   
>   lp = sp.add_parser("list", help="List tests")
>   lp.set_defaults(action=list_tests)
Abhishek Dubey Aug. 26, 2019, 3:39 a.m.
Hi Radostin,

On 25/08/19 3:01 PM, Radostin Stoyanov wrote:
> On 25/08/2019 02:58, Abhishek Dubey wrote:
>> There exists two modes of pre-dump algorithm:
>>      1) splicing memory by parasite
>>          --pre-dump-mode=splice (default)
>>      2) using process_vm_readv syscall
>>          --pre-dump-mode=read
>>
>> Signed-off-by: Abhishek Dubey <dubeyabhishek777@gmail.com>
>> ---
>>   Documentation/criu.txt    |  6 ++++++
>>   criu/config.c             | 10 ++++++++++
>>   criu/cr-dump.c            |  6 +++++-
>>   criu/crtools.c            |  2 ++
>>   criu/include/cr_options.h |  7 +++++++
>>   criu/mem.c                | 13 ++++++++++++-
>>   test/zdtm.py              |  9 ++++++++-
>>   7 files changed, 50 insertions(+), 3 deletions(-)
> Please add pre_dump_mode in images/rpc.proto and lib/c/criu.c

Please verify changes:

https://github.com/dubeyabhishek/criu/commit/189e2b5f3c2486e8b3bb525ff3d0d3298a99b752

>
>
> This will enable the option to be used with RPC and libcriu.
>> diff --git a/Documentation/criu.txt b/Documentation/criu.txt
>> index 94fc542..674967f 100644
>> --- a/Documentation/criu.txt
>> +++ b/Documentation/criu.txt
>> @@ -156,6 +156,12 @@ In addition, *page-server* options may be 
>> specified.
>>       Turn on memory changes tracker in the kernel. If the option is
>>       not passed the memory tracker get turned on implicitly.
>>   +*--pre-dump-mode*='mode'::
>> +    There are two 'mode' to operate pre-dump algorithm. The 'splice' 
>> mode
>> +    is parasite based, whereas 'read' mode is based on process_vm_readv
>> +    syscall. The 'read' mode incurs reduced frozen time and reduced
>> +    memory pressure as compared to 'splice' mode. Default is 
>> 'splice' mode.
>> +
>>   *dump*
>>   ~~~~~~
>>   Performs a checkpoint procedure.
>> diff --git a/criu/config.c b/criu/config.c
>> index 3a54afd..3c65dc0 100644
>> --- a/criu/config.c
>> +++ b/criu/config.c
>> @@ -276,6 +276,7 @@ void init_opts(void)
>>       opts.empty_ns = 0;
>>       opts.status_fd = -1;
>>       opts.log_level = DEFAULT_LOGLEVEL;
>> +    opts.pre_dump_mode = PRE_DUMP_SPLICE;
>>   }
>>     bool deprecated_ok(char *what)
>> @@ -517,6 +518,7 @@ int parse_options(int argc, char **argv, bool 
>> *usage_error,
>>           { "tls-key",            required_argument,    0, 1095},
>>           BOOL_OPT("tls", &opts.tls),
>>           {"tls-no-cn-verify",        no_argument, 
>> &opts.tls_no_cn_verify, true},
>> +        { "pre-dump-mode",        required_argument,    0, 1097},
>>           { },
>>       };
>>   @@ -815,6 +817,14 @@ int parse_options(int argc, char **argv, bool 
>> *usage_error,
>>           case 1095:
>>               SET_CHAR_OPTS(tls_key, optarg);
>>               break;
>> +        case 1097:
>> +            if (!strcmp("read", optarg)) {
>> +                opts.pre_dump_mode = PRE_DUMP_READ;
>> +            } else if (strcmp("splice", optarg)) {
>> +                pr_err("Unable to parse value of --pre-dump-mode\n");
>> +                return 1;
>> +            }
>> +            break;
>>           case 'V':
>>               pr_msg("Version: %s\n", CRIU_VERSION);
>>               if (strcmp(CRIU_GITID, "0"))
>> diff --git a/criu/cr-dump.c b/criu/cr-dump.c
>> index e070b8b..a7ce1ce 100644
>> --- a/criu/cr-dump.c
>> +++ b/criu/cr-dump.c
>> @@ -1513,7 +1513,11 @@ 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)
>> +            ret = 0;  /* Call optimized pre-dump here */
>> +        else
>> +            ret = page_xfer_dump_pages(&xfer, mem_pp);
>>             xfer.close(&xfer);
>>   diff --git a/criu/crtools.c b/criu/crtools.c
>> index 97a6d6d..fc17d83 100644
>> --- a/criu/crtools.c
>> +++ b/criu/crtools.c
>> @@ -442,6 +442,8 @@ usage:
>>   "                        pages images of previous dump\n"
>>   "                        when used on restore, as soon as page is 
>> restored, it\n"
>>   "                        will be punched from the image\n"
>> +"  --pre-dump-mode       splice - parasite based pre-dumping 
>> (default)\n"
>> +"                        read   - process_vm_readv syscall based 
>> pre-dumping\n"
>>   "\n"
>>   "Page/Service server options:\n"
>>   "  --address ADDR        address of server or service\n"
>> diff --git a/criu/include/cr_options.h b/criu/include/cr_options.h
>> index c519c74..e50989f 100644
>> --- a/criu/include/cr_options.h
>> +++ b/criu/include/cr_options.h
>> @@ -39,6 +39,12 @@ 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 */
>> @@ -81,6 +87,7 @@ struct cr_options {
>>       int            evasive_devices;
>>       int            link_remap_ok;
>>       int            log_file_per_pid;
>> +    int            pre_dump_mode;
>>       bool            swrk_restore;
>>       char            *output;
>>       char            *root;
>> diff --git a/criu/mem.c b/criu/mem.c
>> index de66a62..911b9d2 100644
>> --- a/criu/mem.c
>> +++ b/criu/mem.c
>> @@ -482,7 +482,18 @@ static int __parasite_dump_pages_seized(struct 
>> pstree_item *item,
>>       if (mdc->lazy)
>>           memcpy(pargs_iovs(args), pp->iovs,
>>                  sizeof(struct iovec) * pp->nr_iovs);
>> -    ret = drain_pages(pp, ctl, args);
>> +
>> +    /*
>> +     * Faking drain_pages for pre-dump here. Actual drain_pages for 
>> pre-dump
>> +     * will happen after task unfreezing in cr_pre_dump_finish(). 
>> This is
>> +     * actual optimization which reduces time for which process was 
>> frozen
>> +     * during pre-dump.
>> +     */
>> +    if (mdc->pre_dump && opts.pre_dump_mode == PRE_DUMP_READ)
>> +        ret = 0;
>> +    else
>> +        ret = drain_pages(pp, ctl, args);
>> +
>>       if (!ret && !mdc->pre_dump)
>>           ret = xfer_pages(pp, &xfer);
>>       if (ret)
>> diff --git a/test/zdtm.py b/test/zdtm.py
>> index 9b93a51..ca8b165 100755
>> --- a/test/zdtm.py
>> +++ b/test/zdtm.py
>> @@ -1020,6 +1020,7 @@ class criu:
>>           self.__tls = self.__tls_options() if opts['tls'] else []
>>           self.__criu_bin = opts['criu_bin']
>>           self.__crit_bin = opts['crit_bin']
>> +        self.__pre_dump_mode = opts['pre_dump_mode']
>>         def fini(self):
>>           if self.__lazy_migrate:
>> @@ -1276,6 +1277,8 @@ class criu:
>>               a_opts += ['--leave-stopped']
>>           if self.__empty_ns:
>>               a_opts += ['--empty-ns', 'net']
>> +        if self.__pre_dump_mode:
>> +            a_opts += ["--pre-dump-mode", "%s" % self.__pre_dump_mode]
>>             nowait = False
>>           if self.__lazy_migrate and action == "dump":
>> @@ -1865,7 +1868,7 @@ class Launcher:
>>                 'sat', 'script', 'rpc', 'lazy_pages', 'join_ns', 
>> 'dedup', 'sbs',
>>                 'freezecg', 'user', 'dry_run', 'noauto_dedup',
>>                 'remote_lazy_pages', 'show_stats', 'lazy_migrate', 
>> 'remote',
>> -              'tls', 'criu_bin', 'crit_bin')
>> +              'tls', 'criu_bin', 'crit_bin', 'pre_dump_mode')
>>           arg = repr((name, desc, flavor, {d: self.__opts[d] for d in 
>> nd}))
>>             if self.__use_log:
>> @@ -2512,6 +2515,10 @@ rp.add_argument("--criu-bin",
>>   rp.add_argument("--crit-bin",
>>                   help="Path to crit binary",
>>                   default='../crit/crit')
>> +rp.add_argument("--pre-dump-mode",
>> +                help="Use splice or read mode of pre-dumping",
>> +                choices=['splice', 'read'],
>> +                default='splice')
>>     lp = sp.add_parser("list", help="List tests")
>>   lp.set_defaults(action=list_tests)
>
-Abhishek
Dmitry Safonov Aug. 28, 2019, 4:44 p.m.
Hi Abhishek,

Thanks for doing this work,

On 8/25/19 2:58 AM, Abhishek Dubey wrote:
[..]
> +*--pre-dump-mode*='mode'::
> +    There are two 'mode' to operate pre-dump algorithm. The 'splice' mode
> +    is parasite based, whereas 'read' mode is based on process_vm_readv
> +    syscall. The 'read' mode incurs reduced frozen time and reduced
> +    memory pressure as compared to 'splice' mode. Default is 'splice' mode.
> +

It seems from the code that it's safe to do:
pre-dump-1: splice-based
pre-dump-2: read-based
...

Could you add in doc that it's safe to mix those modes?
Also, could you add a zdtm.py launch with this mode to cover it with
Travis-CI? I think, the proper place would be in:
scripts/travis/travis-tests
And another line that will run "mixed" old-new predumps.

That will make sure that further improvements don't break your work on
new pre-dump mechanism.

[..]
>  /*
> + * 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 */
> @@ -81,6 +87,7 @@ struct cr_options {
>  	int			evasive_devices;
>  	int			link_remap_ok;
>  	int			log_file_per_pid;
> +	int			pre_dump_mode;

Minor stilistic nit: Could we make it enum?

enum pre_dump_mode {
   PRE_DUMP_MODE_SPLICE = 0,
   PRE_DUMP_MODE_READ,
};

Might be my preference, so not insisting.

Thanks,
          Dmitry
Abhishek Dubey Aug. 31, 2019, 12:21 p.m.
Hi Dmitry, Andrei,

On 28/08/19 10:14 PM, Dmitry Safonov wrote:
> Hi Abhishek,
>
> Thanks for doing this work
>
> On 8/25/19 2:58 AM, Abhishek Dubey wrote:
> [..]
>> +*--pre-dump-mode*='mode'::
>> +    There are two 'mode' to operate pre-dump algorithm. The 'splice' mode
>> +    is parasite based, whereas 'read' mode is based on process_vm_readv
>> +    syscall. The 'read' mode incurs reduced frozen time and reduced
>> +    memory pressure as compared to 'splice' mode. Default is 'splice' mode.
>> +
> It seems from the code that it's safe to do:
> pre-dump-1: splice-based
> pre-dump-2: read-based
> ...
>
> Could you add in doc that it's safe to mix those modes?

I have sent [PATCH 9/7] to cover this functionality.

Previously, either all pre-dumps need to be READ mode or SPLICE mode.

This patch adds functionality to run pre-dumps in any order in any mode.

Please read the comment for detailed explanation, why previous code was

breaking while handling mix of pre-dump-modes.

> Also, could you add a zdtm.py launch with this mode to cover it with
> Travis-CI? I think, the proper place would be in:
> scripts/travis/travis-tests
> And another line that will run "mixed" old-new predumps.
>
> That will make sure that further improvements don't break your work on
> new pre-dump mechanism.

With --mix-pre-dump option, this can be done now.

Once patch is approved, I will add in travis.

>
> [..]
>>   /*
>> + * 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 */
>> @@ -81,6 +87,7 @@ struct cr_options {
>>   	int			evasive_devices;
>>   	int			link_remap_ok;
>>   	int			log_file_per_pid;
>> +	int			pre_dump_mode;
> Minor stilistic nit: Could we make it enum?
>
> enum pre_dump_mode {
>     PRE_DUMP_MODE_SPLICE = 0,
>     PRE_DUMP_MODE_READ,
> };
>
> Might be my preference, so not insisting.
Sure :)
>
> Thanks,
>            Dmitry