Re: Re: freeze_processes questions

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

[-- Attachment #1: Type: text/plain, Size: 3128 bytes --]

Hi,

On Friday, 8 of April 2005 08:23, Pavel Machek wrote:
> On ÄŒt 07-04-05 23:03:15, Rafael J. Wysocki wrote:
> > Hi,
> > 
> > On Thursday, 7 of April 2005 22:00, Alan Stern wrote:
> > > On Thu, 7 Apr 2005, Rafael J. Wysocki wrote:
> > ]--snip--[ 
> > > > > If that means waiting more than 10 seconds or so, you should just give up.  
> > > > > Return an error and put a message in the log saying something like "Can't
> > > > > suspend because process XXX is busy".
> > > > 
> > > > OK, that's what we do now.  Except that IMO we should clear the PF_FREEZE flag
> > > > for this process and do recalc_sigpending() for it after we give up, because
> > > > otherwise it will enter the refrigerator sooner or later and it will stay there.
> > > > Alternatively, we can disable the "freezing loop" in refrigerator() as soon as
> > > > thaw_processes() is started.  Also, we can avoid setting PF_FREEZE for
> > > > processes in TASK_UNINTERRUPTIBLE, but count them as "freezable".  Etc.
> > > 
> > > Yes, all the necessary cleanup steps should be taken.
> > 
> > Pavel, which approach do you like most?
> 
> Clearing PF_FREEZE when we fail to stop some process sounds okay to
> me. It would be nice if the patch was actually tested ;-).

The patch (against 2.6.12-rc2) follows.  It has been tested, but of course it's difficult to
recreate every possible scenario. :-)

Greets,
Rafael


Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> 

--- old/kernel/power/process.c	2005-04-08 13:12:55.000000000 +0200
+++ new/kernel/power/process.c	2005-04-08 13:15:23.000000000 +0200
@@ -60,6 +60,7 @@ int freeze_processes(void)
        int todo;
        unsigned long start_time;
 	struct task_struct *g, *p;
+	unsigned long flags;
 	
 	printk( "Stopping tasks: " );
 	start_time = jiffies;
@@ -67,7 +68,6 @@ int freeze_processes(void)
 		todo = 0;
 		read_lock(&tasklist_lock);
 		do_each_thread(g, p) {
-			unsigned long flags;
 			if (!freezeable(p))
 				continue;
 			if ((p->flags & PF_FROZEN) ||
@@ -85,13 +85,28 @@ int freeze_processes(void)
 		} while_each_thread(g, p);
 		read_unlock(&tasklist_lock);
 		yield();			/* Yield is okay here */
-		if (time_after(jiffies, start_time + TIMEOUT)) {
+		if (todo && time_after(jiffies, start_time + TIMEOUT)) {
 			printk( "\n" );
 			printk(KERN_ERR " stopping tasks failed (%d tasks remaining)\n", todo );
-			return todo;
+			break;
 		}
 	} while(todo);
 	
+	if (todo) {
+		read_lock(&tasklist_lock);
+		do_each_thread(g, p)
+			if (p->flags & PF_FREEZE) {
+				pr_debug("  clean up: %s\n", p->comm);
+				p->flags &= ~PF_FREEZE;
+				spin_lock_irqsave(&p->sighand->siglock, flags);
+				recalc_sigpending_tsk(p);
+				spin_unlock_irqrestore(&p->sighand->siglock, flags);
+			}
+		while_each_thread(g, p);
+		read_unlock(&tasklist_lock);
+		return todo;
+	}
+		
 	printk( "|\n" );
 	BUG_ON(in_atomic());
 	return 0;

-- 
- Would you tell me, please, which way I ought to go from here?
- That depends a good deal on where you want to get to.
		-- Lewis Carroll "Alice's Adventures in Wonderland"


[-- Attachment #2: Type: text/plain, Size: 0 bytes --]