Re: Re: freeze_processes questions

Rafael J. Wysocki
2005-04-08T11:23:50Z

[-- 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"