[CRIU] [PATCH] seize: Take --timeout option into account when freezing processes

[Pavel Emelyanov]

On 05/26/2016 04:46 PM, Cyrill Gorcunov wrote:
> When we're freezing processes we don't count on anything but
> rather do 5 attempts with constantly increasing delay.
> 
> Lets rather do the following:
> 
>  - take --timeout option into account (which is 5 seconds
>    by default) and split it into 100 ms steps;

Increasing delays make sense I would say. Would you keep them?

>  - when frezing processes check freezer status every 100 ms.
> 
> Same time it looks that 5 seconds by default is too small
> for high loaded containers. Lets increase it to 10 seconds
> by default.
> 
> Reported-by: Stanislav Kinsburskiy <skinsbursky at virtuozzo.com>
> Signed-off-by: Cyrill Gorcunov <gorcunov at virtuozzo.com>
> ---
>  criu/include/cr_options.h |  2 +-
>  criu/seize.c              | 30 ++++++++++++++++++++----------
>  2 files changed, 21 insertions(+), 11 deletions(-)
> 
> diff --git a/criu/include/cr_options.h b/criu/include/cr_options.h
> index b6ae3a1..a9ca1f8 100644
> --- a/criu/include/cr_options.h
> +++ b/criu/include/cr_options.h
> @@ -38,7 +38,7 @@ struct cg_root_opt {
>   */
>  #define DEFAULT_GHOST_LIMIT	(1 << 20)
>  
> -#define DEFAULT_TIMEOUT		5
> +#define DEFAULT_TIMEOUT		10
>  
>  struct irmap;
>  
> diff --git a/criu/seize.c b/criu/seize.c
> index 0ea7a28..863a21e 100644
> --- a/criu/seize.c
> +++ b/criu/seize.c
> @@ -244,10 +244,27 @@ static int freezer_detach(void)
>  
>  static int freeze_processes(void)
>  {
> -	int i, fd, exit_code = -1;
> +	int fd, exit_code = -1;
>  	char path[PATH_MAX];
>  	const char *state = thawed;
>  
> +	static const unsigned long step_ms = 100;
> +	unsigned long nr_attempts = (opts.timeout * 1000000) / step_ms;
> +	unsigned long i;
> +
> +	const struct timespec req = {
> +		.tv_nsec	= step_ms * 1000000,
> +		.tv_sec		= 0,
> +	};
> +
> +	if (unlikely(!nr_attempts)) {

"If timeout if off" means if (!opts.timeout) ;)

> +		/*
> +		 * If timeout is turned off, lets
> +		 * wait for at least 10 seconds.
> +		 */
> +		nr_attempts = (10 * 1000000) / step_ms;

nr_attempts = DEFAULT_TIMEOUT?

> +	}
> +
>  	snprintf(path, sizeof(path), "%s/freezer.state", opts.freeze_cgroup);
>  	fd = open(path, O_RDWR);
>  	if (fd < 0) {
> @@ -275,10 +292,7 @@ static int freeze_processes(void)
>  	 * freezer.state.
>  	 * Here is one extra attempt to check that everything are frozen.
>  	 */
> -	for (i = 0; i <= NR_ATTEMPTS; i++) {
> -		struct timespec req = {};
> -		u64 timeout;
> -
> +	for (i = 0; i <= nr_attempts; i++) {
>  		if (seize_cgroup_tree(opts.freeze_cgroup, state) < 0)
>  			goto err;
>  
> @@ -300,14 +314,10 @@ static int freeze_processes(void)
>  
>  		if (alarm_timeouted())
>  			goto err;
> -
> -		timeout = 100000000 * (i + 1); /* 100 msec */
> -		req.tv_nsec = timeout % 1000000000;
> -		req.tv_sec = timeout / 1000000000;
>  		nanosleep(&req, NULL);
>  	}
>  
> -	if (i > NR_ATTEMPTS) {
> +	if (i > nr_attempts) {
>  		pr_err("Unable to freeze cgroup %s\n", opts.freeze_cgroup);
>  		goto err;
>  	}
>