Re: [PATCH -mm] swsusp: freeze user space processes first

["Rafael J. Wysocki" <rjw@sisk.pl>]

Hi,

On Sunday 05 February 2006 10:38, Andrew Morton wrote:
> "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
> > This patch allows swsusp to freeze processes successfully under heavy load
> > by freezing userspace processes before kernel threads.
> > 
> > ...
> >
> >  /* 0 = success, else # of processes that we failed to stop */
> >  int freeze_processes(void)
> >  {
> > -	int todo;
> > +	int todo, nr_user, user_frozen;
> >  	unsigned long start_time;
> >  	struct task_struct *g, *p;
> >  	unsigned long flags;
> >  
> >  	printk( "Stopping tasks: " );
> >  	start_time = jiffies;
> > +	user_frozen = 0;
> >  	do {
> > -		todo = 0;
> > +		nr_user = todo = 0;
> >  		read_lock(&tasklist_lock);
> >  		do_each_thread(g, p) {
> >  			if (!freezeable(p))
> >  				continue;
> >  			if (frozen(p))
> >  				continue;
> > -
> > -			freeze(p);
> > -			spin_lock_irqsave(&p->sighand->siglock, flags);
> > -			signal_wake_up(p, 0);
> > -			spin_unlock_irqrestore(&p->sighand->siglock, flags);
> > -			todo++;
> > +			if (p->mm && !(p->flags & PF_BORROWED_MM)) {
> > +				/* The task is a user-space one.
> > +				 * Freeze it unless there's a vfork completion
> > +				 * pending
> > +				 */
> > +				if (!p->vfork_done)
> > +					freeze_process(p);
> > +				nr_user++;
> > +			} else {
> > +				/* Freeze only if the user space is frozen */
> > +				if (user_frozen)
> > +					freeze_process(p);
> > +				todo++;
> > +			}
> >  		} while_each_thread(g, p);
> >  		read_unlock(&tasklist_lock);
> > +		todo += nr_user;
> > +		if (!user_frozen && !nr_user) {
> > +			sys_sync();
> > +			start_time = jiffies;
> > +		}
> > +		user_frozen = !nr_user;
> >  		yield();			/* Yield is okay here */
> > -		if (todo && time_after(jiffies, start_time + TIMEOUT)) {
> > -			printk( "\n" );
> > -			printk(KERN_ERR " stopping tasks timed out (%d tasks remaining)\n", todo );
> > +		if (todo && time_after(jiffies, start_time + TIMEOUT))
> >  			break;
> 
> The logic in that loop makes my brain burst.
> 
> What happens if a process does vfork();sleep(100000000)?

The freezing of processes will fail due to the timeout.

Without the if (!p->vfork_done) it would fail too, because the child would
be frozen and the parent would wait for the vfork completion in the
TASK_UNINTERRUPTIBLE state (ie. unfreezeable).  But in that case
we have a race between the "freezer" and the child process
(ie. if the child gets frozen before it completes the vfork completion, the
paret will be unfreezeable) which sometimes leads to a failure when it
should not.  [We have a test case showing this.]

Greetings,
Rafael