Re: question on resume()

Re: question on resume()

[Rafael J. Wysocki]

[Added linux-pm to the Cc list, because I'm going to talk about things that
I know only from reading the code.]

On Tuesday, 30 January 2007 17:50, Oliver Neukum wrote:
> Am Dienstag, 30. Januar 2007 17:32 schrieb Rafael J. Wysocki:
> > However, you can always inspect the PF_FROZEN flag of the tasks in question
> > if that's practicable.
> 
> What would I do with that information? Ignore completion of IO?

I probably should say "that depends", but that wouldn't be very helpful.

Getting back to your initial question, which is if wake_up() may be called
from a driver's .resume() routine, I think the answer is no, it may not,
because in that case the "notified" tasks would be removed from the wait
queue, but the refrigerator() would (wrongly) restore their states as
TASK_UNINTERRUPTIBLE (or TASK_INTERRUPTIBLE for wake_up_interruptible()).

Generally, you are safe if your driver only calls wake_up() from a process
context, but not from .resume() or .suspend() routines (or from an
unfreezeable kernel thread).

Greetings,
Rafael


-- 
If you don't have the time to read,
you don't have the time or the tools to write.
		- Stephen King

Re: question on resume()

[Rafael J. Wysocki]

On Tuesday, 30 January 2007 23:32, Rafael J. Wysocki wrote:
> [Added linux-pm to the Cc list, because I'm going to talk about things that
> I know only from reading the code.]
> 
> On Tuesday, 30 January 2007 17:50, Oliver Neukum wrote:
> > Am Dienstag, 30. Januar 2007 17:32 schrieb Rafael J. Wysocki:
> > > However, you can always inspect the PF_FROZEN flag of the tasks in question
> > > if that's practicable.
> > 
> > What would I do with that information? Ignore completion of IO?
> 
> I probably should say "that depends", but that wouldn't be very helpful.
> 
> Getting back to your initial question, which is if wake_up() may be called
> from a driver's .resume() routine, I think the answer is no, it may not,
> because in that case the "notified" tasks would be removed from the wait
> queue, but the refrigerator() would (wrongly) restore their states as
> TASK_UNINTERRUPTIBLE (or TASK_INTERRUPTIBLE for wake_up_interruptible()).
> 
> Generally, you are safe if your driver only calls wake_up() from a process
> context, but not from .resume() or .suspend() routines (or from an
> unfreezeable kernel thread).

Ah, sorry, I've just realized I was wrong.  Processes in TASK_UNINTERRUPTIBLE
cannot be frozen!  So, the above only applies to wake_up_interruptible().

You don't need to call wake_up() from .resume(), because there are no tasks
to be notified this way and you shouldn't call wake_up_interruptible() from
there.

Greetings,
Rafael


-- 
If you don't have the time to read,
you don't have the time or the tools to write.
		- Stephen King

Re: question on resume()

[Oliver Neukum]

Am Mittwoch, 31. Januar 2007 09:33 schrieb Rafael J. Wysocki:
> On Tuesday, 30 January 2007 23:32, Rafael J. Wysocki wrote:

> > Generally, you are safe if your driver only calls wake_up() from a process
> > context, but not from .resume() or .suspend() routines (or from an
> > unfreezeable kernel thread).
> 
> Ah, sorry, I've just realized I was wrong.  Processes in TASK_UNINTERRUPTIBLE
> cannot be frozen!  So, the above only applies to wake_up_interruptible().

So the kernel will wait for tasks in TASK_UNINTERRUPTIBLE to finish IO
before it calls suspend()? I am confused.

	Regards
		Oliver

Re: question on resume()

[Rafael J. Wysocki]

On Wednesday, 31 January 2007 09:40, Oliver Neukum wrote:
> Am Mittwoch, 31. Januar 2007 09:33 schrieb Rafael J. Wysocki:
> > On Tuesday, 30 January 2007 23:32, Rafael J. Wysocki wrote:
> 
> > > Generally, you are safe if your driver only calls wake_up() from a process
> > > context, but not from .resume() or .suspend() routines (or from an
> > > unfreezeable kernel thread).
> > 
> > Ah, sorry, I've just realized I was wrong.  Processes in TASK_UNINTERRUPTIBLE
> > cannot be frozen!  So, the above only applies to wake_up_interruptible().
> 
> So the kernel will wait for tasks in TASK_UNINTERRUPTIBLE to finish IO
> before it calls suspend()? I am confused.

Yes, it will.  The process freezer can only return success if there are no more
TASK_UNINTERRUPTIBLE tasks.  Otherwise it fails (after a timeout).

Greetings,
Rafael


-- 
If you don't have the time to read,
you don't have the time or the tools to write.
		- Stephen King

Re: question on resume()

[Oliver Neukum]

Am Mittwoch, 31. Januar 2007 09:49 schrieb Rafael J. Wysocki:
> On Wednesday, 31 January 2007 09:40, Oliver Neukum wrote:
> > Am Mittwoch, 31. Januar 2007 09:33 schrieb Rafael J. Wysocki:
> > > On Tuesday, 30 January 2007 23:32, Rafael J. Wysocki wrote:
> > 
> > > > Generally, you are safe if your driver only calls wake_up() from a process
> > > > context, but not from .resume() or .suspend() routines (or from an
> > > > unfreezeable kernel thread).
> > > 
> > > Ah, sorry, I've just realized I was wrong.  Processes in TASK_UNINTERRUPTIBLE
> > > cannot be frozen!  So, the above only applies to wake_up_interruptible().
> > 
> > So the kernel will wait for tasks in TASK_UNINTERRUPTIBLE to finish IO
> > before it calls suspend()? I am confused.
> 
> Yes, it will.  The process freezer can only return success if there are no more
> TASK_UNINTERRUPTIBLE tasks.  Otherwise it fails (after a timeout).

So, this means, on suspend():

1. Don't worry about TASK_UNINTERRUPTIBLE
2. Do worry about TASK_INTERRUPTIBLE
We have to cease IO and must not call wake_up_interruptible()

Isn't that a race until suspend() is called?

On resume():

1. Don't worry about TASK_UNINTERRUPTIBLE
2. Do not restart IO that may call wake_up_interruptible()

When do we restart such IO?

	Regards
		Oliver

Re: question on resume()

[Pavel Machek]

Hi!

> > Yes, it will.  The process freezer can only return success if there are no more
> > TASK_UNINTERRUPTIBLE tasks.  Otherwise it fails (after a timeout).
> 
> So, this means, on suspend():
> 
> 1. Don't worry about TASK_UNINTERRUPTIBLE
> 2. Do worry about TASK_INTERRUPTIBLE
> We have to cease IO and must not call wake_up_interruptible()

"cease IO"? No, I believe it is enough not to start new I/O. Userspace
is frozen at that point, it can't ask you to do I/O.

> Isn't that a race until suspend() is called?

I do not think so.

> On resume():
> 
> 1. Don't worry about TASK_UNINTERRUPTIBLE
> 2. Do not restart IO that may call wake_up_interruptible()
> 
> When do we restart such IO?

We reuse signal handling code to do that for us. It is same situation
as when someone signals task doing I/O.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: question on resume()

[Oliver Neukum]

Am Mittwoch, 31. Januar 2007 10:36 schrieb Pavel Machek:
> Hi!
> 
> > > Yes, it will.  The process freezer can only return success if there are no more
> > > TASK_UNINTERRUPTIBLE tasks.  Otherwise it fails (after a timeout).
> > 
> > So, this means, on suspend():
> > 
> > 1. Don't worry about TASK_UNINTERRUPTIBLE
> > 2. Do worry about TASK_INTERRUPTIBLE
> > We have to cease IO and must not call wake_up_interruptible()
> 
> "cease IO"? No, I believe it is enough not to start new I/O. Userspace
> is frozen at that point, it can't ask you to do I/O.
>
> > Isn't that a race until suspend() is called?
> 
> I do not think so.

What about URBs in flight which are waited for with TASK_INTERRUPTIBLE?

> > On resume():
> > 
> > 1. Don't worry about TASK_UNINTERRUPTIBLE
> > 2. Do not restart IO that may call wake_up_interruptible()
> > 
> > When do we restart such IO?
> 
> We reuse signal handling code to do that for us. It is same situation
> as when someone signals task doing I/O.

What happens to tasks in TASK_INTERRUPTIBLE which are frozen?
Are they interrupted and frozen?

	Regards
		Oliver

Re: [linux-pm] question on resume()

[Pavel Machek]

Hi!

> > > So, this means, on suspend():
> > > 
> > > 1. Don't worry about TASK_UNINTERRUPTIBLE
> > > 2. Do worry about TASK_INTERRUPTIBLE
> > > We have to cease IO and must not call wake_up_interruptible()
> > 
> > "cease IO"? No, I believe it is enough not to start new I/O. Userspace
> > is frozen at that point, it can't ask you to do I/O.
> >
> > > Isn't that a race until suspend() is called?
> > 
> > I do not think so.
> 
> What about URBs in flight which are waited for with TASK_INTERRUPTIBLE?

Same thing as if someone sent a signal to that process. (We are
talking about user processes here, right? Kernel threads have to take
care themselves).

> > > On resume():
> > > 
> > > 1. Don't worry about TASK_UNINTERRUPTIBLE
> > > 2. Do not restart IO that may call wake_up_interruptible()
> > > 
> > > When do we restart such IO?
> > 
> > We reuse signal handling code to do that for us. It is same situation
> > as when someone signals task doing I/O.
> 
> What happens to tasks in TASK_INTERRUPTIBLE which are frozen?
> Are they interrupted and frozen?

Interrupted with fake signal, then frozen, yes.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: question on resume()

[Alan Stern]

On Wed, 31 Jan 2007, Pavel Machek wrote:

> Hi!
> 
> > > Yes, it will.  The process freezer can only return success if there are no more
> > > TASK_UNINTERRUPTIBLE tasks.  Otherwise it fails (after a timeout).
> > 
> > So, this means, on suspend():
> > 
> > 1. Don't worry about TASK_UNINTERRUPTIBLE
> > 2. Do worry about TASK_INTERRUPTIBLE
> > We have to cease IO and must not call wake_up_interruptible()
> 
> "cease IO"? No, I believe it is enough not to start new I/O. Userspace
> is frozen at that point, it can't ask you to do I/O.

There may be I/O requests sitting in a queue, already submitted by
userspace.  The suspend method should wait for existing I/O to complete
and stop processing new entries from the queue.

> > Isn't that a race until suspend() is called?
> 
> I do not think so.

The part about not calling wake_up_interruptible() is indeed a race.  We 
have:

	1. Task is frozen.
	2. Driver must not call wake_up_interruptible().
	3. Driver's suspend() method is called.

How is the driver supposed to satisfy (2) before (3) has occurred?

In fact this shouldn't matter.  There shouldn't be anything wrong with 
calling wake_up_interruptible() on a frozen task.

> > On resume():
> > 
> > 1. Don't worry about TASK_UNINTERRUPTIBLE
> > 2. Do not restart IO that may call wake_up_interruptible()
> > 
> > When do we restart such IO?
> 
> We reuse signal handling code to do that for us. It is same situation
> as when someone signals task doing I/O.

Again you misunderstood the question.  The driver must start queued I/O
when its resume() method is called.  It should then be okay for the driver
to call wake_up_interruptible(), even before tasks are unfrozen.

Alan Stern

Re: question on resume()

[Oliver Neukum]

Am Mittwoch, 31. Januar 2007 16:54 schrieb Alan Stern:
> On Wed, 31 Jan 2007, Pavel Machek wrote:

> > "cease IO"? No, I believe it is enough not to start new I/O. Userspace
> > is frozen at that point, it can't ask you to do I/O.
> 
> There may be I/O requests sitting in a queue, already submitted by
> userspace.  The suspend method should wait for existing I/O to complete
> and stop processing new entries from the queue.

As far as I understand it now, a frozen process will be in the refrigerator.
Thus it cannot be blocking somewhere else in kernel space. Yet we cannot
be sure there's no queued IO, as theres aio.


> > > On resume():
> > > 
> > > 1. Don't worry about TASK_UNINTERRUPTIBLE
> > > 2. Do not restart IO that may call wake_up_interruptible()
> > > 
> > > When do we restart such IO?
> > 
> > We reuse signal handling code to do that for us. It is same situation
> > as when someone signals task doing I/O.
> 
> Again you misunderstood the question.  The driver must start queued I/O
> when its resume() method is called.  It should then be okay for the driver
> to call wake_up_interruptible(), even before tasks are unfrozen.

Isn't there some code in usbfs that'll do homegrown aio and deliver a
signal to a process if io is completed?

	Regards
		Oliver

Re: question on resume()

[Alan Stern]

On Wed, 31 Jan 2007, Oliver Neukum wrote:

> Am Mittwoch, 31. Januar 2007 16:54 schrieb Alan Stern:
> > On Wed, 31 Jan 2007, Pavel Machek wrote:
> 
> > > "cease IO"? No, I believe it is enough not to start new I/O. Userspace
> > > is frozen at that point, it can't ask you to do I/O.
> > 
> > There may be I/O requests sitting in a queue, already submitted by
> > userspace.  The suspend method should wait for existing I/O to complete
> > and stop processing new entries from the queue.
> 
> As far as I understand it now, a frozen process will be in the refrigerator.
> Thus it cannot be blocking somewhere else in kernel space. Yet we cannot
> be sure there's no queued IO, as theres aio.

Or the driver may maintain its own I/O queue, like the HID driver does.

> > > > On resume():
> > > > 
> > > > 1. Don't worry about TASK_UNINTERRUPTIBLE
> > > > 2. Do not restart IO that may call wake_up_interruptible()
> > > > 
> > > > When do we restart such IO?
> > > 
> > > We reuse signal handling code to do that for us. It is same situation
> > > as when someone signals task doing I/O.
> > 
> > Again you misunderstood the question.  The driver must start queued I/O
> > when its resume() method is called.  It should then be okay for the driver
> > to call wake_up_interruptible(), even before tasks are unfrozen.
> 
> Isn't there some code in usbfs that'll do homegrown aio and deliver a
> signal to a process if io is completed?

That's right.  There's also code in the hub driver to call wake_up() on 
the khubd thread when certain I/O operations complete; nevertheless khubd 
_should_ remain frozen along with all the other tasks.

Alan Stern

Re: question on resume()

[Woodruff, Richard]

> Again you misunderstood the question.  The driver must start queued
I/O
> when its resume() method is called.  It should then be okay for the
driver
> to call wake_up_interruptible(), even before tasks are unfrozen.

I kind of like the way MontaVista worked around this in some 2.4 drivers
where no freezer is present.  A tiny amount of state is kept at the
driver and suspend lock outs are added at service entry points and at
thread wake up points inside of the driver.

This way if some process makes a request in or wakes up when the driver
is not ready for the action, that context will get re-slept.

With 2.6 on some of our variants for TI boards we still kept our suspend
lock outs even though the freezer was there.  It allows some interesting
run time idling.  You potentially can turn ON and OFF a driver
individually for something like a high latency operational mode.  I
suppose based on your comments it also works around issues in the
freezer.  It doesn't take all that many lock outs to shore up a driver.

Regards,
Richard W.

Re: question on resume()

[Alan Stern]

On Wed, 31 Jan 2007, Rafael J. Wysocki wrote:

> On Tuesday, 30 January 2007 23:32, Rafael J. Wysocki wrote:
> > [Added linux-pm to the Cc list, because I'm going to talk about things that
> > I know only from reading the code.]
> > 
> > On Tuesday, 30 January 2007 17:50, Oliver Neukum wrote:
> > > Am Dienstag, 30. Januar 2007 17:32 schrieb Rafael J. Wysocki:
> > > > However, you can always inspect the PF_FROZEN flag of the tasks in question
> > > > if that's practicable.
> > > 
> > > What would I do with that information? Ignore completion of IO?
> > 
> > I probably should say "that depends", but that wouldn't be very helpful.
> > 
> > Getting back to your initial question, which is if wake_up() may be called
> > from a driver's .resume() routine, I think the answer is no, it may not,
> > because in that case the "notified" tasks would be removed from the wait
> > queue, but the refrigerator() would (wrongly) restore their states as
> > TASK_UNINTERRUPTIBLE (or TASK_INTERRUPTIBLE for wake_up_interruptible()).

Even though I'm late to this thread, here are some additional thoughts...

Rafael is wrong; wake_up() doesn't remove a task from a wait queue.  It 
makes the task runnable, and then the task removes itself from the wait 
queue after verifying that the necessary condition has been satisfied.

Thus calling wake_up() on a task in the refrigerator will accomplish 
nothing -- no good and no harm.  The task will remain frozen, and when it 
is unfrozen it will realize that the condition has been satisfied and will 
remove itself from the wait queue.

> > Generally, you are safe if your driver only calls wake_up() from a process
> > context, but not from .resume() or .suspend() routines (or from an
> > unfreezeable kernel thread).
> 
> Ah, sorry, I've just realized I was wrong.  Processes in TASK_UNINTERRUPTIBLE
> cannot be frozen!  So, the above only applies to wake_up_interruptible().
> 
> You don't need to call wake_up() from .resume(), because there are no tasks
> to be notified this way and you shouldn't call wake_up_interruptible() from
> there.

While it's true that one doesn't need to call wake_up() from .resume(),
you are overlooking the point of Oliver's question.  .resume() can start
up an I/O operation which can then complete before the tasks are
defrosted.  The I/O's completion routine generally _will_ end up calling
wake_up() on some still-frozen task.  That's just as bad as calling it 
yourself from within the resume routine.

Alan Stern

Re: [linux-pm] question on resume()

[Rafael J. Wysocki]

On Wednesday, 31 January 2007 16:48, Alan Stern wrote:
> On Wed, 31 Jan 2007, Rafael J. Wysocki wrote:
> 
> > On Tuesday, 30 January 2007 23:32, Rafael J. Wysocki wrote:
> > > [Added linux-pm to the Cc list, because I'm going to talk about things that
> > > I know only from reading the code.]
> > > 
> > > On Tuesday, 30 January 2007 17:50, Oliver Neukum wrote:
> > > > Am Dienstag, 30. Januar 2007 17:32 schrieb Rafael J. Wysocki:
> > > > > However, you can always inspect the PF_FROZEN flag of the tasks in question
> > > > > if that's practicable.
> > > > 
> > > > What would I do with that information? Ignore completion of IO?
> > > 
> > > I probably should say "that depends", but that wouldn't be very helpful.
> > > 
> > > Getting back to your initial question, which is if wake_up() may be called
> > > from a driver's .resume() routine, I think the answer is no, it may not,
> > > because in that case the "notified" tasks would be removed from the wait
> > > queue, but the refrigerator() would (wrongly) restore their states as
> > > TASK_UNINTERRUPTIBLE (or TASK_INTERRUPTIBLE for wake_up_interruptible()).
> 
> Even though I'm late to this thread, here are some additional thoughts...
> 
> Rafael is wrong; wake_up() doesn't remove a task from a wait queue.  It 
> makes the task runnable, and then the task removes itself from the wait 
> queue after verifying that the necessary condition has been satisfied.
> 
> Thus calling wake_up() on a task in the refrigerator will accomplish 
> nothing -- no good and no harm.  The task will remain frozen, and when it 
> is unfrozen it will realize that the condition has been satisfied and will 
> remove itself from the wait queue.

That's the point I wasn't quite sure of.

> > > Generally, you are safe if your driver only calls wake_up() from a process
> > > context, but not from .resume() or .suspend() routines (or from an
> > > unfreezeable kernel thread).
> > 
> > Ah, sorry, I've just realized I was wrong.  Processes in TASK_UNINTERRUPTIBLE
> > cannot be frozen!  So, the above only applies to wake_up_interruptible().
> > 
> > You don't need to call wake_up() from .resume(), because there are no tasks
> > to be notified this way and you shouldn't call wake_up_interruptible() from
> > there.
> 
> While it's true that one doesn't need to call wake_up() from .resume(),
> you are overlooking the point of Oliver's question.  .resume() can start
> up an I/O operation which can then complete before the tasks are
> defrosted.  The I/O's completion routine generally _will_ end up calling
> wake_up() on some still-frozen task.  That's just as bad as calling it 
> yourself from within the resume routine.

Okay, but since the tasks remove themselves from wait queues, there's no
problem here. :-)

Greetings,
Rafael


-- 
If you don't have the time to read,
you don't have the time or the tools to write.
		- Stephen King