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

[Cyrill Gorcunov]

On Thu, Aug 04, 2016 at 05:05:41PM +0300, Pavel Emelyanov wrote:
...
> > +++ b/criu/seize.c
> > @@ -319,10 +319,30 @@ static int log_unfrozen_stacks(char *root)
> >  
> >  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,
> > +	};
> 
> Increasing steps were nice. Why do you drop this?

You didn't explain in previous patches why increasing steps are nice?
For what reason? Moreover having solid steps here I think makes code
simplier, and until there indeed some _technical_ reason for delay
extension I would love to keep it constant.

> 
> > +
> > +	if (unlikely(!nr_attempts)) {
> 
> That's ... confusing. I'd better ban --timeout 0 (and negative ;) ) in criu/crtools.c
> and use whatever nr_attempts results here.

You won't belive, but been able to sepcify --timeout 0 here allowed
me and Stas to catch serieous problem in freezer code.

https://lkml.org/lkml/2016/8/3/317

Without this feature we would have to patch criu instead. So you know,
this would be great to keep it for catching more kernel bugs ;)

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

yup, thanks!