freeze_processes questions

freeze_processes questions

[Rafael J. Wysocki]

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

Hi,

I have two questions regarding freeze_processes():

1) Shouldn't we take the TASK_UNINTERRUPTIBLE state into consideration?  Now
we will fail if a process is sleeping when we try to freeze it.

2) Is it necessary to check p->state == TASK_STOPPED and
p->state == TASK_TRACED twice in a row (once in freezable() and
then again in the next if ())?

Greets,
Rafael


-- 
- 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 --]

Re: freeze_processes questions

[Pavel Machek]

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

Hi!
> 
> I have two questions regarding freeze_processes():
> 
> 1) Shouldn't we take the TASK_UNINTERRUPTIBLE state into consideration?  Now
> we will fail if a process is sleeping when we try to freeze it.

We can't allow processes to sleep at arbitrary places. Therefore we
send UNINTERRUPTIBLE task a signal, and hope it can get into
refrigerator in time.

> 2) Is it necessary to check p->state == TASK_STOPPED and
> p->state == TASK_TRACED twice in a row (once in freezable() and
> then again in the next if ())?

Those tests should probably be killed from if() below. As far as I can
see, they can not trigger there.
								Pavel
-- 
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!

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

Re: freeze_processes questions

[Rafael J. Wysocki]

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

Hi,

On Tuesday, 5 of April 2005 11:27, Pavel Machek wrote:
> Hi!
> > 
> > I have two questions regarding freeze_processes():
> > 
> > 1) Shouldn't we take the TASK_UNINTERRUPTIBLE state into consideration?  Now
> > we will fail if a process is sleeping when we try to freeze it.
> 
> We can't allow processes to sleep at arbitrary places. Therefore we
> send UNINTERRUPTIBLE task a signal, and hope it can get into
> refrigerator in time.

Hm, AFAIK we can't kill an UNINTERRUPTIBLE process, can we?  Which means that
we can't deliver a signal to it too ...


> > 2) Is it necessary to check p->state == TASK_STOPPED and
> > p->state == TASK_TRACED twice in a row (once in freezable() and
> > then again in the next if ())?
> 
> Those tests should probably be killed from if() below. As far as I can
> see, they can not trigger there.

OK, patch follows. :-)

Greets,
Rafael


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

--- old/kernel/power/process.c	2005-04-05 11:43:56.000000000 +0200
+++ linux-2.6.12-rc2-new/kernel/power/process.c	2005-04-05 11:45:06.000000000 +0200
@@ -70,9 +70,7 @@ int freeze_processes(void)
 			unsigned long flags;
 			if (!freezeable(p))
 				continue;
-			if ((p->flags & PF_FROZEN) ||
-			    (p->state == TASK_TRACED) ||
-			    (p->state == TASK_STOPPED))
+			if (p->flags & PF_FROZEN)
 				continue;
 
 			/* FIXME: smp problem here: we may not access other process' flags

-- 
- 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 --]

Re: freeze_processes questions

[Rafael J. Wysocki]

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

Hi,

On Tuesday, 5 of April 2005 11:53, Rafael J. Wysocki wrote:
> On Tuesday, 5 of April 2005 11:27, Pavel Machek wrote:
> > Hi!
> > > 
> > > I have two questions regarding freeze_processes():
> > > 
> > > 1) Shouldn't we take the TASK_UNINTERRUPTIBLE state into consideration?  Now
> > > we will fail if a process is sleeping when we try to freeze it.
> > 
> > We can't allow processes to sleep at arbitrary places. Therefore we
> > send UNINTERRUPTIBLE task a signal, and hope it can get into
> > refrigerator in time.
> 
> Hm, AFAIK we can't kill an UNINTERRUPTIBLE process, can we?  Which means that
> we can't deliver a signal to it too ...

Well, the problem is that signal_wake_up() is not sufficient to make
an uninterruptible task get the signal.  It only sets TIF_SIGPENDING for the
task, which is not quite good,  IMHO (when the task finally wakes up,
it gets the "signal" and enters refrigerator(), because it has PF_FREEZE set,
and stays there forever, as there's nothing that could clear PF_FROZEN for
it).

I think it may be fixed in one of the two ways:

1) We can treat uninterruptible tasks as non-freezable (in analogy to stopped
tasks) - if all of the other tasks are frozen, nothing can wake up an uninterruptible
task, so this seems to be a safe approach.

2) We can try to force uninterruptible tasks to get their "signals" anyway using
wake_up_state() directly on them (which I don't like as much).

Greets,
Rafael


-- 
- 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 --]

Re: Re: freeze_processes questions

[Alan Stern]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 357 bytes --]

On Tue, 5 Apr 2005, Rafael J. Wysocki wrote:

> 1) We can treat uninterruptible tasks as non-freezable (in analogy to stopped
> tasks) - if all of the other tasks are frozen, nothing can wake up an uninterruptible
> task, so this seems to be a safe approach.

Not true at all.  An uninterruptible task can be woken up by an interrupt 
handler.

Alan Stern


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

Re: Re: freeze_processes questions

[Rafael J. Wysocki]

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

On Tuesday, 5 of April 2005 20:17, Alan Stern wrote:
> On Tue, 5 Apr 2005, Rafael J. Wysocki wrote:
> 
> > 1) We can treat uninterruptible tasks as non-freezable (in analogy to stopped
> > tasks) - if all of the other tasks are frozen, nothing can wake up an uninterruptible
> > task, so this seems to be a safe approach.
> 
> Not true at all.  An uninterruptible task can be woken up by an interrupt 
> handler.

Right.  So it seems we'll have to force uninterruptible tasks to go to the
refrigerator().

Greets,
Rafael


-- 
- 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 --]

Re: freeze_processes questions

[Pavel Machek]

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

Hi!

> > > > I have two questions regarding freeze_processes():
> > > > 
> > > > 1) Shouldn't we take the TASK_UNINTERRUPTIBLE state into consideration?  Now
> > > > we will fail if a process is sleeping when we try to freeze it.
> > > 
> > > We can't allow processes to sleep at arbitrary places. Therefore we
> > > send UNINTERRUPTIBLE task a signal, and hope it can get into
> > > refrigerator in time.
> > 
> > Hm, AFAIK we can't kill an UNINTERRUPTIBLE process, can we?  Which means that
> > we can't deliver a signal to it too ...
> 
> Well, the problem is that signal_wake_up() is not sufficient to make
> an uninterruptible task get the signal.  It only sets TIF_SIGPENDING for the
> task, which is not quite good,  IMHO (when the task finally wakes up,
> it gets the "signal" and enters refrigerator(), because it has PF_FREEZE set,
> and stays there forever, as there's nothing that could clear PF_FROZEN for
> it).

Do you see any problem with UNINTERRUPTIBLE processess? THey should
not stay in UNINTERRUPTIBLE state for too long.. If they get back into
something normal within timeout, we freeze them just okay.

> I think it may be fixed in one of the two ways:
> 
> 1) We can treat uninterruptible tasks as non-freezable (in analogy to stopped
> tasks) - if all of the other tasks are frozen, nothing can wake up an uninterruptible
> task, so this seems to be a safe approach.

HW event can wake that task up.

> 2) We can try to force uninterruptible tasks to get their "signals" anyway using
> wake_up_state() directly on them (which I don't like as much).

They are probably uninterruptible for a reason...

If something is staying in UNINTERRUPTIBLE for more than 1 second, it
is going to cause problems elsewhere, anyway. Do you see that happening?

									Pavel
-- 
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!

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

Re: freeze_processes questions

[Rafael J. Wysocki]

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

Hi,

On Tuesday, 5 of April 2005 22:37, Pavel Machek wrote:
]--snip--[ 
> > 2) We can try to force uninterruptible tasks to get their "signals" anyway using
> > wake_up_state() directly on them (which I don't like as much).
> 
> They are probably uninterruptible for a reason...
> 
> If something is staying in UNINTERRUPTIBLE for more than 1 second, it
> is going to cause problems elsewhere, anyway. Do you see that happening?

Probably.  Please see, for example, this message sent to l-k:

http://marc.theaimsgroup.com/?l=linux-kernel&m=111268969510393&w=2

I can easily trigger a similar behavior with an uninterruptible task and I
have some problems with freezing tasks on SMP that smell like this too.

If we are going to ignore uninterruptible tasks, I'd propose to set PF_FREEZE
and TIF_SIGPENDING for them without counting them as "todo" in
freeze_processes() (if such a task is woken up afterwards by an interrupt
handler, it'll get the "signal" and it'll go to the refrigerator() immediately,
because of the PF_FREEZE).  At least, I'd do this on resume, as we don't care
for these tasks then anyway and IMHO it's not a good idea to fail because of
them.

I think we should also clear PF_FREEZE for uninterruptible tasks in
thaw_processes(), just in case. :-)

Greets,
Rafael


-- 
- 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 --]

Re: freeze_processes questions

[Pavel Machek]

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

Hi!

> > > 2) We can try to force uninterruptible tasks to get their "signals" anyway using
> > > wake_up_state() directly on them (which I don't like as much).
> > 
> > They are probably uninterruptible for a reason...
> > 
> > If something is staying in UNINTERRUPTIBLE for more than 1 second, it
> > is going to cause problems elsewhere, anyway. Do you see that happening?
> 
> Probably.  Please see, for example, this message sent to l-k:
> 
> http://marc.theaimsgroup.com/?l=linux-kernel&m=111268969510393&w=2

...hmm, perhaps kseriod needs suspend/resume support, or something;
but that does not mean we should handle *all* uninterruptible tasks
that way.

> I can easily trigger a similar behavior with an uninterruptible task and I
> have some problems with freezing tasks on SMP that smell like this too.
> 
> If we are going to ignore uninterruptible tasks, I'd propose to set PF_FREEZE
> and TIF_SIGPENDING for them without counting them as "todo" in

You can't just ignore uninterruptible tasks, sorry.

OTOH, if you want working refrigerator, Nigel has one ;-). We were
talking about merging it to mainline few times already...
								Pavel
-- 
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!

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

Re: Re: freeze_processes questions

[Nigel Cunningham]

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

Hi.

On Wed, 2005-04-06 at 08:57, Pavel Machek wrote:
> Hi!
> 
> > > > 2) We can try to force uninterruptible tasks to get their "signals" anyway using
> > > > wake_up_state() directly on them (which I don't like as much).
> > > 
> > > They are probably uninterruptible for a reason...
> > > 
> > > If something is staying in UNINTERRUPTIBLE for more than 1 second, it
> > > is going to cause problems elsewhere, anyway. Do you see that happening?
> > 
> > Probably.  Please see, for example, this message sent to l-k:
> > 
> > http://marc.theaimsgroup.com/?l=linux-kernel&m=111268969510393&w=2
> 
> ...hmm, perhaps kseriod needs suspend/resume support, or something;
> but that does not mean we should handle *all* uninterruptible tasks
> that way.
> 
> > I can easily trigger a similar behavior with an uninterruptible task and I
> > have some problems with freezing tasks on SMP that smell like this too.
> > 
> > If we are going to ignore uninterruptible tasks, I'd propose to set PF_FREEZE
> > and TIF_SIGPENDING for them without counting them as "todo" in
> 
> You can't just ignore uninterruptible tasks, sorry.

I don't think Rafael is suggesting ignoring them. He's suggesting what
I'm already doing:
- Signal so they enter the freezer if they leave the state;
- Don't count them when deciding whether freezing failed;
- Handle the case where they don't leave the state until post resume (I
let them enter the refrigerator, but have code in there to check whether
the freezer is still on).

In this way, I handle kseriod and anything else uninterruptible without
any problems.

> OTOH, if you want working refrigerator, Nigel has one ;-). We were
> talking about merging it to mainline few times already...

Sorry. I keep getting preoccupied with other things. I'll get my act
together :>

Nigel
-- 
Nigel Cunningham
Software Engineer, Canberra, Australia
http://www.cyclades.com
Bus: +61 (2) 6291 9554; Hme: +61 (2) 6292 8028;  Mob: +61 (417) 100 574

Maintainer of Suspend2 Kernel Patches http://suspend2.net


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

Re: Re: freeze_processes questions

[Rafael J. Wysocki]

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

Hi,

On Wednesday, 6 of April 2005 01:10, Nigel Cunningham wrote:
> Hi.
> 
> On Wed, 2005-04-06 at 08:57, Pavel Machek wrote:
> > Hi!
> > 
> > > > > 2) We can try to force uninterruptible tasks to get their "signals" anyway using
> > > > > wake_up_state() directly on them (which I don't like as much).
> > > > 
> > > > They are probably uninterruptible for a reason...
> > > > 
> > > > If something is staying in UNINTERRUPTIBLE for more than 1 second, it
> > > > is going to cause problems elsewhere, anyway. Do you see that happening?
> > > 
> > > Probably.  Please see, for example, this message sent to l-k:
> > > 
> > > http://marc.theaimsgroup.com/?l=linux-kernel&m=111268969510393&w=2
> > 
> > ...hmm, perhaps kseriod needs suspend/resume support, or something;

It seems that anything compiled in (ie not as a module) which ends up in
call_usermodehelper() during boot is likely to need this kind of handling.

> > but that does not mean we should handle *all* uninterruptible tasks
> > that way.
> > 
> > > I can easily trigger a similar behavior with an uninterruptible task and I
> > > have some problems with freezing tasks on SMP that smell like this too.
> > > 
> > > If we are going to ignore uninterruptible tasks, I'd propose to set PF_FREEZE
> > > and TIF_SIGPENDING for them without counting them as "todo" in
> > 
> > You can't just ignore uninterruptible tasks, sorry.
> 
> I don't think Rafael is suggesting ignoring them.

Words, words ... ;-)

> He's suggesting what I'm already doing:
> - Signal so they enter the freezer if they leave the state;
> - Don't count them when deciding whether freezing failed;
> - Handle the case where they don't leave the state until post resume (I
> let them enter the refrigerator, but have code in there to check whether
> the freezer is still on).

Yup.  Actaully, I think of something like this (tested once):

--- old/kernel/power/process.c	2005-04-06 08:30:47.000000000 +0200
+++ new/kernel/power/process.c	2005-04-06 08:31:31.000000000 +0200
@@ -18,6 +18,7 @@
  */
 #define TIMEOUT	(6 * HZ)
 
+static int refrigerator_disabled;
 
 static inline int freezeable(struct task_struct * p)
 {
@@ -47,9 +48,12 @@ void refrigerator(unsigned long flag)
 	recalc_sigpending(); /* We sent fake signal, clean it up */
 	spin_unlock_irq(&current->sighand->siglock);
 
-	current->flags |= PF_FROZEN;
-	while (current->flags & PF_FROZEN)
-		schedule();
+	if (!refrigerator_disabled) {
+		current->flags |= PF_FROZEN;
+		while (current->flags & PF_FROZEN)
+			schedule();
+	}
+
 	pr_debug("%s left refrigerator\n", current->comm);
 	current->state = save;
 }
@@ -62,6 +66,7 @@ int freeze_processes(void)
 	struct task_struct *g, *p;
 	
 	printk( "Stopping tasks: " );
+	refrigerator_disabled = 0;
 	start_time = jiffies;
 	do {
 		todo = 0;
@@ -72,6 +77,9 @@ int freeze_processes(void)
 				continue;
 			if (p->flags & PF_FROZEN)
 				continue;
+			if ((p->flags & PF_FREEZE) &&
+			    (p->state == TASK_UNINTERRUPTIBLE))
+			    	continue;
 
 			/* FIXME: smp problem here: we may not access other process' flags
 			   without locking */
@@ -100,6 +108,7 @@ void thaw_processes(void)
 	struct task_struct *g, *p;
 
 	printk( "Restarting tasks..." );
+	refrigerator_disabled = 1;
 	read_lock(&tasklist_lock);
 	do_each_thread(g, p) {
 		if (!freezeable(p))

>
> In this way, I handle kseriod and anything else uninterruptible without
> any problems.

Exactly. :-)

Greets,
Rafael


-- 
- 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 --]

Re: Re: freeze_processes questions

[Alan Stern]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 654 bytes --]

On Wed, 6 Apr 2005, Nigel Cunningham wrote:

> I don't think Rafael is suggesting ignoring them. He's suggesting what
> I'm already doing:
> - Signal so they enter the freezer if they leave the state;
> - Don't count them when deciding whether freezing failed;
> - Handle the case where they don't leave the state until post resume (I
> let them enter the refrigerator, but have code in there to check whether
> the freezer is still on).
> 
> In this way, I handle kseriod and anything else uninterruptible without
> any problems.

What happens if a process owns a lock needed to suspend a device and it is 
waiting in TASK_UNINTERRUPTIBLE?

Alan Stern


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

Re: Re: freeze_processes questions

[Rafael J. Wysocki]

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

Hi,

On Wednesday, 6 of April 2005 21:06, Alan Stern wrote:
> On Wed, 6 Apr 2005, Nigel Cunningham wrote:
> 
> > I don't think Rafael is suggesting ignoring them. He's suggesting what
> > I'm already doing:
> > - Signal so they enter the freezer if they leave the state;
> > - Don't count them when deciding whether freezing failed;
> > - Handle the case where they don't leave the state until post resume (I
> > let them enter the refrigerator, but have code in there to check whether
> > the freezer is still on).
> > 
> > In this way, I handle kseriod and anything else uninterruptible without
> > any problems.
> 
> What happens if a process owns a lock needed to suspend a device and it is 
> waiting in TASK_UNINTERRUPTIBLE?

Well, we're in trouble. :-)

However, if any process that we have frozen owns such a lock, we're in trouble
too.  Thus it won't help if we forcibly freeze the uninterruptible process.
Worse yet, there won't be any way to wake it up afterwards.  OTOH, if we leave
it in TASK_UNINTERRUPTIBLE, it may (potentially) be woken up by an interrupt
handler (as you pointed out previously :-)) and release the lock.

Frankly, I can't see any probable scenario leading to this kind of trouble.
Seemingly, it would require someone to take a lock and (knowingly) do
something that could end up in uninterruptible sleep, which IMHO is not
a good idea.

Greets,
Rafael


-- 
- 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 --]

Re: Re: freeze_processes questions

[Alan Stern]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 928 bytes --]

On Wed, 6 Apr 2005, Rafael J. Wysocki wrote:

> > What happens if a process owns a lock needed to suspend a device and it is 
> > waiting in TASK_UNINTERRUPTIBLE?
> 
> Well, we're in trouble. :-)
> 
> However, if any process that we have frozen owns such a lock, we're in trouble
> too.  Thus it won't help if we forcibly freeze the uninterruptible process.
> Worse yet, there won't be any way to wake it up afterwards.  OTOH, if we leave
> it in TASK_UNINTERRUPTIBLE, it may (potentially) be woken up by an interrupt
> handler (as you pointed out previously :-)) and release the lock.
> 
> Frankly, I can't see any probable scenario leading to this kind of trouble.
> Seemingly, it would require someone to take a lock and (knowingly) do
> something that could end up in uninterruptible sleep, which IMHO is not
> a good idea.

It happens all the time in the USB core, when dealing with a 
newly-connected device.

Alan Stern


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

Re: Re: freeze_processes questions

[David Brownell]

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

On Wednesday 06 April 2005 2:03 pm, Alan Stern wrote:
> On Wed, 6 Apr 2005, Rafael J. Wysocki wrote:
> 
> > Frankly, I can't see any probable scenario leading to this kind of trouble.
> > Seemingly, it would require someone to take a lock and (knowingly) do
> > something that could end up in uninterruptible sleep, which IMHO is not
> > a good idea.
> 
> It happens all the time in the USB core, when dealing with a 
> newly-connected device.

True, it does happen then.  One of many examples of where the USB team
must regularly deal with issues that some simpler driver stacks are able
to avoid ... or at least think they're able to avoid.  (Hotplugging and
hot-unplugging tend to complicate things.)

That said, why wouldn't that be a good idea?  The locking infrastructure
in Linux was certainly designed to make that work properly.  And it's
common in other operating systems too.  Tasks block each other all the
time; it's simpler to adopt that model than the alternatives, and hence
a lot easier to maintain.

- Dave

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

Re: Re: freeze_processes questions

[Pavel Machek]

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

Hi!

> > > I don't think Rafael is suggesting ignoring them. He's suggesting what
> > > I'm already doing:
> > > - Signal so they enter the freezer if they leave the state;
> > > - Don't count them when deciding whether freezing failed;
> > > - Handle the case where they don't leave the state until post resume (I
> > > let them enter the refrigerator, but have code in there to check whether
> > > the freezer is still on).
> > > 
> > > In this way, I handle kseriod and anything else uninterruptible without
> > > any problems.
> > 
> > What happens if a process owns a lock needed to suspend a device and it is 
> > waiting in TASK_UNINTERRUPTIBLE?
> 
> Well, we're in trouble. :-)
> 
> However, if any process that we have frozen owns such a lock, we're in trouble
> too.

No, we are not. Processes can't own any locks when they are in
refrigerator... It is not ok to call refrigerator from any context
where you own a lock.

OTOH it is okay to enter TASK_UNINTERRUPTIBLE with locks held.

								Pavel
-- 
Boycott Kodak -- for their patent abuse against Java.

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

Re: Re: freeze_processes questions

[Rafael J. Wysocki]

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

Hi,

On Wednesday, 6 of April 2005 23:17, Pavel Machek wrote:
> Hi!
> 
> > > > I don't think Rafael is suggesting ignoring them. He's suggesting what
> > > > I'm already doing:
> > > > - Signal so they enter the freezer if they leave the state;
> > > > - Don't count them when deciding whether freezing failed;
> > > > - Handle the case where they don't leave the state until post resume (I
> > > > let them enter the refrigerator, but have code in there to check whether
> > > > the freezer is still on).
> > > > 
> > > > In this way, I handle kseriod and anything else uninterruptible without
> > > > any problems.
> > > 
> > > What happens if a process owns a lock needed to suspend a device and it is 
> > > waiting in TASK_UNINTERRUPTIBLE?
> > 
> > Well, we're in trouble. :-)
> > 
> > However, if any process that we have frozen owns such a lock, we're in trouble
> > too.
> 
> No, we are not. Processes can't own any locks when they are in
> refrigerator... It is not ok to call refrigerator from any context
> where you own a lock.

I didn't mean a lock in general, but a lock that is needed to suspend a device.
If a process holding that lock is frozen, the _suspend() routine for the device
won't complete, because it won't be able to get the lock.  IOW it will go to
sleep and we are single-threaded at that time ...  Or am I missing anything?

Rafael
 

-- 
- 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 --]

Re: Re: freeze_processes questions

[Nigel Cunningham]

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

Hi.

On Thu, 2005-04-07 at 07:35, Rafael J. Wysocki wrote:
> On Wednesday, 6 of April 2005 23:17, Pavel Machek wrote:
> > > > > I don't think Rafael is suggesting ignoring them. He's suggesting what
> > > > > I'm already doing:
> > > > > - Signal so they enter the freezer if they leave the state;
> > > > > - Don't count them when deciding whether freezing failed;
> > > > > - Handle the case where they don't leave the state until post resume (I
> > > > > let them enter the refrigerator, but have code in there to check whether
> > > > > the freezer is still on).
> > > > > 
> > > > > In this way, I handle kseriod and anything else uninterruptible without
> > > > > any problems.
> > > > 
> > > > What happens if a process owns a lock needed to suspend a device and it is 
> > > > waiting in TASK_UNINTERRUPTIBLE?
> > > 
> > > Well, we're in trouble. :-)
> > > 
> > > However, if any process that we have frozen owns such a lock, we're in trouble
> > > too.
> > 
> > No, we are not. Processes can't own any locks when they are in
> > refrigerator... It is not ok to call refrigerator from any context
> > where you own a lock.
> 
> I didn't mean a lock in general, but a lock that is needed to suspend a device.
> If a process holding that lock is frozen, the _suspend() routine for the device
> won't complete, because it won't be able to get the lock.  IOW it will go to
> sleep and we are single-threaded at that time ...  Or am I missing anything?

Whenever this topic comes up, the discussion is always abstract. If
someone could think of a lock that might actually deadlock suspending,
we look seriously at the issue. In the meantime, all I can say, from
roughly three years of developing and testing, is that it never seems to
happen. And yes, I know that doesn't necessarily mean it can't.

Perhaps it helps to remember that we're using bdev_io routines. All of
the filesystem locking is completely out of the picture. Furthermore, we
flush (well Suspend2 does) all pending I/O before freezing kernel
threads.

Regards,

Nigel
-- 
Nigel Cunningham
Software Engineer, Canberra, Australia
http://www.cyclades.com
Bus: +61 (2) 6291 9554; Hme: +61 (2) 6292 8028;  Mob: +61 (417) 100 574

Maintainer of Suspend2 Kernel Patches http://suspend2.net


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

Re: Re: freeze_processes questions

[Rafael J. Wysocki]

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

Hi,

On Thursday, 7 of April 2005 00:07, Nigel Cunningham wrote:
> Hi.
> 
> On Thu, 2005-04-07 at 07:35, Rafael J. Wysocki wrote:
> > On Wednesday, 6 of April 2005 23:17, Pavel Machek wrote:
> > > > > > I don't think Rafael is suggesting ignoring them. He's suggesting what
> > > > > > I'm already doing:
> > > > > > - Signal so they enter the freezer if they leave the state;
> > > > > > - Don't count them when deciding whether freezing failed;
> > > > > > - Handle the case where they don't leave the state until post resume (I
> > > > > > let them enter the refrigerator, but have code in there to check whether
> > > > > > the freezer is still on).
> > > > > > 
> > > > > > In this way, I handle kseriod and anything else uninterruptible without
> > > > > > any problems.
> > > > > 
> > > > > What happens if a process owns a lock needed to suspend a device and it is 
> > > > > waiting in TASK_UNINTERRUPTIBLE?
> > > > 
> > > > Well, we're in trouble. :-)
> > > > 
> > > > However, if any process that we have frozen owns such a lock, we're in trouble
> > > > too.
> > > 
> > > No, we are not. Processes can't own any locks when they are in
> > > refrigerator... It is not ok to call refrigerator from any context
> > > where you own a lock.
> > 
> > I didn't mean a lock in general, but a lock that is needed to suspend a device.
> > If a process holding that lock is frozen, the _suspend() routine for the device
> > won't complete, because it won't be able to get the lock.  IOW it will go to
> > sleep and we are single-threaded at that time ...  Or am I missing anything?
> 
> Whenever this topic comes up, the discussion is always abstract. If
> someone could think of a lock that might actually deadlock suspending,
> we look seriously at the issue.

Well, (having thought about it for a while) why, actually, a device's _suspend()
routine would need such a lock?  Devices are suspended on one CPU in
a single thread which can only (potentially) race with interrupt handlers ...

> In the meantime, all I can say, from roughly three years of developing
> and testing, is that it never seems to happen.

Well, I have much less experience, but I don't think it's likely to happen,
too. :-)

Greets,
Rafael
 

-- 
- 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 --]

Re: Re: freeze_processes questions

[Alan Stern]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 436 bytes --]

On Thu, 7 Apr 2005, Rafael J. Wysocki wrote:

> Well, (having thought about it for a while) why, actually, a device's _suspend()
> routine would need such a lock?  Devices are suspended on one CPU in
> a single thread which can only (potentially) race with interrupt handlers ...

You're blinded by thinking that suspend() can be called only during a 
system sleep transition.  Don't forget about runtime power management!

Alan Stern


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

Re: Re: freeze_processes questions

[Rafael J. Wysocki]

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

Hi,

On Thursday, 7 of April 2005 16:41, Alan Stern wrote:
> On Thu, 7 Apr 2005, Rafael J. Wysocki wrote:
> 
> > Well, (having thought about it for a while) why, actually, a device's _suspend()
> > routine would need such a lock?  Devices are suspended on one CPU in
> > a single thread which can only (potentially) race with interrupt handlers ...
> 
> You're blinded by thinking that suspend() can be called only during a 
> system sleep transition.

Well, yes. :-)

> Don't forget about runtime power management! 

Of course you're right.  Thanks for pointing this out.

Greets,
Rafael


-- 
- 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 --]

Re: Re: freeze_processes questions

[Alan Stern]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 692 bytes --]

On Thu, 7 Apr 2005, Nigel Cunningham wrote:

> Whenever this topic comes up, the discussion is always abstract. If
> someone could think of a lock that might actually deadlock suspending,
> we look seriously at the issue. In the meantime, all I can say, from
> roughly three years of developing and testing, is that it never seems to
> happen. And yes, I know that doesn't necessarily mean it can't.

I agree with Nigel.  Certainly in the USB part of the kernel the
uninterruptible sleeps all have rather short, bounded durations.  
Furthermore they correspond to specific events -- a new device is plugged
in -- which are unlikely to occur just as the system is going to sleep.

Alan Stern


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

Re: Re: freeze_processes questions

[Rafael J. Wysocki]

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

Hi,

On Thursday, 7 of April 2005 16:43, Alan Stern wrote:
> On Thu, 7 Apr 2005, Nigel Cunningham wrote:
> 
> > Whenever this topic comes up, the discussion is always abstract. If
> > someone could think of a lock that might actually deadlock suspending,
> > we look seriously at the issue. In the meantime, all I can say, from
> > roughly three years of developing and testing, is that it never seems to
> > happen. And yes, I know that doesn't necessarily mean it can't.
> 
> I agree with Nigel.  Certainly in the USB part of the kernel the
> uninterruptible sleeps all have rather short, bounded durations.  
> Furthermore they correspond to specific events -- a new device is plugged
> in -- which are unlikely to occur just as the system is going to sleep.

Strangely enough, I agree with him too. :-)  Still, Nigel says he is handling
uninterruptible tasks in the way that I was thinking of (as far as I
understand it), which you say is a mistake ...

Greets,
Rafael


-- 
- 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 --]

Re: Re: freeze_processes questions

[Pavel Machek]

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

Hi!

> > > > What happens if a process owns a lock needed to suspend a device and it is 
> > > > waiting in TASK_UNINTERRUPTIBLE?
> > > 
> > > Well, we're in trouble. :-)
> > > 
> > > However, if any process that we have frozen owns such a lock, we're in trouble
> > > too.
> > 
> > No, we are not. Processes can't own any locks when they are in
> > refrigerator... It is not ok to call refrigerator from any context
> > where you own a lock.
> 
> I didn't mean a lock in general, but a lock that is needed to
> suspend a device.

Processes in refrigerator are not allowed to own any locks. That means
they may not own a lock that is needed to suspend a device ;-).

OTOH... we may want to move completely away from refrigerator. Its
quite a hack, and it device support is okay, we'll not really need it.

								Pavel
-- 
Boycott Kodak -- for their patent abuse against Java.

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

Re: Re: freeze_processes questions

[Rafael J. Wysocki]

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

Hi,

On Thursday, 7 of April 2005 11:11, Pavel Machek wrote:
> Hi!
> 
> > > > > What happens if a process owns a lock needed to suspend a device and it is 
> > > > > waiting in TASK_UNINTERRUPTIBLE?
> > > > 
> > > > Well, we're in trouble. :-)
> > > > 
> > > > However, if any process that we have frozen owns such a lock, we're in trouble
> > > > too.
> > > 
> > > No, we are not. Processes can't own any locks when they are in
> > > refrigerator... It is not ok to call refrigerator from any context
> > > where you own a lock.
> > 
> > I didn't mean a lock in general, but a lock that is needed to
> > suspend a device.
> 
> Processes in refrigerator are not allowed to own any locks. That means
> they may not own a lock that is needed to suspend a device ;-).

I am confused now.  AFAICS, we don't check anywhere if a process we
are going to freeze holds any locks.  If we have a task in
TASK_INTERRUPTIBLE, we just send it a fake signal which causes it to go
to the refrigerator().  Could you please tell me how we prevent such a task
from holding any locks?  [It probably doesn't matter, at least practically,
but ...? :-)]

> OTOH... we may want to move completely away from refrigerator. Its
> quite a hack, and it device support is okay, we'll not really need it.

Still, it won't happen soon, I guess. :-)  As of today, we have the
refrigerator and the processes in TASK_UNINTERRUPTIBLE are mishandled.
I think we should do something about it, at least for now, until we drop
the refrigerator altogether (if we are going to drop it).  That's why I
started the discussion and sent the patch.

Greets,
Rafael


-- 
- 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 --]

Re: Re: freeze_processes questions

[Nigel Cunningham]

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

Hi.

On Thu, 2005-04-07 at 22:29, Rafael J. Wysocki wrote:
> I am confused now.  AFAICS, we don't check anywhere if a process we
> are going to freeze holds any locks.  If we have a task in
> TASK_INTERRUPTIBLE, we just send it a fake signal which causes it to go
> to the refrigerator().  Could you please tell me how we prevent such a task
> from holding any locks?  [It probably doesn't matter, at least practically,
> but ...? :-)]

Checking that no locks aren't held is impossible at the moment afaik.
There's no 'I'm holding n locks counter'. And even if there were, if you
get rid of the refrigerator, you still can't guarantee that you won't
deadlock against processes grabbing locks after your atomic copy.

Dropping the freezer also means you're forever limiting suspend to
writing a maximum of one half of memory - unless you're going to do
non-atomic images.

> > OTOH... we may want to move completely away from refrigerator. Its
> > quite a hack, and it device support is okay, we'll not really need it.
> 
> Still, it won't happen soon, I guess. :-)  As of today, we have the
> refrigerator and the processes in TASK_UNINTERRUPTIBLE are mishandled.
> I think we should do something about it, at least for now, until we drop
> the refrigerator altogether (if we are going to drop it).  That's why I
> started the discussion and sent the patch.

I believe dropping the refrigerator is a fundamentally broken idea. You
have to (at least) stop processes doing disk I/O (otherwise the snapshot
you just took won't match the on-disk state). At the same time, though,
you still need I/O to work for your suspend implementation. You're then
talking about putting nous in the drivers which says "Hold this I/O,
it's not suspend but process this - it's suspend."

Regards,

Nigel
-- 
Nigel Cunningham
Software Engineer, Canberra, Australia
http://www.cyclades.com
Bus: +61 (2) 6291 9554; Hme: +61 (2) 6292 8028;  Mob: +61 (417) 100 574

Maintainer of Suspend2 Kernel Patches http://suspend2.net


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

Re: Re: freeze_processes questions

[Alan Stern]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1581 bytes --]

On Thu, 7 Apr 2005, Rafael J. Wysocki wrote:

> I am confused now.  AFAICS, we don't check anywhere if a process we
> are going to freeze holds any locks.

You're not supposed to check.  If the process is running in userspace then 
it can't be holding any locks, by definition.  If it's running in the 
kernel then it's supposed to do its own checking.  It won't call 
refrigerator() unless it knows that no locks are held.

>  If we have a task in
> TASK_INTERRUPTIBLE, we just send it a fake signal which causes it to go
> to the refrigerator().  Could you please tell me how we prevent such a task
> from holding any locks?  [It probably doesn't matter, at least practically,
> but ...? :-)]

You can't prevent it, and what you're doing is a mistake.

> > OTOH... we may want to move completely away from refrigerator. Its
> > quite a hack, and it device support is okay, we'll not really need it.
> 
> Still, it won't happen soon, I guess. :-)  As of today, we have the
> refrigerator and the processes in TASK_UNINTERRUPTIBLE are mishandled.
> I think we should do something about it, at least for now, until we drop
> the refrigerator altogether (if we are going to drop it).  That's why I
> started the discussion and sent the patch.

It's very simple.  Processes in TASK_UNINTERRUPTIBLE can't be put in the
refrigerator, so you have to wait until they can be put there.  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".

Alan Stern


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

Re: Re: freeze_processes questions

[Rafael J. Wysocki]

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

Hi,

On Thursday, 7 of April 2005 16:36, Alan Stern wrote:
> On Thu, 7 Apr 2005, Rafael J. Wysocki wrote:
> 
> > I am confused now.  AFAICS, we don't check anywhere if a process we
> > are going to freeze holds any locks.
> 
> You're not supposed to check.  If the process is running in userspace then 
> it can't be holding any locks, by definition.  If it's running in the 
> kernel then it's supposed to do its own checking.  It won't call 
> refrigerator() unless it knows that no locks are held.

Oh, well.  The processes that are running in the kernel always enter the
refrigerator voluntarily ...

]--snip--[
> > > OTOH... we may want to move completely away from refrigerator. Its
> > > quite a hack, and it device support is okay, we'll not really need it.
> > 
> > Still, it won't happen soon, I guess. :-)  As of today, we have the
> > refrigerator and the processes in TASK_UNINTERRUPTIBLE are mishandled.
> > I think we should do something about it, at least for now, until we drop
> > the refrigerator altogether (if we are going to drop it).  That's why I
> > started the discussion and sent the patch.
> 
> It's very simple.  Processes in TASK_UNINTERRUPTIBLE can't be put in the
> refrigerator, so you have to wait until they can be put there.

It seems that if a process calls wait_for_completion() right before suspend,
the other task supposed to complete its completion may be accidentally frozen
before it's able to do this.  It looks like this happens to kseroid sometimes
on suspend-during-resume.

> 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.

Greets,
Rafael


-- 
- 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 --]

Re: Re: freeze_processes questions

[Alan Stern]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1735 bytes --]

On Thu, 7 Apr 2005, Rafael J. Wysocki wrote:

> > It's very simple.  Processes in TASK_UNINTERRUPTIBLE can't be put in the
> > refrigerator, so you have to wait until they can be put there.
> 
> It seems that if a process calls wait_for_completion() right before suspend,
> the other task supposed to complete its completion may be accidentally frozen
> before it's able to do this.  It looks like this happens to kseroid sometimes
> on suspend-during-resume.

You mean, when a suspend occurs so soon after a resume that the resume has 
fully completed yet?  Aren't there interlocks to prevent that sort of 
thing from happening?

Anyway, if a process is waiting for something that's not going to happen 
because some other process is frozen, then the suspend should fail.  I 
don't see any other choice.  However, it might help in some situations to 
send a FREEZE message to some device drivers before putting processes in 
the refrigerator -- especially drivers that use a kernel thread.

> > 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.

Alan Stern


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

Re: Re: freeze_processes questions

[Rafael J. Wysocki]

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

Hi,

On Thursday, 7 of April 2005 22:00, Alan Stern wrote:
> On Thu, 7 Apr 2005, Rafael J. Wysocki wrote:
> 
> > > It's very simple.  Processes in TASK_UNINTERRUPTIBLE can't be put in the
> > > refrigerator, so you have to wait until they can be put there.
> > 
> > It seems that if a process calls wait_for_completion() right before suspend,
> > the other task supposed to complete its completion may be accidentally frozen
> > before it's able to do this.  It looks like this happens to kseroid sometimes
> > on suspend-during-resume.
> 
> You mean, when a suspend occurs so soon after a resume that the resume has 
> fully completed yet?

No.

During resume, we boot the kernel, check if there's an image to read and
(if there's one) we freeze processes using the refrigerator (we need to get rid
of them so that we can restore the image safely).  Then, it seems, on
some systems, freeze_processes() may be called before all of the compiled-in
drivers complete their initialization.  In particular, kseriod calls usermodehelper
at that time, which makes it wait uninterruptibly for a helper process to
complete.  If that helper process is frozen, we end up with the uninterruptible
kseriod that we can't freeze and _resume_ fails, which is not nice, especially
that we don't need to care for this kseriod, as we are going to restore
the image in a while ...

Perhaps we should not use the refrigerator during resume, or we could use a
slightly modified version?  I don't know.

Greets,
Rafael
 

-- 
- 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 --]

Re: Re: freeze_processes questions

[Alan Stern]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1473 bytes --]

On Thu, 7 Apr 2005, Rafael J. Wysocki wrote:

> During resume, we boot the kernel, check if there's an image to read and
> (if there's one) we freeze processes using the refrigerator (we need to get rid
> of them so that we can restore the image safely).  Then, it seems, on
> some systems, freeze_processes() may be called before all of the compiled-in
> drivers complete their initialization.  In particular, kseriod calls usermodehelper
> at that time, which makes it wait uninterruptibly for a helper process to
> complete.  If that helper process is frozen, we end up with the uninterruptible
> kseriod that we can't freeze and _resume_ fails, which is not nice, especially
> that we don't need to care for this kseriod, as we are going to restore
> the image in a while ...
> 
> Perhaps we should not use the refrigerator during resume, or we could use a
> slightly modified version?  I don't know.

Kernel threads calling (and waiting for!) userspace processes is a very 
difficult problem for suspend.  It comes up in several different places, 
and it doesn't seem to have any easy solution.

It might be best to require that kernel threads should always be
freeze-able when they do this, which in particular implies that they must
not hold any locks.  While that should work, it would create problems for
a number of drivers that use the firmware loader during their probe()  
routine.  There will probably be other similar problems I'm not aware of.

Alan Stern


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

Re: Re: freeze_processes questions

[Pavel Machek]

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

Hi!

> > > It seems that if a process calls wait_for_completion() right before suspend,
> > > the other task supposed to complete its completion may be accidentally frozen
> > > before it's able to do this.  It looks like this happens to kseroid sometimes
> > > on suspend-during-resume.
> > 
> > You mean, when a suspend occurs so soon after a resume that the resume has 
> > fully completed yet?
> 
> No.
> 
> During resume, we boot the kernel, check if there's an image to read and
> (if there's one) we freeze processes using the refrigerator (we need to get rid
> of them so that we can restore the image safely).  Then, it seems, on
> some systems, freeze_processes() may be called before all of the compiled-in
> drivers complete their initialization.  In particular, kseriod calls usermodehelper
> at that time, which makes it wait uninterruptibly for a helper process to
> complete.  If that helper process is frozen, we end up with the uninterruptible
> kseriod that we can't freeze and _resume_ fails, which is not nice, especially
> that we don't need to care for this kseriod, as we are going to restore
> the image in a while ...
> 
> Perhaps we should not use the refrigerator during resume, or we could use a
> slightly modified version?  I don't know.

As we allow resume to be called from userland, other refrigerator is
not really an option. We could insert mdelay before resume, or some
equivalent trick...
								Pavel
-- 
Boycott Kodak -- for their patent abuse against Java.

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

Re: Re: freeze_processes questions

[Rafael J. Wysocki]

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

Hi,

On Friday, 8 of April 2005 08:20, Pavel Machek wrote:
> Hi!
> 
]--snip--[
> 
> As we allow resume to be called from userland, other refrigerator is
> not really an option. We could insert mdelay before resume, or some
> equivalent trick...

I have an idea (provided we can disable/enable usermodehelper as in the
Dmitry's patch):

--- old/kernel/power/process.c	2005-04-06 08:30:47.000000000 +0200
+++ new/kernel/power/process.c	2005-04-08 11:11:23.000000000 +0200
@@ -62,6 +62,26 @@ int freeze_processes(void)
 	struct task_struct *g, *p;
 	
 	printk( "Stopping tasks: " );
+	usermodehelper_disable();
+	yield();
+	start_time = jiffies;
+	do {
+		todo = 0;
+		read_lock(&tasklist_lock);
+		do_each_thread(g, p)
+			if (!(p->flags & PF_NOFREEZE) &&
+			    (p->state == TASK_UNINTERRUPTIBLE))
+				todo++;
+		while_each_thread(g, p);
+		read_unlock(&tasklist_lock);
+		yield();
+		if (time_after(jiffies, start_time + TIMEOUT)) {
+			printk( "\n" );
+			printk(KERN_ERR " stopping tasks faliled (%d unstoppable tasks)\n",
+					todo);
+			return todo;
+		}
+	} while(todo);
 	start_time = jiffies;
 	do {
 		todo = 0;
@@ -100,6 +120,7 @@ void thaw_processes(void)
 	struct task_struct *g, *p;
 
 	printk( "Restarting tasks..." );
+	usermodehelper_enable();
 	read_lock(&tasklist_lock);
 	do_each_thread(g, p) {
 		if (!freezeable(p))

It's not bullet-proof, but should be enough to handle the
"freeze-during-resume" case ...

Greets,
Rafael


-- 
- 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 --]

Re: Re: freeze_processes questions

[Rafael J. Wysocki]

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

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?

Rafael


-- 
- 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 --]

Re: Re: freeze_processes questions

[Pavel Machek]

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

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 ;-).

I do not think you can deal with TASK_UNINTERRUPTIBLE in some
reasonable way... If you do something with them be sure to make it
separate patch :-).
							Pavel
-- 
Boycott Kodak -- for their patent abuse against Java.

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

Re: Re: freeze_processes questions

[Rafael J. Wysocki]

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

Re: Re: freeze_processes questions

[Pavel Machek]

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

Hi!

> > 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. :-)

Applied, modulo some whitespace.
								Pavel

-- 
Boycott Kodak -- for their patent abuse against Java.

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

Re: Re: freeze_processes questions

[Li Shaohua]

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

Hi,
Sorry for raising the problem again after a long time. I got some
reports about ' xxx not stopped' at suspend time. The general cause is
one task is waiting for another task which has been into refrigerator
first. Report an error and stop suspend can't solve the problem. After
the task's dependent task is released from refrigerator, the task itself
will soon go into refrigerator and will never be waked up. 
Nigel's refrigerator patch half solves the problem. It distinct kernel
tasks and user tasks, so can solve the dependence between kernel tasks
and user tasks, but can't solve the user tasks dependence. Right, Nigel?
Below is a proposal method to solve the issue. Assume task A depends on
task B. Task B is frozen first. When we find task A can't be frozen
after a long time, we thaw all tasks and let task A continue (task A has
gotten a freeze signal so it will soon go into refrigerator) and then we
freeze other tasks again. In theory, this can solve all task dependence
(sure can't solve the circled dependence) if the retry times are enough
big, but cost maybe too high. Looks it help my case (syslogd can't be
stopped because it's waitting for kjournald). Any idea?

Thanks,
Shaohua



---

 linux-2.6.11-root/kernel/power/process.c |   36 +++++++++++++++++++++++++++----
 1 files changed, 32 insertions(+), 4 deletions(-)

diff -puN kernel/power/process.c~freeze_process_order kernel/power/process.c
--- linux-2.6.11/kernel/power/process.c~freeze_process_order	2005-04-27 15:39:38.014090112 +0800
+++ linux-2.6.11-root/kernel/power/process.c	2005-04-27 17:02:37.884034808 +0800
@@ -16,7 +16,8 @@
 /* 
  * Timeout for stopping processes
  */
-#define TIMEOUT	(6 * HZ)
+#define TIMEOUT	(3 * HZ)
+#define RETRY (2)
 
 
 static inline int freezeable(struct task_struct * p)
@@ -54,12 +55,34 @@ void refrigerator(unsigned long flag)
 	current->state = save;
 }
 
+static void freeze_processes_failure(void)
+{
+	struct task_struct *g, *p;
+
+	/* thaw frozen processes and let process which received a frozen signal
+	 * but was waitting for other process go.
+	 */
+	read_lock(&tasklist_lock);
+	do_each_thread(g, p) {
+		if (!freezeable(p))
+			continue;
+		if (p->flags & PF_FROZEN) {
+			p->flags &= ~PF_FROZEN;
+			wake_up_process(p);
+		}
+	} while_each_thread(g, p);
+	read_unlock(&tasklist_lock);
+
+	yield();
+}
+
 /* 0 = success, else # of processes that we failed to stop */
 int freeze_processes(void)
 {
        int todo;
        unsigned long start_time;
 	struct task_struct *g, *p;
+	int retry_times = 1;
 	
 	printk( "Stopping tasks: " );
 	start_time = jiffies;
@@ -86,9 +109,14 @@ int freeze_processes(void)
 		read_unlock(&tasklist_lock);
 		yield();			/* Yield is okay here */
 		if (time_after(jiffies, start_time + TIMEOUT)) {
-			printk( "\n" );
-			printk(KERN_ERR " stopping tasks failed (%d tasks remaining)\n", todo );
-			return todo;
+			if (retry_times >= RETRY) {
+				printk( "\n" );
+				printk(KERN_ERR " stopping tasks failed (%d tasks remaining)\n", todo );
+				return todo;
+			}
+			retry_times ++;
+			start_time = jiffies;
+ 			freeze_processes_failure();
 		}
 	} while(todo);
 	
_



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

Re: Re: freeze_processes questions

[Nigel Cunningham]

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

Hi.

On Wed, 2005-04-27 at 19:30, Li Shaohua wrote:
> Hi,
> Sorry for raising the problem again after a long time. I got some
> reports about ' xxx not stopped' at suspend time. The general cause is
> one task is waiting for another task which has been into refrigerator
> first. Report an error and stop suspend can't solve the problem. After
> the task's dependent task is released from refrigerator, the task itself
> will soon go into refrigerator and will never be waked up. 
> Nigel's refrigerator patch half solves the problem. It distinct kernel
> tasks and user tasks, so can solve the dependence between kernel tasks
> and user tasks, but can't solve the user tasks dependence. Right, Nigel?

That's right, but the example you give below is userspace (syslogd) vs
kernel space (kjournald), so I wonder if something could be wrong with
your implementation.

> Below is a proposal method to solve the issue. Assume task A depends on
> task B. Task B is frozen first. When we find task A can't be frozen
> after a long time, we thaw all tasks and let task A continue (task A has
> gotten a freeze signal so it will soon go into refrigerator) and then we
> freeze other tasks again. In theory, this can solve all task dependence
> (sure can't solve the circled dependence) if the retry times are enough
> big, but cost maybe too high. Looks it help my case (syslogd can't be
> stopped because it's waitting for kjournald). Any idea?

Perhaps what is needful is the other component of my refrigerator
implementation. I also mark threads like kjournald with the new flag
PF_SYNCTHREAD, and temporarily give it to threads that enter sys_sync
and siblings. That ensures that processes submitting I/O are frozen,
then the I/O is flushed, then kernel threads such as kjournald are
frozen.

Regards,

Nigel
-- 
Nigel Cunningham
Software Engineer, Canberra, Australia
http://www.cyclades.com
Bus: +61 (2) 6291 9554; Hme: +61 (2) 6292 8028;  Mob: +61 (417) 100 574

Maintainer of Suspend2 Kernel Patches http://suspend2.net


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

Re: Re: freeze_processes questions

[Li Shaohua]

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

Hi,
On Wed, 2005-04-27 at 17:53, Nigel Cunningham wrote:
> > Sorry for raising the problem again after a long time. I got some
> > reports about ' xxx not stopped' at suspend time. The general cause
> is
> > one task is waiting for another task which has been into
> refrigerator
> > first. Report an error and stop suspend can't solve the problem.
> After
> > the task's dependent task is released from refrigerator, the task
> itself
> > will soon go into refrigerator and will never be waked up. 
> > Nigel's refrigerator patch half solves the problem. It distinct
> kernel
> > tasks and user tasks, so can solve the dependence between kernel
> tasks
> > and user tasks, but can't solve the user tasks dependence. Right,
> Nigel?
> 
> That's right, but the example you give below is userspace (syslogd) vs
> kernel space (kjournald), so I wonder if something could be wrong with
> your implementation.
Right, my example can be solved with your patch. But I'd like a more
generic solution to solve all the dependences, like user space tasks
dependencies.

Thanks,
Shaohua


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

Re: Re: freeze_processes questions

[Nigel Cunningham]

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

Hi.

On Thu, 2005-04-28 at 14:48, Li Shaohua wrote:
> Hi,
> On Wed, 2005-04-27 at 17:53, Nigel Cunningham wrote:
> > > Sorry for raising the problem again after a long time. I got some
> > > reports about ' xxx not stopped' at suspend time. The general cause
> > is
> > > one task is waiting for another task which has been into
> > refrigerator
> > > first. Report an error and stop suspend can't solve the problem.
> > After
> > > the task's dependent task is released from refrigerator, the task
> > itself
> > > will soon go into refrigerator and will never be waked up. 
> > > Nigel's refrigerator patch half solves the problem. It distinct
> > kernel
> > > tasks and user tasks, so can solve the dependence between kernel
> > tasks
> > > and user tasks, but can't solve the user tasks dependence. Right,
> > Nigel?
> > 
> > That's right, but the example you give below is userspace (syslogd) vs
> > kernel space (kjournald), so I wonder if something could be wrong with
> > your implementation.
> Right, my example can be solved with your patch. But I'd like a more
> generic solution to solve all the dependences, like user space tasks
> dependencies.

Do you have a particular example in mind? I haven't seen or heard of any
freezing failures in Suspend2 for ages, so I'm struggling to see what is
lacking.

Regards,

Nigel
-- 
Nigel Cunningham
Software Engineer, Canberra, Australia
http://www.cyclades.com
Bus: +61 (2) 6291 9554; Hme: +61 (2) 6292 8028;  Mob: +61 (417) 100 574

Maintainer of Suspend2 Kernel Patches http://suspend2.net


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

Re: Re: freeze_processes questions

[Li Shaohua]

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

On Thu, 2005-04-28 at 13:05, Nigel Cunningham wrote:
> > On Wed, 2005-04-27 at 17:53, Nigel Cunningham wrote:
> > > > Sorry for raising the problem again after a long time. I got
> some
> > > > reports about ' xxx not stopped' at suspend time. The general
> cause
> > > is
> > > > one task is waiting for another task which has been into
> > > refrigerator
> > > > first. Report an error and stop suspend can't solve the problem.
> > > After
> > > > the task's dependent task is released from refrigerator, the
> task
> > > itself
> > > > will soon go into refrigerator and will never be waked up. 
> > > > Nigel's refrigerator patch half solves the problem. It distinct
> > > kernel
> > > > tasks and user tasks, so can solve the dependence between kernel
> > > tasks
> > > > and user tasks, but can't solve the user tasks dependence.
> Right,
> > > Nigel?
> > > 
> > > That's right, but the example you give below is userspace
> (syslogd) vs
> > > kernel space (kjournald), so I wonder if something could be wrong
> with
> > > your implementation.
> > Right, my example can be solved with your patch. But I'd like a more
> > generic solution to solve all the dependences, like user space tasks
> > dependencies.
> 
> Do you have a particular example in mind? I haven't seen or heard of
> any
> freezing failures in Suspend2 for ages, so I'm struggling to see what
> is
> lacking.
No, actually. Maybe I should send the proposal till I really find a case
:). I only noticed the syslogd case. Somebody reported the
'mdnsresponder' process case, but didn't look at it yet.
http://bugme.osdl.org/show_bug.cgi?id=3964

Thanks,
Shaohua


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

Re: Re: freeze_processes questions

[Nigel Cunningham]

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

Hi.

On Thu, 2005-04-28 at 15:21, Li Shaohua wrote:
> > Do you have a particular example in mind? I haven't seen or heard of
> > any
> > freezing failures in Suspend2 for ages, so I'm struggling to see what
> > is
> > lacking.
> No, actually. Maybe I should send the proposal till I really find a case
> :). I only noticed the syslogd case. Somebody reported the
> 'mdnsresponder' process case, but didn't look at it yet.
> http://bugme.osdl.org/show_bug.cgi?id=3964

Ah. Ok. The ipw2100 driver starts a workqueue. Since I also have patches
to make kthreads freezable, we sometimes have to tell people to add an
extra parameter to the create_workqueue call. Once that's done, I
believe it all works fine.

I keep saying I must seek to merge these patches. I've gotten caught up
on trying to get everything ready to merge at once, and confused by what
Pavel is and isn't changing :> Pavel, since you're cc'd I might as well
ask, am I still right to submit those patches again? If so, I'll try to
do it this evening.

On a related topic, I've tried to get hotplug cpu support working with
suspending, but it looks like more is needed. In particular, the 'dead'
cpu still needs state saved and restored, so we're not really gaining
anything with the current implementation. If hotplug really killed the
CPU and rebooted it, the story might be different.

Regards,

Nigel
-- 
Nigel Cunningham
Software Engineer, Canberra, Australia
http://www.cyclades.com
Bus: +61 (2) 6291 9554; Hme: +61 (2) 6292 8028;  Mob: +61 (417) 100 574

Maintainer of Suspend2 Kernel Patches http://suspend2.net


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

Re: Re: freeze_processes questions

[Li Shaohua]

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

On Thu, 2005-04-28 at 13:49, Nigel Cunningham wrote:
> > > Do you have a particular example in mind? I haven't seen or heard
> of
> > > any
> > > freezing failures in Suspend2 for ages, so I'm struggling to see
> what
> > > is
> > > lacking.
> > No, actually. Maybe I should send the proposal till I really find a
> case
> > :). I only noticed the syslogd case. Somebody reported the
> > 'mdnsresponder' process case, but didn't look at it yet.
> > http://bugme.osdl.org/show_bug.cgi?id=3964
> 
> Ah. Ok. The ipw2100 driver starts a workqueue. Since I also have
> patches
> to make kthreads freezable, we sometimes have to tell people to add an
> extra parameter to the create_workqueue call. Once that's done, I
> believe it all works fine.
> 
> I keep saying I must seek to merge these patches. I've gotten caught
> up
> on trying to get everything ready to merge at once, and confused by
> what
> Pavel is and isn't changing :> Pavel, since you're cc'd I might as
> well
> ask, am I still right to submit those patches again? If so, I'll try
> to
> do it this evening.
> 
> On a related topic, I've tried to get hotplug cpu support working with
> suspending, but it looks like more is needed. In particular, the
> 'dead'
> cpu still needs state saved and restored, so we're not really gaining
> anything with the current implementation. If hotplug really killed the
> CPU and rebooted it, the story might be different.
What states should be saved/restored for the dead CPU? The dead CPU is
in idle thread before becoming dead, so ingoring save/restore general
registers doesn't matter. Sure there are something which must be
saved/restored, like the MTRR registers and possible Local APIC
registers if their value are different with the boot time. But there are
no much such cases, I think.

Thanks,
Shaohua


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

Re: Re: freeze_processes questions

[Nigel Cunningham]

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

Hi.

On Thu, 2005-04-28 at 16:01, Li Shaohua wrote:
> What states should be saved/restored for the dead CPU? The dead CPU is
> in idle thread before becoming dead, so ingoring save/restore general
> registers doesn't matter. Sure there are something which must be
> saved/restored, like the MTRR registers and possible Local APIC
> registers if their value are different with the boot time. But there are
> no much such cases, I think.

Well, I don't have it working perfectly yet, but saving and restoring
MTRRs definitely did make a difference. Prior to that I added the same
save and restore of CPU registers we use at the moment (suspend2 style)
and shifted the irq disable call above the wait loop (so the comment is
true). Given what you say, I'll try reversing the other changes and just
leaving in the MTRR save/restore, then see if it's as reliable as it is
now.

For the record, without anything added, I was getting spontaneous
reboots in the copy back with just hotplug. With everything I have now
added, I'm always making it to the end of the resume, but then getting
seg faults when I try to start apps. Should be simple to fix, but I
won't be trying this evening as I'm going to my last LUG meeting here in
Canberra. We move to Brisbane May 9.

Regards,

Nigel
-- 
Nigel Cunningham
Software Engineer, Canberra, Australia
http://www.cyclades.com
Bus: +61 (2) 6291 9554; Hme: +61 (2) 6292 8028;  Mob: +61 (417) 100 574

Maintainer of Suspend2 Kernel Patches http://suspend2.net


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

Re: Re: freeze_processes questions

[Pavel Machek]

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

Hi!
> > > Do you have a particular example in mind? I haven't seen or heard of
> > > any
> > > freezing failures in Suspend2 for ages, so I'm struggling to see what
> > > is
> > > lacking.
> > No, actually. Maybe I should send the proposal till I really find a case
> > :). I only noticed the syslogd case. Somebody reported the
> > 'mdnsresponder' process case, but didn't look at it yet.
> > http://bugme.osdl.org/show_bug.cgi?id=3964
> 
> Ah. Ok. The ipw2100 driver starts a workqueue. Since I also have patches
> to make kthreads freezable, we sometimes have to tell people to add an
> extra parameter to the create_workqueue call. Once that's done, I
> believe it all works fine.
> 
> I keep saying I must seek to merge these patches. I've gotten caught up
> on trying to get everything ready to merge at once, and confused by what
> Pavel is and isn't changing :> Pavel, since you're cc'd I might as well
> ask, am I still right to submit those patches again? If so, I'll try to
> do it this evening.

Yes, I'd like those patches...

> On a related topic, I've tried to get hotplug cpu support working with
> suspending, but it looks like more is needed. In particular, the 'dead'
> cpu still needs state saved and restored, so we're not really gaining
> anything with the current implementation. If hotplug really killed the
> CPU and rebooted it, the story might be different.

It works for me, but you need to play with -mm kernel where neccessary
support is implemented...
								Pavel
-- 
Boycott Kodak -- for their patent abuse against Java.

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

Re: Re: freeze_processes questions

[Pavel Machek]

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

Hi!

> > That's right, but the example you give below is userspace (syslogd) vs
> > kernel space (kjournald), so I wonder if something could be wrong with
> > your implementation.
> Right, my example can be solved with your patch. But I'd like a more
> generic solution to solve all the dependences, like user space tasks
> dependencies.

How can usespace processes depend on each other? By definition user
task sleeps in place where it holds no locks... it can't block anyone.

								Pavel
-- 
Boycott Kodak -- for their patent abuse against Java.

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

Re: freeze_processes questions

[Stefan Seyfried]

Pavel Machek wrote:
> Hi!

> How can usespace processes depend on each other? By definition user
> task sleeps in place where it holds no locks... it can't block anyone.

what was the problem with mysql some months back? I never fully
understood it, but wasn't it 2 mysql processes communicating via a pipe
or something like that and the "wrong end" frozen first?
-- 
Stefan Seyfried, QA / R&D Team Mobile Devices, SUSE LINUX Nürnberg.

"Any ideas, John?"
"Well, surrounding them's out."

Re: freeze_processes questions

[Pavel Machek]

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

Hi!

> > How can usespace processes depend on each other? By definition user
> > task sleeps in place where it holds no locks... it can't block anyone.
> 
> what was the problem with mysql some months back? I never fully
> understood it, but wasn't it 2 mysql processes communicating via a pipe
> or something like that and the "wrong end" frozen first?

I never fully understood it, either (it still exists in mainline,
IIRC), but it apparently was process calling signal functions in too
tight loop. It was reproducible with just one process.

								Pavel
-- 
Boycott Kodak -- for their patent abuse against Java.

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

Re: Re: freeze_processes questions

[Pavel Machek]

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

Hi!

> > > > OTOH... we may want to move completely away from refrigerator. Its
> > > > quite a hack, and it device support is okay, we'll not really need it.
> > > 
> > > Still, it won't happen soon, I guess. :-)  As of today, we have the
> > > refrigerator and the processes in TASK_UNINTERRUPTIBLE are mishandled.
> > > I think we should do something about it, at least for now, until we drop
> > > the refrigerator altogether (if we are going to drop it).  That's why I
> > > started the discussion and sent the patch.
> > 
> > It's very simple.  Processes in TASK_UNINTERRUPTIBLE can't be put in the
> > refrigerator, so you have to wait until they can be put there.
> 
> It seems that if a process calls wait_for_completion() right before suspend,
> the other task supposed to complete its completion may be accidentally frozen
> before it's able to do this.  It looks like this happens to kseroid sometimes
> on suspend-during-resume.

Hmm, so maybe rule needs to be "other tasks may not be waiting on you
using wait_for_completion when you enter refrigerator?
								Pavel
-- 
Boycott Kodak -- for their patent abuse against Java.

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

Re: Re: freeze_processes questions

[David Brownell]

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

On Tuesday 05 April 2005 4:10 pm, Nigel Cunningham wrote:
> 
> In this way, I handle kseriod and anything else uninterruptible without
> any problems.

This may be off-topic for this thread ... but does anyone
have system wakeup from PS/2 keyboard/mouse working?

Last time I tried to use it, it didn't work.  Not clear where
the problem was, but I suspect it was a case of ACPI support
not having anything to hook up to in the driver.

Hence the (potential) relevance on this thread:  the current
approach to suspend/resume in serio drivers needing updating,
which may interact with how kseriod is used.

- Dave

  

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

Re: Re: freeze_processes questions

[Nigel Cunningham]

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

Hi David.

On Thu, 2005-04-07 at 07:34, David Brownell wrote:
> On Tuesday 05 April 2005 4:10 pm, Nigel Cunningham wrote:
> > 
> > In this way, I handle kseriod and anything else uninterruptible without
> > any problems.
> 
> This may be off-topic for this thread ... but does anyone
> have system wakeup from PS/2 keyboard/mouse working?

Never tried it, I'm afraid. My keyboard has 'Power', 'Sleep' and 'Wake'
buttons, but they seem to be directly connected to /dev/null.

There's a /proc/acpi entry that controls the enabling wake events. Have
you tried fiddling with it?

Regards,

Nigel

> Last time I tried to use it, it didn't work.  Not clear where
> the problem was, but I suspect it was a case of ACPI support
> not having anything to hook up to in the driver.
> 
> Hence the (potential) relevance on this thread:  the current
> approach to suspend/resume in serio drivers needing updating,
> which may interact with how kseriod is used.
> 
> - Dave
> 
>   
-- 
Nigel Cunningham
Software Engineer, Canberra, Australia
http://www.cyclades.com
Bus: +61 (2) 6291 9554; Hme: +61 (2) 6292 8028;  Mob: +61 (417) 100 574

Maintainer of Suspend2 Kernel Patches http://suspend2.net


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

Re: Re: freeze_processes questions

[David Brownell]

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

On Wednesday 06 April 2005 3:03 pm, Nigel Cunningham wrote:
> Hi David.
> 
> On Thu, 2005-04-07 at 07:34, David Brownell wrote:
> > On Tuesday 05 April 2005 4:10 pm, Nigel Cunningham wrote:
> > > 
> > > In this way, I handle kseriod and anything else uninterruptible without
> > > any problems.
> > 
> > This may be off-topic for this thread ... but does anyone
> > have system wakeup from PS/2 keyboard/mouse working?
> 
> Never tried it, I'm afraid. My keyboard has 'Power', 'Sleep' and 'Wake'
> buttons, but they seem to be directly connected to /dev/null.
> 
> There's a /proc/acpi entry that controls the enabling wake events. Have
> you tried fiddling with it?

That's what didn't work.  (Combined with entering S1 then trying
to use the relevant buttons ... or for that matter, any buttons.)

- Dave


> Regards,
> 
> Nigel
> 
> > Last time I tried to use it, it didn't work.  Not clear where
> > the problem was, but I suspect it was a case of ACPI support
> > not having anything to hook up to in the driver.
> > 
> > Hence the (potential) relevance on this thread:  the current
> > approach to suspend/resume in serio drivers needing updating,
> > which may interact with how kseriod is used.
> > 
> > - Dave
> > 
> >   
> -- 
> Nigel Cunningham
> Software Engineer, Canberra, Australia
> http://www.cyclades.com
> Bus: +61 (2) 6291 9554; Hme: +61 (2) 6292 8028;  Mob: +61 (417) 100 574
> 
> Maintainer of Suspend2 Kernel Patches http://suspend2.net
> 
> 

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

Re: Re: freeze_processes questions

[Pavel Machek]

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

Hi!

> > In this way, I handle kseriod and anything else uninterruptible without
> > any problems.
> 
> This may be off-topic for this thread ... but does anyone
> have system wakeup from PS/2 keyboard/mouse working?

It worked for me from PS/2 keyboard... I did not set up anything on
linux side, I just enabled it with bios. It worked okay with both
ACPI/S3 and swsusp...
								Pavel
-- 
Boycott Kodak -- for their patent abuse against Java.

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

Re: Re: freeze_processes questions

[Pavel Machek]

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

Hi!

> > OTOH, if you want working refrigerator, Nigel has one ;-). We were
> > talking about merging it to mainline few times already...
> 
> Sorry. I keep getting preoccupied with other things. I'll get my act
> together :>

I thought "Rafael seems to have enough motivation to merge that one"
:-).
								Pavel
-- 
Boycott Kodak -- for their patent abuse against Java.

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

Re: Re: freeze_processes questions

[Nigel Cunningham]

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

Hi.

On Fri, 2005-04-08 at 16:23, Pavel Machek wrote:
> Hi!
> 
> > > OTOH, if you want working refrigerator, Nigel has one ;-). We were
> > > talking about merging it to mainline few times already...
> > 
> > Sorry. I keep getting preoccupied with other things. I'll get my act
> > together :>
> 
> I thought "Rafael seems to have enough motivation to merge that one"
> :-).

We've only been discussing part of the problem. I'll pick up the ball
and submit the whole workthreads/SYNCTHREAD/process.c-upgrade solution.

Regards,

Nigel
-- 
Nigel Cunningham
Software Engineer, Canberra, Australia
http://www.cyclades.com
Bus: +61 (2) 6291 9554; Hme: +61 (2) 6292 8028;  Mob: +61 (417) 100 574

Maintainer of Suspend2 Kernel Patches http://suspend2.net


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