Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Miklos Szeredi]

> > I fear, that your efforts to "save" the freezer are in vain.  It is
> > already moderately hackish with that PF_FREEZER_SKIP and the kernel
> > dotted randomly with try_to_freeze() calls, but adding bandaids to try
> > to order freezing userspace processes in the right order would just
> > make it a horrible mess.
> 
> I agree that bandaids won't work.  What's needed is something more 
> radical.  Things like FUSE must be written so that the kernel parts 
> _can_ freeze even while they are waiting for a response from a user 
> thread.

This has already been discussed, with the conclusion, that it can't be
done without hacking VFS internals.

The basic problem is that the freezer tries to get every user process
out of the kernel even when those processes have _nothing_ to do with
drivers and could happily stay in kernel land across a suspend or even
hibernate.

If we could have a good grip on when a request is entering a driver,
it would be easy to take care of this.  I guess network and block
devices are easy.  For others there's no obvious common place where
such barriers could be placed so it's more work, but nothing
conceptually problematic.  Is this about right?

Miklos

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Oliver Neukum]

Am Donnerstag, 5. Juli 2007 schrieb Alan Stern:
> On Thu, 5 Jul 2007, Miklos Szeredi wrote:
> 
> > I fear, that your efforts to "save" the freezer are in vain.  It is
> > already moderately hackish with that PF_FREEZER_SKIP and the kernel
> > dotted randomly with try_to_freeze() calls, but adding bandaids to try
> > to order freezing userspace processes in the right order would just
> > make it a horrible mess.
> 
> I agree that bandaids won't work.  What's needed is something more 
> radical.  Things like FUSE must be written so that the kernel parts 
> _can_ freeze even while they are waiting for a response from a user 
> thread.

OK, some radical ideas.

In principle we want a deadlock here. Tasks that are frozen due to fuse
are as good as frozen, there's no need to formally freeze them.
The bad uninterruptible tasks are those waiting for hardware.

Can we detect the difference? Perhaps we can label those crucial locks
and when only tasks in the refrigerator and tasks waiting on these locks
are left we are content and go on to suspending the device tree.

	Flame away
		Oliver

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Rafael J. Wysocki]

On Thursday, 5 July 2007 23:37, Oliver Neukum wrote:
> Am Donnerstag, 5. Juli 2007 schrieb Alan Stern:
> > On Thu, 5 Jul 2007, Miklos Szeredi wrote:
> > 
> > > I fear, that your efforts to "save" the freezer are in vain.  It is
> > > already moderately hackish with that PF_FREEZER_SKIP and the kernel
> > > dotted randomly with try_to_freeze() calls, but adding bandaids to try
> > > to order freezing userspace processes in the right order would just
> > > make it a horrible mess.
> > 
> > I agree that bandaids won't work.  What's needed is something more 
> > radical.  Things like FUSE must be written so that the kernel parts 
> > _can_ freeze even while they are waiting for a response from a user 
> > thread.
> 
> OK, some radical ideas.
> 
> In principle we want a deadlock here. Tasks that are frozen due to fuse
> are as good as frozen, there's no need to formally freeze them.
> The bad uninterruptible tasks are those waiting for hardware.
> 
> Can we detect the difference? Perhaps we can label those crucial locks
> and when only tasks in the refrigerator and tasks waiting on these locks
> are left we are content and go on to suspending the device tree.

Well, I think we can just make the freezer handle uninterruptible tasks.

If an uninterruptible task is stuck holding a lock that's needed for suspend,
the freezerless suspend will deadlock anyway. :-)

The only reason (I know of) why we don't handle uninterruptible tasks in the
freezer is that we're afraid of the suspend process deadlocking with an
uninterruptible task holding a lock, but AFAICS the probability of such an
event is extremely small.

Also, the powermac suspend will deadlock in such cases, so the fact that
it doesn't deadlock means that they don't occur very often (if at all).

I have a patch for that, will post it in a separate thread in a while.

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Benjamin Herrenschmidt]

On Fri, 2007-07-06 at 09:13 +0200, Rafael J. Wysocki wrote:
> 
> The only reason (I know of) why we don't handle uninterruptible tasks in the
> freezer is that we're afraid of the suspend process deadlocking with an
> uninterruptible task holding a lock, but AFAICS the probability of such an
> event is extremely small.

What would deadlock specifically ? One of the drivers trying to acquire
that lock ? It would be a driver bug then.

> Also, the powermac suspend will deadlock in such cases, so the fact that
> it doesn't deadlock means that they don't occur very often (if at all).

Ben.

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Oliver Neukum]

Am Freitag, 6. Juli 2007 schrieb Benjamin Herrenschmidt:
> On Fri, 2007-07-06 at 09:13 +0200, Rafael J. Wysocki wrote:
> > 
> > The only reason (I know of) why we don't handle uninterruptible tasks in the
> > freezer is that we're afraid of the suspend process deadlocking with an
> > uninterruptible task holding a lock, but AFAICS the probability of such an
> > event is extremely small.
> 
> What would deadlock specifically ? One of the drivers trying to acquire
> that lock ? It would be a driver bug then.

Your driver's write method looks like:

mutex_lock();
poke_some_hardware();
wait_event_uninterruptible(); //for result
res = evaluate_result();
mutex_unlock();
return res;

If you put a task into the refrigerator at wait_event_interruptible()
you will deadlock if you need this lock for the driver to go to suspend.
The suspend method then must not take the lock _and_ it must be
aware that there may be an ongoing operation.

	Regards
		Oliver

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Rafael J. Wysocki]

On Friday, 6 July 2007 11:31, Oliver Neukum wrote:
> Am Freitag, 6. Juli 2007 schrieb Benjamin Herrenschmidt:
> > On Fri, 2007-07-06 at 09:13 +0200, Rafael J. Wysocki wrote:
> > > 
> > > The only reason (I know of) why we don't handle uninterruptible tasks in the
> > > freezer is that we're afraid of the suspend process deadlocking with an
> > > uninterruptible task holding a lock, but AFAICS the probability of such an
> > > event is extremely small.
> > 
> > What would deadlock specifically ? One of the drivers trying to acquire
> > that lock ? It would be a driver bug then.
> 
> Your driver's write method looks like:
> 
> mutex_lock();
> poke_some_hardware();
> wait_event_uninterruptible(); //for result
> res = evaluate_result();
> mutex_unlock();
> return res;
> 
> If you put a task into the refrigerator at wait_event_interruptible()
> you will deadlock if you need this lock for the driver to go to suspend.
> The suspend method then must not take the lock _and_ it must be
> aware that there may be an ongoing operation.

s/interruptible/uninterruptible/

> you will deadlock if you need this lock for the driver to go to suspend.
> The suspend method then must not take the lock _and_ it must be
> aware that there may be an ongoing operation.

Well, is there any driver in the tree that works like that _and_ has a
.suspend() method requiring the same lock?

Besides, I'm not going to put the task into the refrigerator at that point.

Please read http://lkml.org/lkml/2007/7/6/71

Moreover, I claim that, in the context of your example, _if_ the task is stuck
at the wait_event_uninterruptible(), _then_ the freezerless suspend will
deadlock with the task.

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Benjamin Herrenschmidt]

On Fri, 2007-07-06 at 11:31 +0200, Oliver Neukum wrote:
> Am Freitag, 6. Juli 2007 schrieb Benjamin Herrenschmidt:
> > On Fri, 2007-07-06 at 09:13 +0200, Rafael J. Wysocki wrote:
> > > 
> > > The only reason (I know of) why we don't handle uninterruptible tasks in the
> > > freezer is that we're afraid of the suspend process deadlocking with an
> > > uninterruptible task holding a lock, but AFAICS the probability of such an
> > > event is extremely small.
> > 
> > What would deadlock specifically ? One of the drivers trying to acquire
> > that lock ? It would be a driver bug then.
> 
> Your driver's write method looks like:
> 
> mutex_lock();
> poke_some_hardware();
> wait_event_uninterruptible(); //for result
> res = evaluate_result();
> mutex_unlock();
> return res;
> 
> If you put a task into the refrigerator at wait_event_interruptible()
> you will deadlock if you need this lock for the driver to go to suspend.
> The suspend method then must not take the lock _and_ it must be
> aware that there may be an ongoing operation.

Well... 2 things here. Either you have a freezer in which case the
chances of the above scenario are increased, or you don't, in which case
your suspend method will just sleep on the lock until outstanding HW
accesses that have that lock are completed, and everything is fine.

You need to be careful with one thing though, whether you have a freezer
or not. If you driver, in some code path, whatever it is (ioctl, kernel
thread, workqueue, ...) does something like:

mutex_lock
kmalloc(...,GFP_KERNEL);
mutex_unlock

And it's suspend callback then does:

mutex_lock

The problem here is that the disks might already have been suspended
prior to your driver being called. Thus, any attempt at pushing things
out to swap or dirty mmap'ings back to storage will hang, thus kmalloc
can potentially hang (afaik), and you will deadlock.

That's what I've been talking about earlier when I said that we should
have some security in SLAB/SLUB/Buddy allocators, to silently turn
GFP_KERNEL to at least GFP_NOIO or even ATOMIC before we start
suspending drivers.

Now, another way to deal with that would have to use
pre-suspend/post-resume notifications, and have drivers avoid doing the
above between those, but that's much harder. (Essentially, drivers would
have to either make sure they don't do things like blocking allocations,
even implicitely, or possibly fall back to a degraded synchronous mode
or that sort of thing).

I think it's much simpler to tweak slab/slub/buddy instead :-)

Note that the above issue is orthogonal to our freezer discussion, it's
just one of the potential deadlock cause we have with suspend that needs
to be fixed.

Cheers,
Ben.

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Rafael J. Wysocki]

On Saturday, 7 July 2007 04:44, Benjamin Herrenschmidt wrote:
> On Fri, 2007-07-06 at 11:31 +0200, Oliver Neukum wrote:
> > Am Freitag, 6. Juli 2007 schrieb Benjamin Herrenschmidt:
> > > On Fri, 2007-07-06 at 09:13 +0200, Rafael J. Wysocki wrote:
> > > > 
> > > > The only reason (I know of) why we don't handle uninterruptible tasks in the
> > > > freezer is that we're afraid of the suspend process deadlocking with an
> > > > uninterruptible task holding a lock, but AFAICS the probability of such an
> > > > event is extremely small.
> > > 
> > > What would deadlock specifically ? One of the drivers trying to acquire
> > > that lock ? It would be a driver bug then.
> > 
> > Your driver's write method looks like:
> > 
> > mutex_lock();
> > poke_some_hardware();
> > wait_event_uninterruptible(); //for result
> > res = evaluate_result();
> > mutex_unlock();
> > return res;
> > 
> > If you put a task into the refrigerator at wait_event_interruptible()
> > you will deadlock if you need this lock for the driver to go to suspend.
> > The suspend method then must not take the lock _and_ it must be
> > aware that there may be an ongoing operation.
> 
> Well... 2 things here. Either you have a freezer in which case the
> chances of the above scenario are increased,

How so? :-)

> or you don't, in which case 
> your suspend method will just sleep on the lock until outstanding HW
> accesses that have that lock are completed, and everything is fine.
> 
> You need to be careful with one thing though, whether you have a freezer
> or not. If you driver, in some code path, whatever it is (ioctl, kernel
> thread, workqueue, ...) does something like:
> 
> mutex_lock
> kmalloc(...,GFP_KERNEL);
> mutex_unlock
> 
> And it's suspend callback then does:
> 
> mutex_lock
> 
> The problem here is that the disks might already have been suspended
> prior to your driver being called. Thus, any attempt at pushing things
> out to swap or dirty mmap'ings back to storage will hang, thus kmalloc
> can potentially hang (afaik), and you will deadlock.
> 
> That's what I've been talking about earlier when I said that we should
> have some security in SLAB/SLUB/Buddy allocators, to silently turn
> GFP_KERNEL to at least GFP_NOIO or even ATOMIC before we start
> suspending drivers.
> 
> Now, another way to deal with that would have to use
> pre-suspend/post-resume notifications, and have drivers avoid doing the
> above between those, but that's much harder. (Essentially, drivers would
> have to either make sure they don't do things like blocking allocations,
> even implicitely, or possibly fall back to a degraded synchronous mode
> or that sort of thing).
> 
> I think it's much simpler to tweak slab/slub/buddy instead :-)
> 
> Note that the above issue is orthogonal to our freezer discussion, it's
> just one of the potential deadlock cause we have with suspend that needs
> to be fixed.

Agreed.

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Benjamin Herrenschmidt]

> > Well... 2 things here. Either you have a freezer in which case the
> > chances of the above scenario are increased,
> 
> How so? :-)

I meant you have a freezer that freezes uninterruptible tasks.

Ben.

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Benjamin Herrenschmidt]

On Fri, 2007-07-06 at 11:53 +0200, Rafael J. Wysocki wrote:
> 
> Moreover, I claim that, in the context of your example, _if_ the task
> is stuck
> at the wait_event_uninterruptible(), _then_ the freezerless suspend
> will
> deadlock with the task.

Why would the task be stuck there if it's not becasue of a freezer ? The
only reason I see would be a dependency on something like kmalloc trying
to push things to disk, or a lock owned by another user process, but the
former is a generic issue I've discussed in a separate mail and the
later is a driver bug imho.

Ben.

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Benjamin Herrenschmidt]

On Thu, 2007-07-05 at 10:23 -0400, Alan Stern wrote:
> 
> How will that help?  Block the kernel thread in the freezer or block it 
> in the driver -- either way it is blocked.  So how do your deadlocks 
> get resolved?

Because nobody is waiting on that kernel thread anyway without a freezer
so there is no deadlock anymore.

> I disagree with your analysis -- not that it's completely wrong, but it 
> points out an existing basic problem in the kernel.  The kernel should 
> never depend on userspace!  More correctly, a task executing in the 
> kernel should never block with any sort of mutex or other lock held (in 
> a way that would preclude it from being frozen, let's say) while 
> waiting for a response from userspace.
> 
> Then the dependency graph would be easy to construct: User tasks can
> depend on whatever they want, and kernel threads never depend on a user
> task.

In an idea world, there would be no hunger...

> If this contradicts the existing implementations and APIs for userspace 
> filesystems, then so be it.  My conclusion would be that the 
> implementations and APIs should be changed.

Why are you guys working so hard and spending so much energy to try to
avoid doing the right thing is beyond my understanding...

> It _does_ apply to kernel threads.  That's exactly why I wrote above 
> that kernel threads which try to do I/O during a suspend will need 
> extra attention.

Ok none at all if you don't have a freezer.

Ben.

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Rafael J. Wysocki]

On Friday, 6 July 2007 00:59, Benjamin Herrenschmidt wrote:
> On Thu, 2007-07-05 at 10:23 -0400, Alan Stern wrote:
> > 
> > How will that help?  Block the kernel thread in the freezer or block it 
> > in the driver -- either way it is blocked.  So how do your deadlocks 
> > get resolved?
> 
> Because nobody is waiting on that kernel thread anyway without a freezer
> so there is no deadlock anymore.

I'm not sure what you mean.  The freezer doesn't wait for threads that are
already frozen ...

> > I disagree with your analysis -- not that it's completely wrong, but it 
> > points out an existing basic problem in the kernel.  The kernel should 
> > never depend on userspace!  More correctly, a task executing in the 
> > kernel should never block with any sort of mutex or other lock held (in 
> > a way that would preclude it from being frozen, let's say) while 
> > waiting for a response from userspace.
> > 
> > Then the dependency graph would be easy to construct: User tasks can
> > depend on whatever they want, and kernel threads never depend on a user
> > task.
> 
> In an idea world, there would be no hunger...
> 
> > If this contradicts the existing implementations and APIs for userspace 
> > filesystems, then so be it.  My conclusion would be that the 
> > implementations and APIs should be changed.
> 
> Why are you guys working so hard and spending so much energy to try to
> avoid doing the right thing is beyond my understanding...
> 
> > It _does_ apply to kernel threads.  That's exactly why I wrote above 
> > that kernel threads which try to do I/O during a suspend will need 
> > extra attention.
> 
> Ok none at all if you don't have a freezer.

Provided that drivers can handle that.

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Alan Stern]

On Fri, 6 Jul 2007, Benjamin Herrenschmidt wrote:

> Why are you guys working so hard and spending so much energy to try to
> avoid doing the right thing is beyond my understanding...
> 
> > It _does_ apply to kernel threads.  That's exactly why I wrote above 
> > that kernel threads which try to do I/O during a suspend will need 
> > extra attention.
> 
> Ok none at all if you don't have a freezer.

In answer to both questions: We need the freezer in order to implement 
hibernate.  Even if we take your advice and stop using the freezer 
during suspend, these issues would still remain and would need to be 
solved.

Alan Stern

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Pavel Machek]

Hi!

> > How will that help?  Block the kernel thread in the freezer or block it 
> > in the driver -- either way it is blocked.  So how do your deadlocks 
> > get resolved?
> 
> Because nobody is waiting on that kernel thread anyway without a freezer
> so there is no deadlock anymore.

In the deadlock we are seeing, _someone_ is waiting on userspace
thread, that leads to deadlock with freezer. We don't know who,
because we have not seen the sysrq-t dumps.

The "unknown who" will deadlock on fused frozen by driver, too. We
really need to fix the "unknown who" here.
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Paul Mackerras]

Alan Stern writes:

> In answer to both questions: We need the freezer in order to implement 
> hibernate.  Even if we take your advice and stop using the freezer 
> during suspend, these issues would still remain and would need to be 
> solved.

Stepping back for a minute, let's think about what the freezer is
trying to achieve.  I think that currently there are three basic
design goals:

A. Ensure that device drivers don't get I/O requests after being
   suspended.

B. Ensure that driver suspend routines don't end up blocking forever
   on mutexes or semaphores held by frozen tasks.

C. Provide a way to get an atomic snapshot of memory for hibernation.

Now, it's easy enough to freeze all processes (or all except one), if
you don't have goal B.  Just offline non-boot cpus and disable
interrupts, or use stop_machine().  But goal B implies that you can't
necessarily just stop all tasks wherever they are.  In fact it means
there are points where it is safe to stop a given task, and there may
be points where it isn't safe to stop it - and there is no practical
way to determine those points reliably.[1]

That implies to me that we can have a freezer as long as we do nothing
that can sleep, while tasks are frozen.  In other words, I think the
freezer is a viable option for hibernation as long as we restrict
driver hibernate routines to doing only things which don't sleep.  I
_think_ that should be doable since hibernate routines only need to
wait for outstanding DMAs to complete, as I understand it, which can
be done by polling.

Paul.

[1] For a start, there's no way to determine which mutexes a task
holds.  Even if there were, we would then also have to know which
mutexes it is going to try to acquire before it releases the one we
want, which is pretty much unknowable.

One can say "we'll only freeze kernel tasks that ask to be frozen" but
then one has no way to guarantee A, and we still don't reliably
guarantee B if we have user-level filesystems or device drivers.

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Oliver Neukum]

Am Donnerstag, 5. Juli 2007 schrieb Alan Stern:
> On Thu, 5 Jul 2007, Oliver Neukum wrote:
> 
> > > Obviously.  But I wasn't about the server trying to acquire a lock
> > > held by a client.  I was talking about a client trying to acquire a
> > > lock held by _another_ client.
> > > 
> > > If this coincides with the server (or some other task which the server
> > > is depending on) being frozen before the clients, the freezer has a
> > > problem.
> > 
> > True, but that case can only happen if servers are frozen before clients.
> > You don't need a full dependency graph. A simple set sequence of two
> > classes of tasks will do.
> 
> Just to make things more complicated...  Since a server isn't
> restricted in what it can do, what happens when one server depends on
> another server?

The same principle applies. If you really want that you can solve this
by freezing servers in the reverse sequence they were started.

The main point remains. If you have a circular dependency anywhere
among the servers you can deadlock independent of the freezer.

	Regards
		Oliver

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Miklos Szeredi]

> > On Thu, 5 Jul 2007, Oliver Neukum wrote:
> > 
> > > > Obviously.  But I wasn't about the server trying to acquire a lock
> > > > held by a client.  I was talking about a client trying to acquire a
> > > > lock held by _another_ client.
> > > > 
> > > > If this coincides with the server (or some other task which the server
> > > > is depending on) being frozen before the clients, the freezer has a
> > > > problem.
> > > 
> > > True, but that case can only happen if servers are frozen before clients.
> > > You don't need a full dependency graph. A simple set sequence of two
> > > classes of tasks will do.
> > 
> > Just to make things more complicated...  Since a server isn't
> > restricted in what it can do, what happens when one server depends on
> > another server?
> 
> The same principle applies. If you really want that you can solve this
> by freezing servers in the reverse sequence they were started.

You just can't know what constitutes a "server", processes which read
from the fuse device are candidates, but all tasks which communicate
with these in some way are also.  And that basically makes every
userspace task in the system a candidate server and you are no further
than before.

> The main point remains. If you have a circular dependency anywhere
> among the servers you can deadlock independent of the freezer.

Well, Duh.

Miklos

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Alan Stern]

On Thu, 5 Jul 2007, Miklos Szeredi wrote:

> I fear, that your efforts to "save" the freezer are in vain.  It is
> already moderately hackish with that PF_FREEZER_SKIP and the kernel
> dotted randomly with try_to_freeze() calls, but adding bandaids to try
> to order freezing userspace processes in the right order would just
> make it a horrible mess.

I agree that bandaids won't work.  What's needed is something more 
radical.  Things like FUSE must be written so that the kernel parts 
_can_ freeze even while they are waiting for a response from a user 
thread.

Alan Stern

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Alan Stern]

On Wed, 4 Jul 2007, Paul Mackerras wrote:

> Whether or not to resume a suspended device when an I/O request comes
> in is a policy decision, and there could be cases where the user wants
> I/O requests to be blocked, or to fail, or to be dropped while the
> device is suspended, even for runtime power management.  For example,
> a sound card could be suspended due to a low-battery condition, and in
> that case you would want the driver to just drop any data that
> userspace tries to write to the soundcard.

We have provisions for that (my earlier description was somewhat 
incomplete).

> > Yes, the code could be changed to keep track of the reason for a device
> > suspend.  But that just raises the old problem of what to do when
> > there's an I/O request for a suspended device during STR.
> 
> Is this actually a real problem?  I would think the policy would be
> "block" for block devices (pun not intended :), "drop" for network
> devices, etc.

It is indeed a real problem, or at least, it can be.

> > Consider a particularly troublesome case: During STR, a non-frozen task
> > writes to /sys/bus/BBB/drivers/DDD/bind.  The sysfs core grabs the
> > device semaphore and calls the driver's probe routine.  If the driver
> > isn't PM-aware it simply tries to initialize the device and fails
> > because the device is already suspended.  That's no good; it isn't
> > transparent.
> 
> How did the device get suspended if it didn't have a driver?  If it
> did have a driver, why didn't the bind attempt fail?

Bus subsystems can suspend devices with no drivers.

> Suppose the device-model core code simply blocked all bind and unbind
> requests while suspend is under way, until resume is finished.
> Wouldn't that solve the problem?

It would help.  It would help even more if the sysfs core also blocked
all I/O while suspend is under way.  (Although this might be tricky, 
considering that the suspend is initiated by a sysfs write...)

The fact remains that lots of drivers would still need to be changed.  
In the read and write methods someone would have to add code amounting
to this:

	if (suspend_is_under_way()) {
		mutex_unlock(...);
		block_until_resume();
		goto restart;
	}

Freezing userspace is a small amount of code by comparison.

Alan Stern

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Paul Mackerras]

Alan Stern writes:

> > > Yes, the code could be changed to keep track of the reason for a device
> > > suspend.  But that just raises the old problem of what to do when
> > > there's an I/O request for a suspended device during STR.
> > 
> > Is this actually a real problem?  I would think the policy would be
> > "block" for block devices (pun not intended :), "drop" for network
> > devices, etc.
> 
> It is indeed a real problem, or at least, it can be.

How so?  Can you give me an example?

> > How did the device get suspended if it didn't have a driver?  If it
> > did have a driver, why didn't the bind attempt fail?
> 
> Bus subsystems can suspend devices with no drivers.

Interesting.  I assume this is for buses for which there is a
bus-specific but device-independent suspend procedure defined.

It would seem sensible to me that the PM core should get the bus to
resume such a device before calling a driver probe routine.  The
resume should be blocked or deferred while a system suspend is
underway.  In fact I think that all driver bind/unbind and probe
operations should be deferred while the system is suspending (i.e. put
on a list to be done after the system resumes).

> It would help.  It would help even more if the sysfs core also blocked
> all I/O while suspend is under way.  (Although this might be tricky, 
> considering that the suspend is initiated by a sysfs write...)

I didn't think sysfs got involved at all in normal read and write
requests, so I don't know how it would block them...

> The fact remains that lots of drivers would still need to be changed.  
> In the read and write methods someone would have to add code amounting
> to this:
> 
> 	if (suspend_is_under_way()) {
> 		mutex_unlock(...);
> 		block_until_resume();
> 		goto restart;
> 	}
> 
> Freezing userspace is a small amount of code by comparison.

Normally devices have some sort of queue of pending operations.  So
all that is required on suspend is to stop processing the queue and
wait for any currently-underway operations to complete.  The blocking
then happens naturally using the normal I/O wait mechanisms.

Paul.

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Alan Stern]

On Thu, 5 Jul 2007, Paul Mackerras wrote:

> Alan Stern writes:
> 
> > > > Yes, the code could be changed to keep track of the reason for a device
> > > > suspend.  But that just raises the old problem of what to do when
> > > > there's an I/O request for a suspended device during STR.
> > > 
> > > Is this actually a real problem?  I would think the policy would be
> > > "block" for block devices (pun not intended :), "drop" for network
> > > devices, etc.
> > 
> > It is indeed a real problem, or at least, it can be.
> 
> How so?  Can you give me an example?

The example I quoted earlier about binding during a suspend will do.  I 
agree that we can and should try to prevent it from ever occurring.

Read and write are a problem only in that fixing them would potentially
involve changing lots of drivers; I don't think they pose a serious
theoretical obstacle.  (Lord knows what will happen with async I/O!)

Any other entry points to drivers are also potential problems, but it's 
hard to say anything definite about them since they are so varied.

> > Bus subsystems can suspend devices with no drivers.
> 
> Interesting.  I assume this is for buses for which there is a
> bus-specific but device-independent suspend procedure defined.

Yes.

> It would seem sensible to me that the PM core should get the bus to
> resume such a device before calling a driver probe routine.  The
> resume should be blocked or deferred while a system suspend is
> underway.  In fact I think that all driver bind/unbind and probe
> operations should be deferred while the system is suspending (i.e. put
> on a list to be done after the system resumes).

Getting the PM core to resume a device before probing could be 
difficult; in general it doesn't know enough about specific device 
behaviors to do something like that.  But the subsystem certainly ought 
to take care of it.  USB does.

Yes, bind/unbind/etc. should be deferred during a system suspend.  But
it has to be done carefully, because these operations generally involve
locks that can't be released.  They need to be prevented at their
source, not in the driver core.  That's one reason why khubd needs to
be frozen (being part of the USB hub driver, it is the task responsible
for binding and unbinding drivers to USB devices).

Another thing to look out for is registration and unregistration of 
drivers.  These activities also cause bind/unbind operations.  Note 
that if userspace is frozen then neither insmod nor rmmod can run.  :-)

> > It would help.  It would help even more if the sysfs core also blocked
> > all I/O while suspend is under way.  (Although this might be tricky, 
> > considering that the suspend is initiated by a sysfs write...)
> 
> I didn't think sysfs got involved at all in normal read and write
> requests, so I don't know how it would block them...

All I/O to sysfs attributes passes through the routines in fs/sysfs/*.  
It could be blocked there.  (But if userspace is frozen it won't need 
to be.)

> Normally devices have some sort of queue of pending operations.

That's certainly true of block devices, whose drivers use the block 
subsystem.  It's not true for lots of other devices, though.

>  So
> all that is required on suspend is to stop processing the queue and
> wait for any currently-underway operations to complete.  The blocking
> then happens naturally using the normal I/O wait mechanisms.

In my experience, most non-block drivers do not have any queue of 
pending I/O operations.  They simply carry out requests as they arrive.

Alan Stern

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Alan Stern]

On Wed, 4 Jul 2007, Paul Mackerras wrote:

> Alan Stern writes:
> 
> > > Most drivers suspended their hardware in the second call.  If they are
> > > in the middle of a conversation with their device that *has* to be
> > > completed, they can do that by polling.
> > 
> > Ugh.  That will cause problems when you try to integrate runtime 
> > suspend.  In fact this whole approach is unsuitable for runtime PM and 
> > it obscures the similarities between runtime PM and STR.
> 
> Yes there are similarities, but it would be a big mistake to say that
> a requirement for STR is that all drivers do runtime PM.

That's not what I'm saying.  What I'm saying is that it would be a big 
mistake to force all drivers which implement runtime PM to do it using 
a separate code path from system PM.

> The main attraction of the late-suspend call is that it really does,
> reliably, guarantee that the driver's I/O request methods won't get
> called between the late-suspend call and the early-resume call.

For some drivers (like USB), carrying out an actual suspend requires a
delay.  Right now we implement those delays using wait_event(),
wait_for_completion(), and so on.  Would you have us check at runtime
whether or not a system suspend is underway and in each case use a
busy-loop instead if it is?

What happens if, in order to carry out the late-suspend, a driver needs
to acquire a mutex which happens to be held by some other task?  That
other task won't be able to run and release the mutex, so you will
deadlock.

Alan Stern

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Paul Mackerras]

Alan Stern writes:

> That's not what I'm saying.  What I'm saying is that it would be a big 
> mistake to force all drivers which implement runtime PM to do it using 
> a separate code path from system PM.

OK; I can accept that provided there is a way to change the "what to
do with an I/O request" policy from auto-resume to something else
while we're suspending the system (and presumably restore the old
policy on system resume if the device was runtime-suspended at the
point where the system was suspended).

> > The main attraction of the late-suspend call is that it really does,
> > reliably, guarantee that the driver's I/O request methods won't get
> > called between the late-suspend call and the early-resume call.
> 
> For some drivers (like USB), carrying out an actual suspend requires a
> delay.  Right now we implement those delays using wait_event(),
> wait_for_completion(), and so on.  Would you have us check at runtime
> whether or not a system suspend is underway and in each case use a
> busy-loop instead if it is?

No; the late suspend call isn't appropriate for all drivers.  It is a
simple and safe way to do the suspend for some drivers, mostly the
simpler ones.  Things that are complex enough to have a subsystem
(e.g. USB) would want to use the early suspend call.

> What happens if, in order to carry out the late-suspend, a driver needs
> to acquire a mutex which happens to be held by some other task?  That
> other task won't be able to run and release the mutex, so you will
> deadlock.

Then late-suspend is not appropriate for that driver, and it needs to
use the early-suspend call, and do something such as setting a flag
that the I/O request function tests.

Paul.

Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Oliver Neukum]

Am Dienstag, 3. Juli 2007 schrieb Miklos Szeredi:
> > And a further question. The freezer is not atomic. What do you do
> > if a task not yet frozen calls sys_sync(), but fuse is already frozen?
> 
> What do you do if a task not yet frozen writes to a pipe, on the other
> end of which is a task already frozen?

The same as you do with a pipe when the reader is not ready.

> It doesn't matter.  The only thing that should matter during suspend
> (not hibernate) is saving the state of devices to ram, and putting the
> devices to sleep.

Well, but you did remove sys_sync() from the freezer, which is
and must be called in the hibernate path.
 
> I'm not sure why this can't be made atomic, but assuming, that it
> can't, fuse should still not need to be implicated.  If it is, that's
> an indication about something wrong in the suspend procedure.

Nope, something's wrong in fuse. You must be able to deal with sync
until every task is frozen.

	Regards
		Oliver

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Alan Stern]

On Tue, 3 Jul 2007, Oliver Neukum wrote:

> Well, but you did remove sys_sync() from the freezer, which is
> and must be called in the hibernate path.

That's not really true.  We _want_ to call sys_sync() in both the 
hibernate and suspend paths (in case the batteries run down), to help 
avoid filesystem problems if something goes wrong with the resume.  But 
it isn't a hard requirement.

> > I'm not sure why this can't be made atomic, but assuming, that it
> > can't, fuse should still not need to be implicated.  If it is, that's
> > an indication about something wrong in the suspend procedure.
> 
> Nope, something's wrong in fuse. You must be able to deal with sync
> until every task is frozen.

That's ridiculous.  FUSE itself runs partially as a user task.  How can
you expect it to carry out a sync or anything else when it is frozen?

I suppose you could "deal" with it by having the kernel portion return
an error if the userspace part is frozen.  If the hibernate/suspend 
code bothered to check the return value, it would immediately abort 
the suspend.

Alan Stern

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Miklos Szeredi]

> > Well, but you did remove sys_sync() from the freezer, which is
> > and must be called in the hibernate path.
> 
> That's not really true.  We _want_ to call sys_sync() in both the 
> hibernate and suspend paths (in case the batteries run down), to help 
> avoid filesystem problems if something goes wrong with the resume.  But 
> it isn't a hard requirement.
> 
> > > I'm not sure why this can't be made atomic, but assuming, that it
> > > can't, fuse should still not need to be implicated.  If it is, that's
> > > an indication about something wrong in the suspend procedure.
> > 
> > Nope, something's wrong in fuse. You must be able to deal with sync
> > until every task is frozen.
> 
> That's ridiculous.  FUSE itself runs partially as a user task.  How can
> you expect it to carry out a sync or anything else when it is frozen?
> 
> I suppose you could "deal" with it by having the kernel portion return
> an error if the userspace part is frozen.  If the hibernate/suspend 
> code bothered to check the return value, it would immediately abort 
> the suspend.

I strongly believe, that we don't want to deal with it.  If we want to
call sync(), do it while the system is fully operational.  It's a best
effort thing anyway, and you can loose data in other ways if resume
fails.

Miklos

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Oliver Neukum]

Am Dienstag, 3. Juli 2007 schrieben Sie:
> On Tue, 3 Jul 2007, Oliver Neukum wrote:
> 
> > Well, but you did remove sys_sync() from the freezer, which is
> > and must be called in the hibernate path.
> 
> That's not really true.  We _want_ to call sys_sync() in both the 
> hibernate and suspend paths (in case the batteries run down), to help 
> avoid filesystem problems if something goes wrong with the resume.  But 
> it isn't a hard requirement.

But the ability to launder pages is needed. During hibernation we need
to shrink memory. I don't see how this would be fundamentally different
from calling sync.

> > > I'm not sure why this can't be made atomic, but assuming, that it
> > > can't, fuse should still not need to be implicated.  If it is, that's
> > > an indication about something wrong in the suspend procedure.
> > 
> > Nope, something's wrong in fuse. You must be able to deal with sync
> > until every task is frozen.
> 
> That's ridiculous.  FUSE itself runs partially as a user task.  How can
> you expect it to carry out a sync or anything else when it is frozen?

I don't and it might point to a fundamental problem.
But I cannot help but notice that syscalls may happen while the system
is partially frozen. It must be dealt with.

> I suppose you could "deal" with it by having the kernel portion return
> an error if the userspace part is frozen.  If the hibernate/suspend 
> code bothered to check the return value, it would immediately abort 
> the suspend.

Where exactly would that code notice the errors?

	Regards
		Oliver

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Rafael J. Wysocki]

On Tuesday, 3 July 2007 22:19, Miklos Szeredi wrote:
> > > Well, but you did remove sys_sync() from the freezer, which is
> > > and must be called in the hibernate path.
> > 
> > That's not really true.  We _want_ to call sys_sync() in both the 
> > hibernate and suspend paths (in case the batteries run down), to help 
> > avoid filesystem problems if something goes wrong with the resume.  But 
> > it isn't a hard requirement.
> > 
> > > > I'm not sure why this can't be made atomic, but assuming, that it
> > > > can't, fuse should still not need to be implicated.  If it is, that's
> > > > an indication about something wrong in the suspend procedure.
> > > 
> > > Nope, something's wrong in fuse. You must be able to deal with sync
> > > until every task is frozen.
> > 
> > That's ridiculous.  FUSE itself runs partially as a user task.  How can
> > you expect it to carry out a sync or anything else when it is frozen?
> > 
> > I suppose you could "deal" with it by having the kernel portion return
> > an error if the userspace part is frozen.  If the hibernate/suspend 
> > code bothered to check the return value, it would immediately abort 
> > the suspend.

Er, do_sync() doesn't return a result.

> I strongly believe, that we don't want to deal with it.  If we want to
> call sync(), do it while the system is fully operational.  It's a best
> effort thing anyway, and you can loose data in other ways if resume
> fails.

The requirement of syncing when the system (including the user space) is fully
operational is FUSE-specific.  Thus we'd rather like to sync FUSE filesystems
before freezing the user space and freeze the other filesystems after freezing
the user space.

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Bodo Eggert]

Miklos Szeredi <miklos@szeredi.hu> wrote:

>> > So to summarize, the plan that makes things work with fuse is:
>> > 
>> > - For STR, don't do the freezer thing.
>> > 
>> > - For STD, don't sys_sync() after you froze
>> > 
>> > There might be -other- issues, but that should get you through some of
>> 
>> At the risk of repeating myself. Character device drivers are written
>> with the assumption that normal io and suspend/resume do not race
>> with each other due to the freezer.
>> What do you intend to do about that?
> 
> Oliver, can you please explain your worries in a bit more detail?

Try suspending while printing.
-- 
What's worse than a Male Chauvinist Pig?
A woman that won't do what she's told.

Friß, Spammer: tgyvaxz@.qcLz7l.7eggert.dyndns.org

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Oliver Neukum]

Am Dienstag, 3. Juli 2007 schrieb Benjamin Herrenschmidt:
> So to summarize, the plan that makes things work with fuse is:
> 
>  - For STR, don't do the freezer thing.
> 
>  - For STD, don't sys_sync() after you froze
> 
> There might be -other- issues, but that should get you through some of

At the risk of repeating myself. Character device drivers are written
with the assumption that normal io and suspend/resume do not race
with each other due to the freezer.
What do you intend to do about that?

	Regards
		Oliver

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Miklos Szeredi]

> > So to summarize, the plan that makes things work with fuse is:
> > 
> >  - For STR, don't do the freezer thing.
> > 
> >  - For STD, don't sys_sync() after you froze
> > 
> > There might be -other- issues, but that should get you through some of
> 
> At the risk of repeating myself. Character device drivers are written
> with the assumption that normal io and suspend/resume do not race
> with each other due to the freezer.
> What do you intend to do about that?

Oliver, can you please explain your worries in a bit more detail?

I don't claim to know anything about how STR or hibernate works, but
neither seem to have any problem with I/O on the fuse device "racing"
with them.

And conceptually I can't see anything that would cause trouble either.
The fuse kernel module just provides a specialized IPC mechanism,
where one userspace process communicates with another using file
operations and a char dev.

Miklos

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Oliver Neukum]

Am Dienstag, 3. Juli 2007 schrieb Miklos Szeredi:
> > > So to summarize, the plan that makes things work with fuse is:
> > > 
> > >  - For STR, don't do the freezer thing.
> > > 
> > >  - For STD, don't sys_sync() after you froze
> > > 
> > > There might be -other- issues, but that should get you through some of
> > 
> > At the risk of repeating myself. Character device drivers are written
> > with the assumption that normal io and suspend/resume do not race
> > with each other due to the freezer.
> > What do you intend to do about that?
> 
> Oliver, can you please explain your worries in a bit more detail?
> 
> I don't claim to know anything about how STR or hibernate works, but
> neither seem to have any problem with I/O on the fuse device "racing"
> with them.

The problem is not with fuse. The problem is generic in nature.

If you remove the freezer, user space remains active until the last CPU
goes into suspend. It can do syscalls. Or do you know a clean way to exempt
only the tasks fuse might use?

Now device drivers have a guaranteed temporal sequence:

last io -> suspend() -> resume() [or disconnect()] -> new io

This is because suspend() is called after the freezer goes into action. If
you remove the freezer, you need to deal with

1. io to suspended devices
2. resume() assuming that the device is in the state suspend() left it
3. io changing a device's state while suspend is saving it

and you need to fix this for all device drivers, not just those fuse is
involved with. Removing the freezer means doing a more or less full
audit of every driver and additional locking in many drivers.

	Regards
		Oliver

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Miklos Szeredi]

> > > > So to summarize, the plan that makes things work with fuse is:
> > > > 
> > > >  - For STR, don't do the freezer thing.
> > > > 
> > > >  - For STD, don't sys_sync() after you froze
> > > > 
> > > > There might be -other- issues, but that should get you through some of
> > > 
> > > At the risk of repeating myself. Character device drivers are written
> > > with the assumption that normal io and suspend/resume do not race
> > > with each other due to the freezer.
> > > What do you intend to do about that?
> > 
> > Oliver, can you please explain your worries in a bit more detail?
> > 
> > I don't claim to know anything about how STR or hibernate works, but
> > neither seem to have any problem with I/O on the fuse device "racing"
> > with them.
> 
> The problem is not with fuse. The problem is generic in nature.
> 
> If you remove the freezer, user space remains active until the last CPU
> goes into suspend. It can do syscalls. Or do you know a clean way to exempt
> only the tasks fuse might use?

You are talking about hibernate, right?  Suspending (to ram) is
instantaneous, in that _after_ suspend no CPU is active obviously.

> Now device drivers have a guaranteed temporal sequence:
> 
> last io -> suspend() -> resume() [or disconnect()] -> new io
> 
> This is because suspend() is called after the freezer goes into action. If
> you remove the freezer, you need to deal with
> 
> 1. io to suspended devices
> 2. resume() assuming that the device is in the state suspend() left it
> 3. io changing a device's state while suspend is saving it
> 
> and you need to fix this for all device drivers, not just those fuse is
> involved with. Removing the freezer means doing a more or less full
> audit of every driver and additional locking in many drivers.

OK, this has _nothing_ at all to do with fuse then, and everything to
do with disk I/O.

Just because fuse is a filesystem, it doesn't have to do anything with
block devices, just like procfs doesn't either.

So removing the freezer from the hibernate path would be problematic,
but as I understand this is not what has been proposed, only removing
the freezer from the STR path, which should be OK.

Miklos

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Oliver Neukum]

Am Dienstag, 3. Juli 2007 schrieb Miklos Szeredi:
> > > > > So to summarize, the plan that makes things work with fuse is:
> > > > > 
> > > > >  - For STR, don't do the freezer thing.
> > > > > 
> > > > >  - For STD, don't sys_sync() after you froze
> > > > > 
> > > > > There might be -other- issues, but that should get you through some of
> > > > 
> > > > At the risk of repeating myself. Character device drivers are written
> > > > with the assumption that normal io and suspend/resume do not race
> > > > with each other due to the freezer.
> > > > What do you intend to do about that?
> > > 
> > > Oliver, can you please explain your worries in a bit more detail?
> > > 
> > > I don't claim to know anything about how STR or hibernate works, but
> > > neither seem to have any problem with I/O on the fuse device "racing"
> > > with them.
> > 
> > The problem is not with fuse. The problem is generic in nature.
> > 
> > If you remove the freezer, user space remains active until the last CPU
> > goes into suspend. It can do syscalls. Or do you know a clean way to exempt
> > only the tasks fuse might use?
> 
> You are talking about hibernate, right?  Suspending (to ram) is
> instantaneous, in that _after_ suspend no CPU is active obviously.

If that is so, why do you care? If it is really atomic, fuse has no chance
to call out to its component in user space either. Removing the freezer
cannot make a difference.

Something is fishy here.

	Regards
		Oliver

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Benjamin Herrenschmidt]

On Tue, 2007-07-03 at 13:27 +0200, Oliver Neukum wrote:
> > You are talking about hibernate, right?  Suspending (to ram) is
> > instantaneous, in that _after_ suspend no CPU is active obviously.
> 
> If that is so, why do you care? If it is really atomic, fuse has no
> chance
> to call out to its component in user space either. Removing the
> freezer
> cannot make a difference.

It's not atomic. You will get called after suspend() in drivers. The
thing is ... you just have to deal with it :-)

Ben.

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Oliver Neukum]

Am Dienstag, 3. Juli 2007 schrieb Benjamin Herrenschmidt:
> On Tue, 2007-07-03 at 13:27 +0200, Oliver Neukum wrote:
> > > You are talking about hibernate, right?  Suspending (to ram) is
> > > instantaneous, in that _after_ suspend no CPU is active obviously.
> > 
> > If that is so, why do you care? If it is really atomic, fuse has no
> > chance
> > to call out to its component in user space either. Removing the
> > freezer
> > cannot make a difference.
> 
> It's not atomic. You will get called after suspend() in drivers. The
> thing is ... you just have to deal with it :-)

So you are volunteering to go through all drivers >:-> ?

	Regards
		Oliver

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Pavel Machek]

Hi!

> > > I don't claim to know anything about how STR or hibernate works, but
> > > neither seem to have any problem with I/O on the fuse device "racing"
> > > with them.
> > 
> > The problem is not with fuse. The problem is generic in nature.
> > 
> > If you remove the freezer, user space remains active until the last CPU
> > goes into suspend. It can do syscalls. Or do you know a clean way to exempt
> > only the tasks fuse might use?
> 
> You are talking about hibernate, right?  Suspending (to ram) is
> instantaneous, in that _after_ suspend no CPU is active obviously.

No, suspend to ram is not instantaneous.

We may have 16 cpus, and we may have 250 disks that need to be spun
down. That takes time, and is really not atomic operation.
								Pavel

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

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Benjamin Herrenschmidt]

> The problem is not with fuse. The problem is generic in nature.
> 
> If you remove the freezer, user space remains active until the last CPU
> goes into suspend. It can do syscalls. Or do you know a clean way to exempt
> only the tasks fuse might use?
>
> Now device drivers have a guaranteed temporal sequence:
> 
> last io -> suspend() -> resume() [or disconnect()] -> new io

No, that's always been bullshit. You can have IOs emitted by kernel
threads (think knfsd, and that's just one among many others). Beside,
relying on having userland frozen means that your driver will be unable
to be "live" suspended/resumed for more ambitious dynamic power
management schemes.

So it's always been wrong, imho, to rely on that. I've had powermac STR
work fine without the freezer for years, and few drivers have been a
problem, and we just fixed them.

The freezer thingy, at best, hides problems, causing them not to be
fixed.

> This is because suspend() is called after the freezer goes into action. If
> you remove the freezer, you need to deal with
> 
> 1. io to suspended devices
> 2. resume() assuming that the device is in the state suspend() left it
> 3. io changing a device's state while suspend is saving it
> 
> and you need to fix this for all device drivers, not just those fuse is
> involved with. Removing the freezer means doing a more or less full
> audit of every driver and additional locking in many drivers.

Yes, more or less.

The good news is that a whole lot of drivers don't really care much, and
in some cases, things can be done trivially with a bit of help from the
upper layers. But yeah, as I've been explaining over and over again, the
lazy approach here doesn't work.

Ben.

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Oliver Neukum]

Am Dienstag, 3. Juli 2007 schrieb Benjamin Herrenschmidt:
> > Now device drivers have a guaranteed temporal sequence:
> > 
> > last io -> suspend() -> resume() [or disconnect()] -> new io
> 
> No, that's always been bullshit. You can have IOs emitted by kernel
> threads (think knfsd, and that's just one among many others). Beside,

That's why we have the problem of freezing the kernel threads or not.
Short of knfsd very few kernel threads really operate on their own and
those can use the new notifier chain.

> relying on having userland frozen means that your driver will be unable
> to be "live" suspended/resumed for more ambitious dynamic power
> management schemes.

Only if they work without cooperation and idle detection in the drivers.

> So it's always been wrong, imho, to rely on that. I've had powermac STR
> work fine without the freezer for years, and few drivers have been a
> problem, and we just fixed them.

Powermacs are somewhat limited in hardware used with them (OK, powerbook
have PCMCIA, but how often is that used?)

You want to have all that pain for fuse?

	Regards
		Oliver

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Paul Mackerras]

Oliver Neukum writes:

> That's why we have the problem of freezing the kernel threads or not.

That problem is a symptom of the deeper conceptual problem, as is the
problem with FUSE.

> You want to have all that pain for fuse?

I'd certainly rather get the drivers right, and maybe have an
occasional deadlock if I miss something, than have a GUARANTEED
deadlock every time I suspend with a FUSE filesystem mounted (which is
pretty much every time, since I use encfs regularly).

Paul.

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Miklos Szeredi]

> > That's why we have the problem of freezing the kernel threads or not.
> 
> That problem is a symptom of the deeper conceptual problem, as is the
> problem with FUSE.
> 
> > You want to have all that pain for fuse?
> 
> I'd certainly rather get the drivers right, and maybe have an
> occasional deadlock if I miss something, than have a GUARANTEED
> deadlock every time I suspend with a FUSE filesystem mounted (which is
> pretty much every time, since I use encfs regularly).

That's weird, I never had a suspend problem due to a fuse mount,
though I have them all the time.  And I suspect, that even the sync()
thing that suspend does is not the real cause, because sync() actually
does nothing in fuse filesystems.

So there's something else going on, which obviously has to do with
freezing user processes, but it's not clear what.

Miklos

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Paul Mackerras]

Miklos Szeredi writes:

> That's weird, I never had a suspend problem due to a fuse mount,
> though I have them all the time.  And I suspect, that even the sync()

Well, I don't either, because we don't freeze processes on
powerbooks.  But I have heard that other people have problems with
suspending with a fuse filesystem mounted.  Maybe the difference is
whether or not the filesystem is writable?

> thing that suspend does is not the real cause, because sync() actually
> does nothing in fuse filesystems.

It's not the filesystem sync method, as I understand it, it's that if
there are dirty pages in the page cache for files on the fuse
filesystem, the system will initiate a write-out on them and wait for
it to finish.  But if the fuse userspace is frozen, the write-out will
never complete.

Paul.

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Miklos Szeredi]

> > That's weird, I never had a suspend problem due to a fuse mount,
> > though I have them all the time.  And I suspect, that even the sync()
> 
> Well, I don't either, because we don't freeze processes on
> powerbooks.  But I have heard that other people have problems with
> suspending with a fuse filesystem mounted.  Maybe the difference is
> whether or not the filesystem is writable?
> 
> > thing that suspend does is not the real cause, because sync() actually
> > does nothing in fuse filesystems.
> 
> It's not the filesystem sync method, as I understand it, it's that if
> there are dirty pages in the page cache for files on the fuse
> filesystem,

Currently fuse doesn't produce dirty pages.  Normal writes are done
synchronously, and writable mmap is not supported.  So sync() should
really be a no-op for fuse.

> the system will initiate a write-out on them and wait for it to
> finish.  But if the fuse userspace is frozen, the write-out will
> never complete.

Maybe there is some other fs operation being done, possibly not
directly, but by waiting for a kernel thread, that does that.

It would be nice, if someone who can reproduce the deadlock could
debug it.  Does sysrq still work during suspend?

Miklos

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Rafael J. Wysocki]

On Wednesday, 4 July 2007 10:26, Miklos Szeredi wrote:
> > > That's weird, I never had a suspend problem due to a fuse mount,
> > > though I have them all the time.  And I suspect, that even the sync()
> > 
> > Well, I don't either, because we don't freeze processes on
> > powerbooks.  But I have heard that other people have problems with
> > suspending with a fuse filesystem mounted.  Maybe the difference is
> > whether or not the filesystem is writable?
> > 
> > > thing that suspend does is not the real cause, because sync() actually
> > > does nothing in fuse filesystems.
> > 
> > It's not the filesystem sync method, as I understand it, it's that if
> > there are dirty pages in the page cache for files on the fuse
> > filesystem,
> 
> Currently fuse doesn't produce dirty pages.  Normal writes are done
> synchronously, and writable mmap is not supported.  So sync() should
> really be a no-op for fuse.
> 
> > the system will initiate a write-out on them and wait for it to
> > finish.  But if the fuse userspace is frozen, the write-out will
> > never complete.
> 
> Maybe there is some other fs operation being done, possibly not
> directly, but by waiting for a kernel thread, that does that.

We're going to limit the freezing of kernel threads to the ones that explicitly
want to be frozen, so if that's the case, then I think it'll be fixed soon.

> It would be nice, if someone who can reproduce the deadlock could
> debug it.

Agreed.

> Does sysrq still work during suspend? 

Yes, it should work.

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Rafael J. Wysocki]

On Tuesday, 3 July 2007 13:07, Oliver Neukum wrote:
> Am Dienstag, 3. Juli 2007 schrieb Miklos Szeredi:
> > > > So to summarize, the plan that makes things work with fuse is:
> > > > 
> > > >  - For STR, don't do the freezer thing.
> > > > 
> > > >  - For STD, don't sys_sync() after you froze
> > > > 
> > > > There might be -other- issues, but that should get you through some of
> > > 
> > > At the risk of repeating myself. Character device drivers are written
> > > with the assumption that normal io and suspend/resume do not race
> > > with each other due to the freezer.
> > > What do you intend to do about that?
> > 
> > Oliver, can you please explain your worries in a bit more detail?
> > 
> > I don't claim to know anything about how STR or hibernate works, but
> > neither seem to have any problem with I/O on the fuse device "racing"
> > with them.
> 
> The problem is not with fuse. The problem is generic in nature.
> 
> If you remove the freezer, user space remains active until the last CPU
> goes into suspend. It can do syscalls. Or do you know a clean way to exempt
> only the tasks fuse might use?
> 
> Now device drivers have a guaranteed temporal sequence:
> 
> last io -> suspend() -> resume() [or disconnect()] -> new io
> 
> This is because suspend() is called after the freezer goes into action. If
> you remove the freezer, you need to deal with
> 
> 1. io to suspended devices
> 2. resume() assuming that the device is in the state suspend() left it
> 3. io changing a device's state while suspend is saving it
> 
> and you need to fix this for all device drivers, not just those fuse is
> involved with. Removing the freezer means doing a more or less full
> audit of every driver and additional locking in many drivers.

Agreed.

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Miklos Szeredi]

> > > > So to summarize, the plan that makes things work with fuse is:
> > > > 
> > > >  - For STR, don't do the freezer thing.
> > > > 
> > > >  - For STD, don't sys_sync() after you froze
> > > > 
> > > > There might be -other- issues, but that should get you through some of
> > > 
> > > At the risk of repeating myself. Character device drivers are written
> > > with the assumption that normal io and suspend/resume do not race
> > > with each other due to the freezer.
> > > What do you intend to do about that?
> > 
> > Oliver, can you please explain your worries in a bit more detail?
> > 
> > I don't claim to know anything about how STR or hibernate works, but
> > neither seem to have any problem with I/O on the fuse device "racing"
> > with them.
> 
> The problem is not with fuse. The problem is generic in nature.
> 
> If you remove the freezer, user space remains active until the last CPU
> goes into suspend. It can do syscalls. Or do you know a clean way to exempt
> only the tasks fuse might use?
> 
> Now device drivers have a guaranteed temporal sequence:
> 
> last io -> suspend() -> resume() [or disconnect()] -> new io
> 
> This is because suspend() is called after the freezer goes into action. If
> you remove the freezer, you need to deal with
> 
> 1. io to suspended devices
> 2. resume() assuming that the device is in the state suspend() left it
> 3. io changing a device's state while suspend is saving it
> 
> and you need to fix this for all device drivers, not just those fuse is
> involved with.

Fuse is not involved with _any_ device drivers.  It is fully unaware
of suspend issues and I think that's how it should stay ;)

> Removing the freezer means doing a more or less full
> audit of every driver and additional locking in many drivers.

How about a "CONFIG_NOFREEZE (experimental): only turn this on if you
want to fix buggy drivers that can fail during suspend with the
freezer turned off"?

I'm guessing quite a few kernel developers would be willing to turn on
such an option.

Miklos

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Paul Mackerras]

Oliver Neukum writes:

> At the risk of repeating myself. Character device drivers are written
> with the assumption that normal io and suspend/resume do not race
> with each other due to the freezer.
> What do you intend to do about that?

Going back to the old powerbook sleep code, we had a two-phase
suspend: drivers got notified once when userspace is still running,
with interrupts enabled, in process context; and then a second time
with interrupts disabled and with only one CPU up, so the process
that is initiating the suspend is the only process running (since
interrupts are disabled and nothing it does can sleep, no other
process can get to run).

I still believe that is the right way to go, although we currently
only have a single-phase suspend.

Most drivers suspended their hardware in the second call.  If they are
in the middle of a conversation with their device that *has* to be
completed, they can do that by polling.  If it's a character device, a
better approach would be to set a flag or whatever in the first
suspend call to make sure that no new conversations get started with
the device, sleeping if necessary.

I'm actually having a hard time thinking of how to test your assertion
since there are so few things on a typical computer that are plain
character devices driving real hardware.  A serial port would be about
the only one; keyboards and mice (and serial ports :) are USB these
days, or ADB on older powerbooks.

Or did you mean to include drivers for pseudo-devices (e.g. ptys)?  I
don't see why they would have a suspend method at all.

Paul.

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Benjamin Herrenschmidt]

On Tue, 2007-07-03 at 09:44 +0200, Oliver Neukum wrote:
> Am Dienstag, 3. Juli 2007 schrieb Benjamin Herrenschmidt:
> > So to summarize, the plan that makes things work with fuse is:
> > 
> >  - For STR, don't do the freezer thing.
> > 
> >  - For STD, don't sys_sync() after you froze
> > 
> > There might be -other- issues, but that should get you through some of
> 
> At the risk of repeating myself. Character device drivers are written
> with the assumption that normal io and suspend/resume do not race
> with each other due to the freezer.
> What do you intend to do about that?

Ugh ... "character devices" ... that's a pretty wide statement...
there's lots of those and very different one from the other...

Any sane device-driver will have to cope with being suspended in a
"live" system. I've demonstrated multiple times in the past why this is
necessary anyway, for things like dynamic power management, among
others.

The whole freezer thing is a hack job to avoid fixing drivers that need
fixing. Unfortunately, I believe in that area, it's simply not
sustainable. Besides, getting drivers to behave properly isn't very hard
in most cases.

Ben.

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Oliver Neukum]

Am Dienstag, 3. Juli 2007 schrieb Benjamin Herrenschmidt:
> On Tue, 2007-07-03 at 09:44 +0200, Oliver Neukum wrote:
> > Am Dienstag, 3. Juli 2007 schrieb Benjamin Herrenschmidt:
> > > So to summarize, the plan that makes things work with fuse is:
> > > 
> > >  - For STR, don't do the freezer thing.
> > > 
> > >  - For STD, don't sys_sync() after you froze
> > > 
> > > There might be -other- issues, but that should get you through some of
> > 
> > At the risk of repeating myself. Character device drivers are written
> > with the assumption that normal io and suspend/resume do not race
> > with each other due to the freezer.
> > What do you intend to do about that?
> 
> Ugh ... "character devices" ... that's a pretty wide statement...
> there's lots of those and very different one from the other...

That is a good summary of the problem ;-(

> Any sane device-driver will have to cope with being suspended in a
> "live" system. I've demonstrated multiple times in the past why this is
> necessary anyway, for things like dynamic power management, among
> others.

That is an interesting notion. I'd rather see device drivers reporting
their devices idle and requsting to be suspended.
But in any case it doesn't solve the problem.

	Regards
		Oliver

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Rafael J. Wysocki]

On Tuesday, 3 July 2007 13:46, Oliver Neukum wrote:
> Am Dienstag, 3. Juli 2007 schrieb Benjamin Herrenschmidt:
> > On Tue, 2007-07-03 at 09:44 +0200, Oliver Neukum wrote:
> > > Am Dienstag, 3. Juli 2007 schrieb Benjamin Herrenschmidt:
> > > > So to summarize, the plan that makes things work with fuse is:
> > > > 
> > > >  - For STR, don't do the freezer thing.
> > > > 
> > > >  - For STD, don't sys_sync() after you froze
> > > > 
> > > > There might be -other- issues, but that should get you through some of
> > > 
> > > At the risk of repeating myself. Character device drivers are written
> > > with the assumption that normal io and suspend/resume do not race
> > > with each other due to the freezer.
> > > What do you intend to do about that?
> > 
> > Ugh ... "character devices" ... that's a pretty wide statement...
> > there's lots of those and very different one from the other...
> 
> That is a good summary of the problem ;-(
> 
> > Any sane device-driver will have to cope with being suspended in a
> > "live" system. I've demonstrated multiple times in the past why this is
> > necessary anyway, for things like dynamic power management, among
> > others.
> 
> That is an interesting notion. I'd rather see device drivers reporting
> their devices idle and requsting to be suspended.
> But in any case it doesn't solve the problem.

Agreed.

What I think will solve the problem in the long run is to:

1) Separate the hibernation code from the suspend code (ie. hibernation-related
callbacks should generally be different from suspend-related callback for each
driver).
2) Remove the freezing of kernel threads from each of them (in the hibernation
case, if possible) an fix the things that get broken.
3) Remove the freezing of user space from the suspend code path and fix the
things that get broken.

Going to step 3) before doing 1) and 2) doesn't seem to be the right thing to
me.

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Oliver Neukum]

Am Dienstag, 3. Juli 2007 schrieb Paul Mackerras:
> I'm actually having a hard time thinking of how to test your assertion
> since there are so few things on a typical computer that are plain
> character devices driving real hardware.  A serial port would be about
> the only one; keyboards and mice (and serial ports :) are USB these
> days, or ADB on older powerbooks.

USB devices certainly have suspend methods.

	Regards
		Oliver

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Paul Mackerras]

Oliver Neukum writes:

> USB devices certainly have suspend methods.

Indeed, and the USB framework has code to know when the host
controller is suspended and avoid trying to send out urbs in that
case.  Or at least it did last time I looked at it in any detail; it's
been "just working" - including suspending and resuming, without the
freezer - for quite a while now.

Paul.

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Oliver Neukum]

Am Mittwoch, 4. Juli 2007 schrieb Paul Mackerras:
> Oliver Neukum writes:
> 
> > USB devices certainly have suspend methods.
> 
> Indeed, and the USB framework has code to know when the host
> controller is suspended and avoid trying to send out urbs in that
> case.  Or at least it did last time I looked at it in any detail; it's
> been "just working" - including suspending and resuming, without the
> freezer - for quite a while now.

And what happens to that IO? I suspended and lost data is not an
acceptable behavior.

	Oliver

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Paul Mackerras]

Oliver Neukum writes:

> > Indeed, and the USB framework has code to know when the host
> > controller is suspended and avoid trying to send out urbs in that
> > case.  Or at least it did last time I looked at it in any detail; it's
> > been "just working" - including suspending and resuming, without the
> > freezer - for quite a while now.
> 
> And what happens to that IO? I suspended and lost data is not an
> acceptable behavior.

It's not lost, it's sitting in RAM, and will be sent out when you
resume.

Paul.

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Oliver Neukum]

Am Mittwoch, 4. Juli 2007 schrieb Paul Mackerras:
> Oliver Neukum writes:
> 
> > > Indeed, and the USB framework has code to know when the host
> > > controller is suspended and avoid trying to send out urbs in that
> > > case.  Or at least it did last time I looked at it in any detail; it's
> > > been "just working" - including suspending and resuming, without the
> > > freezer - for quite a while now.
> > 
> > And what happens to that IO? I suspended and lost data is not an
> > acceptable behavior.
> 
> It's not lost, it's sitting in RAM, and will be sent out when you
> resume.

Unfortunately this is not the case. The URB will error out.

	Oliver

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Paul Mackerras]

Oliver Neukum writes:

> > It's not lost, it's sitting in RAM, and will be sent out when you
> > resume.
> 
> Unfortunately this is not the case. The URB will error out.

So the higher-level driver needs to do the sensible thing, i.e.,
resubmit the URB after resume.  It's not rocket science.  The data is
not lost, it's sitting in RAM, and the higher-level driver will send
it out when you resume.  If not, then we fix the higher-level driver.

Of course with USB there is the interesting question of whether the
device is still there when we resume.  But if it isn't, the situation
is no different to the user asynchronously unplugging the device
during operation, and if we lose data in that situation, we can only
blame the user. :)

Paul.

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Oliver Neukum]

Am Mittwoch, 4. Juli 2007 schrieb Paul Mackerras:
> Oliver Neukum writes:
> 
> > > It's not lost, it's sitting in RAM, and will be sent out when you
> > > resume.
> > 
> > Unfortunately this is not the case. The URB will error out.
> 
> So the higher-level driver needs to do the sensible thing, i.e.,
> resubmit the URB after resume.  It's not rocket science.  The data is
> not lost, it's sitting in RAM, and the higher-level driver will send
> it out when you resume.  If not, then we fix the higher-level driver.

You cannot simply restart the URB without thinking.
The device after resumption may or may not be in the stage
you left it. It needs to be rechecked and some settings must be
renewed. You cannot simple throw an URB from an arbitrary
stage of the protocol at it.
Suspension of devices can only happen at some points
in the protocol.

	Oliver

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Paul Mackerras]

Oliver Neukum writes:

> You cannot simply restart the URB without thinking.
> The device after resumption may or may not be in the stage
> you left it. It needs to be rechecked and some settings must be
> renewed. You cannot simple throw an URB from an arbitrary
> stage of the protocol at it.
> Suspension of devices can only happen at some points
> in the protocol.

Yeah, and?

I said "the higher-level driver needs to do the sensible thing".

Paul.

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Oliver Neukum]

Am Mittwoch, 4. Juli 2007 schrieb Paul Mackerras:
> Oliver Neukum writes:
> 
> > You cannot simply restart the URB without thinking.
> > The device after resumption may or may not be in the stage
> > you left it. It needs to be rechecked and some settings must be
> > renewed. You cannot simple throw an URB from an arbitrary
> > stage of the protocol at it.
> > Suspension of devices can only happen at some points
> > in the protocol.
> 
> Yeah, and?
> 
> I said "the higher-level driver needs to do the sensible thing".

They can't. Device specific protocols are known to the drivers only.
The fact remains, remove the freezer and you need to go through
all drivers.

	Oliver

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Paul Mackerras]

Oliver Neukum writes:

> They can't. Device specific protocols are known to the drivers only.
> The fact remains, remove the freezer and you need to go through
> all drivers.

The freezer does not actually mean that you don't have to get the
drivers right, because kernel threads can issue I/O requests.

Paul.

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Oliver Neukum]

Am Mittwoch, 4. Juli 2007 schrieb Paul Mackerras:
> Oliver Neukum writes:
> 
> > They can't. Device specific protocols are known to the drivers only.
> > The fact remains, remove the freezer and you need to go through
> > all drivers.
> 
> The freezer does not actually mean that you don't have to get the
> drivers right, because kernel threads can issue I/O requests.

Kernel threads do not issue requests for the hell of it. And yes,
kernel threads must be aware of suspension. Threads are few,
drivers many.

	Oliver

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Alan Stern]

On Wed, 4 Jul 2007, Paul Mackerras wrote:

> Oliver Neukum writes:
> 
> > USB devices certainly have suspend methods.
> 
> Indeed, and the USB framework has code to know when the host
> controller is suspended and avoid trying to send out urbs in that
> case.  Or at least it did last time I looked at it in any detail; it's
> been "just working" - including suspending and resuming, without the
> freezer - for quite a while now.

Evidently you haven't been stressing it.  You might try suspending
while printing to a USB printer.

Alan Stern

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Alan Stern]

On Tue, 3 Jul 2007, Paul Mackerras wrote:

> Going back to the old powerbook sleep code, we had a two-phase
> suspend: drivers got notified once when userspace is still running,
> with interrupts enabled, in process context; and then a second time
> with interrupts disabled and with only one CPU up, so the process
> that is initiating the suspend is the only process running (since
> interrupts are disabled and nothing it does can sleep, no other
> process can get to run).
> 
> I still believe that is the right way to go, although we currently
> only have a single-phase suspend.
> 
> Most drivers suspended their hardware in the second call.  If they are
> in the middle of a conversation with their device that *has* to be
> completed, they can do that by polling.

Ugh.  That will cause problems when you try to integrate runtime 
suspend.  In fact this whole approach is unsuitable for runtime PM and 
it obscures the similarities between runtime PM and STR.

>  If it's a character device, a
> better approach would be to set a flag or whatever in the first
> suspend call to make sure that no new conversations get started with
> the device, sleeping if necessary.
> 
> I'm actually having a hard time thinking of how to test your assertion
> since there are so few things on a typical computer that are plain
> character devices driving real hardware.  A serial port would be about
> the only one; keyboards and mice (and serial ports :) are USB these
> days, or ADB on older powerbooks.

You don't have to restrict yourself to character devices driving real 
hardware.  The same issues apply to USB and other buses.

Alan Stern

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Paul Mackerras]

Alan Stern writes:

> > Most drivers suspended their hardware in the second call.  If they are
> > in the middle of a conversation with their device that *has* to be
> > completed, they can do that by polling.
> 
> Ugh.  That will cause problems when you try to integrate runtime 
> suspend.  In fact this whole approach is unsuitable for runtime PM and 
> it obscures the similarities between runtime PM and STR.

Yes there are similarities, but it would be a big mistake to say that
a requirement for STR is that all drivers do runtime PM.

If a driver does runtime PM, that's great, and it is useful for
implementing STR.  However, there are a class of devices for which
runtime PM is not possible or not useful, but which can suspend/resume
just fine as part of suspending/resuming the complete system, and for
which all that is needed is some small amount of simple hardware
poking just before the system as a whole is put into suspend.  For
those a late-suspend call with interrupts off is the simplest and best
way to go.

Think of a serial port on a motherboard for instance, where the only
power control is the overall power control for the system.  All that
is needed is to poll for the transmitter being empty (with timeout, of
course) in the late-suspend call (and possibly also turn off output
drivers, perhaps), and to reinitialize some registers in an
early-resume call.

The main attraction of the late-suspend call is that it really does,
reliably, guarantee that the driver's I/O request methods won't get
called between the late-suspend call and the early-resume call.

Paul.

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Johannes Berg]

[-- Attachment #1.1: Type: text/plain, Size: 1073 bytes --]

On Tue, 2007-07-03 at 14:56 +0200, Rafael J. Wysocki wrote:

> Still, can you please read this post from Alan Stern:
> 
> https://lists.linux-foundation.org/pipermail/linux-pm/2007-June/012847.html
> 
> ?  I don't think I'm able to repeat the arguments given in there in a
> convincing way.

As I read it, Alan basically has two objections:
 (1) drivers shouldn't need to worry about this
 (2) suspend should be transparent to userspace

His proposed solution (freezing tasks when they cross the kernel
boundary) helps for the s-t-r case, but in fact doesn't solve (1)
because devices can be suspended at runtime and then you certainly do
not want to freeze tasks that try to access the device.

(2) is related but not identical, what if you have a device suspended at
runtime and some tasks tries to access it; should the task block until
you wake up that device?

I think the core of the discussion isn't appreciated by everybody here
yet---we need to solve both run-time and suspend-to-ram-time device
suspend, not just one of them.

johannes

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

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

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Alan Stern]

On Tue, 3 Jul 2007, Johannes Berg wrote:

> On Tue, 2007-07-03 at 14:56 +0200, Rafael J. Wysocki wrote:
> 
> > Still, can you please read this post from Alan Stern:
> > 
> > https://lists.linux-foundation.org/pipermail/linux-pm/2007-June/012847.html
> > 
> > ?  I don't think I'm able to repeat the arguments given in there in a
> > convincing way.
> 
> As I read it, Alan basically has two objections:
>  (1) drivers shouldn't need to worry about this
>  (2) suspend should be transparent to userspace
> 
> His proposed solution (freezing tasks when they cross the kernel
> boundary) helps for the s-t-r case, but in fact doesn't solve (1)
> because devices can be suspended at runtime and then you certainly do
> not want to freeze tasks that try to access the device.
> 
> (2) is related but not identical, what if you have a device suspended at
> runtime and some tasks tries to access it; should the task block until
> you wake up that device?

Time for me to jump in.

USB already implements runtime PM.  If a device is suspended at runtime
and a task tries to access it, the device is automatically resumed.  
No problem there.

The problem comes when the system is doing a STR.  Right now the code
doesn't keep track of the difference between a runtime suspend and a
system suspend -- once the device is suspended, it's suspended, period.  
Consequently, a non-frozen user task trying to do I/O to a suspended
device during STR will cause that device to resume, thereby forcing the
system suspend to abort.  Something much like this has actually
happened and been reported as a bug on LKML (I don't have a URL handy,
and it was actually a non-frozen kernel thread interfering with
hibernate rather than a non-frozen user task interfering with STR, but
the principle is the same).

Yes, the code could be changed to keep track of the reason for a device
suspend.  But that just raises the old problem of what to do when
there's an I/O request for a suspended device during STR.

> I think the core of the discussion isn't appreciated by everybody here
> yet---we need to solve both run-time and suspend-to-ram-time device
> suspend, not just one of them.

Runtime suspend isn't a problem.  Only STR.

Consider a particularly troublesome case: During STR, a non-frozen task
writes to /sys/bus/BBB/drivers/DDD/bind.  The sysfs core grabs the
device semaphore and calls the driver's probe routine.  If the driver
isn't PM-aware it simply tries to initialize the device and fails
because the device is already suspended.  That's no good; it isn't
transparent.

So assume the driver is PM-aware.  It tries to resume the device, which
fails because STR is underway.  Now what can it do?  There's only one 
possibility: It must block until the resume call can succeed.  But when 
is that?

It has to be before the PM core tries to resume the device, because the 
core will try to acquire the device semaphore and will block waiting 
for the probe call to complete.  But it has to be after the PM core 
resumes the device's parent, because obviously the device can't resume 
until its parent is awake.

As you can see, this is a very difficult problem to solve.

Alan Stern

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Johannes Berg]

[-- Attachment #1.1: Type: text/plain, Size: 3019 bytes --]

On Tue, 2007-07-03 at 10:50 -0400, Alan Stern wrote:

> Time for me to jump in.

:)

> USB already implements runtime PM.  If a device is suspended at runtime
> and a task tries to access it, the device is automatically resumed.  
> No problem there.

Right.

> The problem comes when the system is doing a STR.  Right now the code
> doesn't keep track of the difference between a runtime suspend and a
> system suspend -- once the device is suspended, it's suspended, period.  
> Consequently, a non-frozen user task trying to do I/O to a suspended
> device during STR will cause that device to resume, thereby forcing the
> system suspend to abort.  Something much like this has actually
> happened and been reported as a bug on LKML (I don't have a URL handy,
> and it was actually a non-frozen kernel thread interfering with
> hibernate rather than a non-frozen user task interfering with STR, but
> the principle is the same).

Yeah, I can see that happen.

> Yes, the code could be changed to keep track of the reason for a device
> suspend.  But that just raises the old problem of what to do when
> there's an I/O request for a suspended device during STR.
> 
> > I think the core of the discussion isn't appreciated by everybody here
> > yet---we need to solve both run-time and suspend-to-ram-time device
> > suspend, not just one of them.
> 
> Runtime suspend isn't a problem.  Only STR.

Ah but for all those character devices people were saying are the
problem we haven't even solved runtime suspend as far as I can tell from
the discussion.

> Consider a particularly troublesome case: During STR, a non-frozen task
> writes to /sys/bus/BBB/drivers/DDD/bind.  The sysfs core grabs the
> device semaphore and calls the driver's probe routine.  If the driver
> isn't PM-aware it simply tries to initialize the device and fails
> because the device is already suspended.  That's no good; it isn't
> transparent.
> 
> So assume the driver is PM-aware.  It tries to resume the device, which
> fails because STR is underway.  Now what can it do?  There's only one 
> possibility: It must block until the resume call can succeed.  But when 
> is that?
> 
> It has to be before the PM core tries to resume the device, because the 
> core will try to acquire the device semaphore and will block waiting 
> for the probe call to complete.  But it has to be after the PM core 
> resumes the device's parent, because obviously the device can't resume 
> until its parent is awake.
> 
> As you can see, this is a very difficult problem to solve.

Indeed. Actually, one could argue that it's impossible to solve the
problem as long as we try to call out to userspace during suspend and
need to wait until that's finished, like in the case of sys_sync() and
fuse filesystems, and probably other cases. Maybe we should make *those*
calls return a failure so that the suspend isn't transparent inside the
kernel but is transparent to userspace.

johannes

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

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

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Rafael J. Wysocki]

On Tuesday, 3 July 2007 16:59, Johannes Berg wrote:
> On Tue, 2007-07-03 at 10:50 -0400, Alan Stern wrote:
> 
> > Time for me to jump in.
> 
> :)
> 
> > USB already implements runtime PM.  If a device is suspended at runtime
> > and a task tries to access it, the device is automatically resumed.  
> > No problem there.
> 
> Right.
> 
> > The problem comes when the system is doing a STR.  Right now the code
> > doesn't keep track of the difference between a runtime suspend and a
> > system suspend -- once the device is suspended, it's suspended, period.  
> > Consequently, a non-frozen user task trying to do I/O to a suspended
> > device during STR will cause that device to resume, thereby forcing the
> > system suspend to abort.  Something much like this has actually
> > happened and been reported as a bug on LKML (I don't have a URL handy,
> > and it was actually a non-frozen kernel thread interfering with
> > hibernate rather than a non-frozen user task interfering with STR, but
> > the principle is the same).
> 
> Yeah, I can see that happen.
> 
> > Yes, the code could be changed to keep track of the reason for a device
> > suspend.  But that just raises the old problem of what to do when
> > there's an I/O request for a suspended device during STR.
> > 
> > > I think the core of the discussion isn't appreciated by everybody here
> > > yet---we need to solve both run-time and suspend-to-ram-time device
> > > suspend, not just one of them.
> > 
> > Runtime suspend isn't a problem.  Only STR.
> 
> Ah but for all those character devices people were saying are the
> problem we haven't even solved runtime suspend as far as I can tell from
> the discussion.
> 
> > Consider a particularly troublesome case: During STR, a non-frozen task
> > writes to /sys/bus/BBB/drivers/DDD/bind.  The sysfs core grabs the
> > device semaphore and calls the driver's probe routine.  If the driver
> > isn't PM-aware it simply tries to initialize the device and fails
> > because the device is already suspended.  That's no good; it isn't
> > transparent.
> > 
> > So assume the driver is PM-aware.  It tries to resume the device, which
> > fails because STR is underway.  Now what can it do?  There's only one 
> > possibility: It must block until the resume call can succeed.  But when 
> > is that?
> > 
> > It has to be before the PM core tries to resume the device, because the 
> > core will try to acquire the device semaphore and will block waiting 
> > for the probe call to complete.  But it has to be after the PM core 
> > resumes the device's parent, because obviously the device can't resume 
> > until its parent is awake.
> > 
> > As you can see, this is a very difficult problem to solve.
> 
> Indeed. Actually, one could argue that it's impossible to solve the
> problem as long as we try to call out to userspace during suspend and
> need to wait until that's finished, like in the case of sys_sync() and
> fuse filesystems, and probably other cases. Maybe we should make *those*
> calls return a failure so that the suspend isn't transparent inside the
> kernel but is transparent to userspace.

Well, it generally needs more consideration. :-)

I think that we should introduce mechanisms that will allow us to notify all
kernel subsystems, including FUSE and similar, that the system is going to
enter a sleep state (one of those is the notifier chain introduced recently).

Then, they may react to such a notification by entering a "suspend" mode
of operation in which they will return errors from some callbacks that
otherwise should have succeeded etc.  That depends on the subsystem in
question.

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Miklos Szeredi]

> > Indeed. Actually, one could argue that it's impossible to solve the
> > problem as long as we try to call out to userspace during suspend and
> > need to wait until that's finished, like in the case of sys_sync() and
> > fuse filesystems, and probably other cases. Maybe we should make *those*
> > calls return a failure so that the suspend isn't transparent inside the
> > kernel but is transparent to userspace.
> 
> Well, it generally needs more consideration. :-)
> 
> I think that we should introduce mechanisms that will allow us to notify all
> kernel subsystems, including FUSE and similar, that the system is going to
> enter a sleep state (one of those is the notifier chain introduced recently).

Ugh, please no.

Believe me, fuse is doing _nothing_ out of the ordinary, and should
not need special treatment during suspend/resume.  If suspend itself
is doing something that triggers fuse activity, then that's a bug,
such as the sync() thing that started this thread.

> Then, they may react to such a notification by entering a "suspend" mode
> of operation in which they will return errors from some callbacks that
> otherwise should have succeeded etc.  That depends on the subsystem in
> question.

Sounds horrible.

Why do we need to deal with subsystem interdependencies during
suspend?  Isn't it about saving device state to ram?  That definitely
_should not_ need to trigger anything that touches filesystems or
other subsystems.

Miklos

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Rafael J. Wysocki]

On Tuesday, 3 July 2007 19:38, Miklos Szeredi wrote:
> > > Indeed. Actually, one could argue that it's impossible to solve the
> > > problem as long as we try to call out to userspace during suspend and
> > > need to wait until that's finished, like in the case of sys_sync() and
> > > fuse filesystems, and probably other cases. Maybe we should make *those*
> > > calls return a failure so that the suspend isn't transparent inside the
> > > kernel but is transparent to userspace.
> > 
> > Well, it generally needs more consideration. :-)
> > 
> > I think that we should introduce mechanisms that will allow us to notify all
> > kernel subsystems, including FUSE and similar, that the system is going to
> > enter a sleep state (one of those is the notifier chain introduced recently).
> 
> Ugh, please no.
> 
> Believe me, fuse is doing _nothing_ out of the ordinary, and should
> not need special treatment during suspend/resume.  If suspend itself
> is doing something that triggers fuse activity, then that's a bug,
> such as the sync() thing that started this thread.

Apart from the sync, it shouldn't trigger any fs activity.  Still, some other
task running concurrently with the suspend code may do that and _if_
we are going to allow that to happen (and we do, if we remove the freezer
from the suspend code path), we will have to take that into consideration.

> > Then, they may react to such a notification by entering a "suspend" mode
> > of operation in which they will return errors from some callbacks that
> > otherwise should have succeeded etc.  That depends on the subsystem in
> > question.
> 
> Sounds horrible.
> 
> Why do we need to deal with subsystem interdependencies during
> suspend?  Isn't it about saving device state to ram?

No.  In addition, the devices should be prevented from generating interrupts
or initiating DMA transfers and put into low power states before we actually
suspend the system (platform).  This is a bit difficult to do while all user
space is running.

> That definitely _should not_ need to trigger anything that touches
> filesystems or other subsystems.

Right, but the subsystems may do something that affects devices.

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Alan Stern]

On Tue, 3 Jul 2007, Johannes Berg wrote:

> > Runtime suspend isn't a problem.  Only STR.
> 
> Ah but for all those character devices people were saying are the
> problem we haven't even solved runtime suspend as far as I can tell from
> the discussion.

The technique used by USB for runtime PM should work fine with other
devices.  They may not have implemented it yet, but I consider the
matter more-or-less solved.


> > As you can see, this is a very difficult problem to solve.
> 
> Indeed. Actually, one could argue that it's impossible to solve the
> problem as long as we try to call out to userspace during suspend and
> need to wait until that's finished, like in the case of sys_sync() and
> fuse filesystems, and probably other cases. Maybe we should make *those*
> calls return a failure so that the suspend isn't transparent inside the
> kernel but is transparent to userspace.

I disagree.  The problem isn't the kernel calling userspace; it's
userspace trying to do I/O at a time when everything is supposed to be
quiescing.  Detecting that and blocking it in drivers is hard and
error-prone; preventing it by freezing userspace is easy and cheap.

The reasons why the PPC people dislike the whole idea aren't clear to
me.  If it were necessary to have some user task running in order to
carry out the STR then their objection would make sense -- obviously
that task couldn't do its job if it were frozen.  But it isn't
necessary, or at least it should not be.

Userspace will be effectively "frozen" while the system as a whole is 
suspended.  So what's wrong with freezing it a little early?  Despite 
Ben's comments, it seems to me that the freezer doesn't hide problems 
-- it prevents them.

Now people may claim that the freezer implementation itself is buggy.  
I wouldn't dispute it.  But the bugs should be fixable; nobody has 
pointed out anything fundamentally wrong with the idea AFAICT.

Alan Stern

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Paul Mackerras]

Alan Stern writes:

> I disagree.  The problem isn't the kernel calling userspace; it's
> userspace trying to do I/O at a time when everything is supposed to be
> quiescing.  Detecting that and blocking it in drivers is hard and
> error-prone; preventing it by freezing userspace is easy and cheap.

And unreliable, and prone to deadlocks, and invasive - requiring
changes to kernel threads that have nothing to do with drivers or
suspend/resume.

> The reasons why the PPC people dislike the whole idea aren't clear to
> me. 

Our experience is that it isn't necessary.  It's extra code that in
practice causes deadlocks and added maintenance burden for no
discernable benefit.

> If it were necessary to have some user task running in order to
> carry out the STR then their objection would make sense -- obviously
> that task couldn't do its job if it were frozen.  But it isn't
> necessary, or at least it should not be.

The freezer doesn't achieve its stated goal of preventing drivers from
getting I/O requests after suspend, since kernel threads can (and do)
initiate I/O.  So then we say that some kernel threads need to be
frozen and others don't, but making that decision is difficult and
error-prone.

Besides, any kernel thread that does I/O is potentially doing that in
order to complete some other I/O request.  So we want to freeze it in
order to prevent new I/O requests from being initiated, but we don't
want to freeze it so that existing I/O requests can be completed.
Thus we have a fundamental conflict in the notion of the freezer.

In fact I believe that making a distinction between user and kernel
threads is wrong and likely to lead to problems, since userspace can
be involved in doing I/O (e.g. FUSE or the user-space driver
framework).  So the argument of the previous paragraph also applies to
some userspace processes.

> Userspace will be effectively "frozen" while the system as a whole is 
> suspended.  So what's wrong with freezing it a little early?  Despite 
> Ben's comments, it seems to me that the freezer doesn't hide problems 
> -- it prevents them.

No, it appears to prevent them, but doesn't in fact.

I remain convinced that the right approach is to fix the drivers to do
one of two things; either do something in the suspend call to block
further requests to the device, or use a late-suspend call to put
their device into a low-power state.  Of course, correctly-written
frameworks can do a lot to help the chipset drivers here.

> Now people may claim that the freezer implementation itself is buggy.  
> I wouldn't dispute it.  But the bugs should be fixable; nobody has 
> pointed out anything fundamentally wrong with the idea AFAICT.

The fundamental problem is the kernel threads and user processes that
we need running to complete existing I/O requests, but which may
initiate new I/O requests in doing so.

The right way to solve the problem is to do the request blocking in
the drivers.

Paul.

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Alan Stern]

On Wed, 4 Jul 2007, Paul Mackerras wrote:

> Alan Stern writes:
> 
> > I disagree.  The problem isn't the kernel calling userspace; it's
> > userspace trying to do I/O at a time when everything is supposed to be
> > quiescing.  Detecting that and blocking it in drivers is hard and
> > error-prone; preventing it by freezing userspace is easy and cheap.
> 
> And unreliable, and prone to deadlocks, and invasive - requiring
> changes to kernel threads that have nothing to do with drivers or
> suspend/resume.

Let's agree the kernel threads and the freezer are a separate issue.  
In the most recent kernels, the freezer does not suspend kernel threads 
by default.

I agree the kernel threads which try to do I/O during a suspend will 
need extra attention.  However if these threads are necessary for the 
suspend procedure, then blocking them (which is how people on this 
thread have been saying driver should treat I/O requests during a 
suspend) will cause additional problems.  There's no way around it; 
these threads _will_ require more work.

> > The reasons why the PPC people dislike the whole idea aren't clear to
> > me. 
> 
> Our experience is that it isn't necessary.  It's extra code that in
> practice causes deadlocks and added maintenance burden for no
> discernable benefit.

I have discussed the benefits elsewhere.  As for the deadlocks -- do 
you still observe them if you use the version of the freezer which 
doesn't freeze kernel threads?

> The freezer doesn't achieve its stated goal of preventing drivers from
> getting I/O requests after suspend, since kernel threads can (and do)
> initiate I/O.  So then we say that some kernel threads need to be
> frozen and others don't, but making that decision is difficult and
> error-prone.

No -- we say that the kernel threads which generate I/O requests during 
suspend need to be changed.

> In fact I believe that making a distinction between user and kernel
> threads is wrong and likely to lead to problems, since userspace can
> be involved in doing I/O (e.g. FUSE or the user-space driver
> framework).  So the argument of the previous paragraph also applies to
> some userspace processes.

Userspace cannot do I/O directly on its own, apart from some
exceptional situations where a privileged task directly twiddles some
I/O ports or the equivalent.

There remains the problem of user tasks whose assistance is required to 
carry out some I/O (as with FUSE).  If the I/O can be deferred until 
after the resume, then there's no problem.  If the I/O can be carried 
out before the suspend, then it should be.  And finally, if the I/O 
must be done during the suspend, you're in real trouble -- how do you 
do I/O to a suspended device?

> I remain convinced that the right approach is to fix the drivers to do
> one of two things; either do something in the suspend call to block
> further requests to the device, or use a late-suspend call to put
> their device into a low-power state.  Of course, correctly-written
> frameworks can do a lot to help the chipset drivers here.

The first alternative is a possibility.  My argument all along has been 
that it is difficult and error-prone, and it adds more overhead to 
system operation (even when not suspending!) than simply freezing 
userspace.

Alan Stern

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Paul Mackerras]

Alan Stern writes:

> Let's agree the kernel threads and the freezer are a separate issue.  

No, I don't think they are a separate issue, because I think the
distinction the freezer makes between kernel threads and user threads
is a false and misleading distinction.

> In the most recent kernels, the freezer does not suspend kernel threads 
> by default.

And therefore doesn't guarantee that drivers won't get I/O requests
after being suspended, as far as I can see...

> I agree the kernel threads which try to do I/O during a suspend will 
> need extra attention.  However if these threads are necessary for the 
> suspend procedure, then blocking them (which is how people on this 
> thread have been saying driver should treat I/O requests during a 
> suspend) will cause additional problems.  There's no way around it; 
> these threads _will_ require more work.

There is a way around it; do the request blocking in the drivers,
where it belongs.

> > > The reasons why the PPC people dislike the whole idea aren't clear to
> > > me. 
> > 
> > Our experience is that it isn't necessary.  It's extra code that in
> > practice causes deadlocks and added maintenance burden for no
> > discernable benefit.
> 
> I have discussed the benefits elsewhere.  As for the deadlocks -- do 
> you still observe them if you use the version of the freezer which 
> doesn't freeze kernel threads?

In general the only way to guarantee there are no deadlocks is to
construct the graph of dependencies between tasks.  Those dependencies
are not in practice observable from outside the tasks, so it is
virtually impossible to construct the graph.

The "don't freeze kernel threads" thing is an attempt to make a crude
approximation to the dependency graph (by saying kernel threads only
depend on other kernel threads), but the approximation breaks down
when you have FUSE or user-level device drivers.

> Userspace cannot do I/O directly on its own, apart from some
> exceptional situations where a privileged task directly twiddles some
> I/O ports or the equivalent.

Userspace can be involved in servicing I/O requests; not just FUSE,
but also user-level nfsd and user-level PPP demonstrate that.

> There remains the problem of user tasks whose assistance is required to 
> carry out some I/O (as with FUSE).  If the I/O can be deferred until 
> after the resume, then there's no problem.  If the I/O can be carried 
> out before the suspend, then it should be.  And finally, if the I/O 
> must be done during the suspend, you're in real trouble -- how do you 
> do I/O to a suspended device?

So why doesn't that argument apply to kernel threads? :)

> > I remain convinced that the right approach is to fix the drivers to do
> > one of two things; either do something in the suspend call to block
> > further requests to the device, or use a late-suspend call to put
> > their device into a low-power state.  Of course, correctly-written
> > frameworks can do a lot to help the chipset drivers here.
> 
> The first alternative is a possibility.  My argument all along has been 
> that it is difficult and error-prone, and it adds more overhead to 
> system operation (even when not suspending!) than simply freezing 
> userspace.

It does actually provably solve the problem though, which is more than
the freezer does.

Paul.

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Oliver Neukum]

Am Donnerstag, 5. Juli 2007 schrieb Paul Mackerras:
> > I have discussed the benefits elsewhere.  As for the deadlocks -- do 
> > you still observe them if you use the version of the freezer which 
> > doesn't freeze kernel threads?
> 
> In general the only way to guarantee there are no deadlocks is to
> construct the graph of dependencies between tasks.  Those dependencies
> are not in practice observable from outside the tasks, so it is
> virtually impossible to construct the graph.

In which way can user space tasks depend on each other in a way that
allows a them members of that cycle to be in uninterruptible sleep?

	Regards
		Oliver

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Miklos Szeredi]

> > > I have discussed the benefits elsewhere.  As for the deadlocks -- do 
> > > you still observe them if you use the version of the freezer which 
> > > doesn't freeze kernel threads?
> > 
> > In general the only way to guarantee there are no deadlocks is to
> > construct the graph of dependencies between tasks.  Those dependencies
> > are not in practice observable from outside the tasks, so it is
> > virtually impossible to construct the graph.
> 
> In which way can user space tasks depend on each other in a way that
> allows a them members of that cycle to be in uninterruptible sleep?

 - process A calls rename() on a fuse fs
 - process B, the fuse server, starts to process the rename request
 - process B is frozen before it can reply

Now process A is unfreezable.  We cannot make rename() restartable,
hence it cannot be interruptible.

Miklos

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Oliver Neukum]

Am Donnerstag, 5. Juli 2007 schrieb Miklos Szeredi:
> > > > I have discussed the benefits elsewhere.  As for the deadlocks -- do 
> > > > you still observe them if you use the version of the freezer which 
> > > > doesn't freeze kernel threads?
> > > 
> > > In general the only way to guarantee there are no deadlocks is to
> > > construct the graph of dependencies between tasks.  Those dependencies
> > > are not in practice observable from outside the tasks, so it is
> > > virtually impossible to construct the graph.
> > 
> > In which way can user space tasks depend on each other in a way that
> > allows a them members of that cycle to be in uninterruptible sleep?
> 
>  - process A calls rename() on a fuse fs
>  - process B, the fuse server, starts to process the rename request
>  - process B is frozen before it can reply
> 
> Now process A is unfreezable.  We cannot make rename() restartable,
> hence it cannot be interruptible.

Then this is a problem specific to fuse. You should teach fuse to block
suspension while such operations are being performed.

	Regards
		Oliver

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Miklos Szeredi]

> > > > > I have discussed the benefits elsewhere.  As for the deadlocks -- do 
> > > > > you still observe them if you use the version of the freezer which 
> > > > > doesn't freeze kernel threads?
> > > > 
> > > > In general the only way to guarantee there are no deadlocks is to
> > > > construct the graph of dependencies between tasks.  Those dependencies
> > > > are not in practice observable from outside the tasks, so it is
> > > > virtually impossible to construct the graph.
> > > 
> > > In which way can user space tasks depend on each other in a way that
> > > allows a them members of that cycle to be in uninterruptible sleep?
> > 
> >  - process A calls rename() on a fuse fs
> >  - process B, the fuse server, starts to process the rename request
> >  - process B is frozen before it can reply
> > 
> > Now process A is unfreezable.  We cannot make rename() restartable,
> > hence it cannot be interruptible.
> 
> Then this is a problem specific to fuse. You should teach fuse to block
> suspension while such operations are being performed.

And teach VFS to block suspension, while waiting on a mutex held by
another process performing a fuse operation.

I can already hear the beautiful praise from Al Viro at the sight of
that ;)

Miklos

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Oliver Neukum]

Am Donnerstag, 5. Juli 2007 schrieb Miklos Szeredi:
> > > > > > I have discussed the benefits elsewhere.  As for the deadlocks -- do 
> > > > > > you still observe them if you use the version of the freezer which 
> > > > > > doesn't freeze kernel threads?
> > > > > 
> > > > > In general the only way to guarantee there are no deadlocks is to
> > > > > construct the graph of dependencies between tasks.  Those dependencies
> > > > > are not in practice observable from outside the tasks, so it is
> > > > > virtually impossible to construct the graph.
> > > > 
> > > > In which way can user space tasks depend on each other in a way that
> > > > allows a them members of that cycle to be in uninterruptible sleep?
> > > 
> > >  - process A calls rename() on a fuse fs
> > >  - process B, the fuse server, starts to process the rename request
> > >  - process B is frozen before it can reply
> > > 
> > > Now process A is unfreezable.  We cannot make rename() restartable,
> > > hence it cannot be interruptible.
> > 
> > Then this is a problem specific to fuse. You should teach fuse to block
> > suspension while such operations are being performed.
> 
> And teach VFS to block suspension, while waiting on a mutex held by
> another process performing a fuse operation.
> 
> I can already hear the beautiful praise from Al Viro at the sight of
> that ;)

There is that.

OK, bite the bullet. Tasks involved in fuse are special. Give them a flag
and teach the freezer to put them on ice only after all other task are
frozen. In a way they are kernel, there's no use denying that.

	Regards
		Oliver

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Miklos Szeredi]

> > And teach VFS to block suspension, while waiting on a mutex held by
> > another process performing a fuse operation.
> > 
> > I can already hear the beautiful praise from Al Viro at the sight of
> > that ;)
> 
> There is that.
> 
> OK, bite the bullet. Tasks involved in fuse are special. Give them a flag
> and teach the freezer to put them on ice only after all other task are
> frozen. In a way they are kernel, there's no use denying that.

And flag every other process, that the flagged process is
communicating with?  How are you proposing to do that?

Quoting Paul:

"1. The freezer cannot be guaranteed deadlock-free without constructing
   a dependency graph between tasks (both user and kernel), which is
   virtually impossible since the dependencies are not externally
   observable."

Miklos

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Oliver Neukum]

Am Donnerstag, 5. Juli 2007 schrieb Miklos Szeredi:
> > > And teach VFS to block suspension, while waiting on a mutex held by
> > > another process performing a fuse operation.
> > > 
> > > I can already hear the beautiful praise from Al Viro at the sight of
> > > that ;)
> > 
> > There is that.
> > 
> > OK, bite the bullet. Tasks involved in fuse are special. Give them a flag
> > and teach the freezer to put them on ice only after all other task are
> > frozen. In a way they are kernel, there's no use denying that.
> 
> And flag every other process, that the flagged process is
> communicating with?  How are you proposing to do that?
> 
> Quoting Paul:
> 
> "1. The freezer cannot be guaranteed deadlock-free without constructing
>    a dependency graph between tasks (both user and kernel), which is
>    virtually impossible since the dependencies are not externally
>    observable."

A deadlock requires that the circular wait is uninterruptible. Normal IPC
isn't.

What are you doing in the userland portions of fuse? Some kind of IPC
with other tasks? There is a limit to which you can push kernel functionality
into user space.

	Regards
		Oliver

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Miklos Szeredi]

> > > > And teach VFS to block suspension, while waiting on a mutex held by
> > > > another process performing a fuse operation.
> > > > 
> > > > I can already hear the beautiful praise from Al Viro at the sight of
> > > > that ;)
> > > 
> > > There is that.
> > > 
> > > OK, bite the bullet. Tasks involved in fuse are special. Give them a flag
> > > and teach the freezer to put them on ice only after all other task are
> > > frozen. In a way they are kernel, there's no use denying that.
> > 
> > And flag every other process, that the flagged process is
> > communicating with?  How are you proposing to do that?
> > 
> > Quoting Paul:
> > 
> > "1. The freezer cannot be guaranteed deadlock-free without constructing
> >    a dependency graph between tasks (both user and kernel), which is
> >    virtually impossible since the dependencies are not externally
> >    observable."
> 
> A deadlock requires that the circular wait is uninterruptible. Normal IPC
> isn't.
> 
> What are you doing in the userland portions of fuse? Some kind of IPC
> with other tasks?

Anything, writing to a file, writing to shared memory, sending things
over the network.  There's no limit to what a filesystem daemon may
do.  It's a perfectly ordinary unprivileged userspace process.  And
this is a feature not a bug.

> There is a limit to which you can push kernel functionality into
> user space.

Limiting what a userspace filesystem can do would defeat the whole
purpose of the bloody thing.  This is not negotiable ;)

Miklos

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Rafael J. Wysocki]

On Thursday, 5 July 2007 12:14, Miklos Szeredi wrote:
> > > > > And teach VFS to block suspension, while waiting on a mutex held by
> > > > > another process performing a fuse operation.
> > > > > 
> > > > > I can already hear the beautiful praise from Al Viro at the sight of
> > > > > that ;)
> > > > 
> > > > There is that.
> > > > 
> > > > OK, bite the bullet. Tasks involved in fuse are special. Give them a flag
> > > > and teach the freezer to put them on ice only after all other task are
> > > > frozen. In a way they are kernel, there's no use denying that.
> > > 
> > > And flag every other process, that the flagged process is
> > > communicating with?  How are you proposing to do that?
> > > 
> > > Quoting Paul:
> > > 
> > > "1. The freezer cannot be guaranteed deadlock-free without constructing
> > >    a dependency graph between tasks (both user and kernel), which is
> > >    virtually impossible since the dependencies are not externally
> > >    observable."

This statement is ganarally false.

There is the limitation in the freezer that it cannot handle uninterruptible
tasks and that's all.

Now, this is not usual for user space tasks to make other user space
tasks become uninterruptible.  I'd say this is a little strange.

> > A deadlock requires that the circular wait is uninterruptible. Normal IPC
> > isn't.
> > 
> > What are you doing in the userland portions of fuse? Some kind of IPC
> > with other tasks?
> 
> Anything, writing to a file, writing to shared memory, sending things
> over the network.  There's no limit to what a filesystem daemon may
> do.  It's a perfectly ordinary unprivileged userspace process.  And
> this is a feature not a bug.
> 
> > There is a limit to which you can push kernel functionality into
> > user space.
> 
> Limiting what a userspace filesystem can do would defeat the whole
> purpose of the bloody thing.  This is not negotiable ;)

Which doesn't change the fact that FUSE _is_ special, because it adds
dependencies between processed that were not present before.

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Miklos Szeredi]

> > Limiting what a userspace filesystem can do would defeat the whole
> > purpose of the bloody thing.  This is not negotiable ;)
> 
> Which doesn't change the fact that FUSE _is_ special, because it adds
> dependencies between processed that were not present before.

OK, fuse is special.  So is the userspace driver framework (UIO)
proposed by Greg KH and co.  Now what can be done about these?

 - making them not-special is not an option due to the established
   interfaces, which don't allow restartability.

 - fixing the freezer is pretty much impossible because the
   dependencies between the tasks cannot be known.

 - removing the freezer and fixing the drivers seems workable, we
   already have a prototype in the form of the powermac architecture.

It seems pretty clear cut.  Whining about how much problems this will
cause won't get us nearer to a solution.

Miklos

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Rafael J. Wysocki]

On Thursday, 5 July 2007 13:54, Miklos Szeredi wrote:
> > > Limiting what a userspace filesystem can do would defeat the whole
> > > purpose of the bloody thing.  This is not negotiable ;)
> > 
> > Which doesn't change the fact that FUSE _is_ special, because it adds
> > dependencies between processed that were not present before.
> 
> OK, fuse is special.  So is the userspace driver framework (UIO)
> proposed by Greg KH and co.  Now what can be done about these?
> 
>  - making them not-special is not an option due to the established
>    interfaces, which don't allow restartability.
> 
>  - fixing the freezer is pretty much impossible because the
>    dependencies between the tasks cannot be known.
> 
>  - removing the freezer and fixing the drivers seems workable, we
>    already have a prototype in the form of the powermac architecture.
> 
> It seems pretty clear cut.  Whining about how much problems this will
> cause won't get us nearer to a solution.

Yes, that's pretty clear cut, but we should start from fixing the drivers. :-)

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Oliver Neukum]

Am Donnerstag, 5. Juli 2007 schrieb Rafael J. Wysocki:
> > It seems pretty clear cut.  Whining about how much problems this will
> > cause won't get us nearer to a solution.
> 
> Yes, that's pretty clear cut, but we should start from fixing the drivers. :-)

If, at a minimum, we can determine that we can STD without a freezer.
It makes no sense to invest a lot of work to face the same problem
again with STD.

	Regards
		Oliver

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Matthew Garrett]

On Thu, Jul 05, 2007 at 03:28:33PM +0200, Oliver Neukum wrote:

> If, at a minimum, we can determine that we can STD without a freezer.
> It makes no sense to invest a lot of work to face the same problem
> again with STD.

I have a model for STD that avoids the need to freeze the entirity of 
userspace, but I need to find some more time to flesh it out.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Rafael J. Wysocki]

On Thursday, 5 July 2007 15:28, Oliver Neukum wrote:
> Am Donnerstag, 5. Juli 2007 schrieb Rafael J. Wysocki:
> > > It seems pretty clear cut.  Whining about how much problems this will
> > > cause won't get us nearer to a solution.
> > 
> > Yes, that's pretty clear cut, but we should start from fixing the drivers. :-)
> 
> If, at a minimum, we can determine that we can STD without a freezer.

No, we can't.

> It makes no sense to invest a lot of work to face the same problem
> again with STD.

Arguably, it does make sense, because for many platforms the hibernation
is irrelevant and we're going to separate the suspend and hibernation
frameworks anyway.

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Rafael J. Wysocki]

On Thursday, 5 July 2007 15:46, Matthew Garrett wrote:
> On Thu, Jul 05, 2007 at 03:28:33PM +0200, Oliver Neukum wrote:
> 
> > If, at a minimum, we can determine that we can STD without a freezer.
> > It makes no sense to invest a lot of work to face the same problem
> > again with STD.
> 
> I have a model for STD that avoids the need to freeze the entirity of 
> userspace, but I need to find some more time to flesh it out.

You can just describe it, as far as I'm concerned. :-)

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Matthew Garrett]

On Thu, Jul 05, 2007 at 04:09:24PM +0200, Rafael J. Wysocki wrote:
> On Thursday, 5 July 2007 15:46, Matthew Garrett wrote:
> > I have a model for STD that avoids the need to freeze the entirity of 
> > userspace, but I need to find some more time to flesh it out.
> 
> You can just describe it, as far as I'm concerned. :-)

The basic model is that nobody's really described a use-case where we 
actually care about restoring system state. What people want is to be 
able to restore application state. So, arguably, what we want isn't to 
save the entire kernel state and application state in one go because we 
can reconstruct a huge amount of that afterwards.

This isn't too much of a problem. All we actually need to be able to do 
is to atomically dump process state (which requires the freezer, but 
doesn't require freezing the entire system), shut down, get the system 
back into approximately the correct state (remount filesystems, start X, 
whatever) and then restore the processes.

Now, obviously, there's actually quite a lot of complexity here that I'm 
neatly eliding :) The biggest issue is restoring hardware state. We'd 
require quite a different model to the existing one, but I think there 
are arguments there for it being helpful anywy. Keeping state in the 
midlevels rather than the low-level drivers would give us much more 
ability to deal with hardware issues, and potentially allow the 
replacement of faulty hardware without userspace caring (freeze your 
mission-critical application, hotplug the network card, let the kernel 
restore state and resume it)

There's other advantages to this. As long as the kernel hasn't changed 
too much it would be possible to restore userspace across kernel 
security upgrades. You end up saving less to disk so performance should 
be better. Touching filesystems between suspend and resume doesn't 
result in the entire world ending.

I've mocked up a basic implementation using cryopid, but it's somewhat 
limited by the lack of support for sockets. I'd like to move more of the 
smarts into the kernel (Hurray, checkpointing!) and then see how much 
hardware support ends up horifically broken.
-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Ray Lee]

On 7/5/07, Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> On Thu, Jul 05, 2007 at 04:09:24PM +0200, Rafael J. Wysocki wrote:
> > On Thursday, 5 July 2007 15:46, Matthew Garrett wrote:
> > > I have a model for STD that avoids the need to freeze the entirity of
> > > userspace, but I need to find some more time to flesh it out.
> >
> > You can just describe it, as far as I'm concerned. :-)
>
> The basic model is that nobody's really described a use-case where we
> actually care about restoring system state. What people want is to be
> able to restore application state.

Hmm, careful. There are a bunch of people who use suspend2 exactly
because it saves and restores the page cache, leaving the system in a
usable state without waiting for the universe to swap back in from
disk. It makes a big difference on older laptops with slow drives.
While the other advantages you list for process cryogenics are pretty
neat, let's remember that the 99% use case for STD is laptops.

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Rafael J. Wysocki]

On Thursday, 5 July 2007 16:23, Matthew Garrett wrote:
> On Thu, Jul 05, 2007 at 04:09:24PM +0200, Rafael J. Wysocki wrote:
> > On Thursday, 5 July 2007 15:46, Matthew Garrett wrote:
> > > I have a model for STD that avoids the need to freeze the entirity of 
> > > userspace, but I need to find some more time to flesh it out.
> > 
> > You can just describe it, as far as I'm concerned. :-)
> 
> The basic model is that nobody's really described a use-case where we 
> actually care about restoring system state. What people want is to be 
> able to restore application state. So, arguably, what we want isn't to 
> save the entire kernel state and application state in one go because we 
> can reconstruct a huge amount of that afterwards.

Hmm, I think that will take more time than just restoring the entire system
state.

> This isn't too much of a problem. All we actually need to be able to do 
> is to atomically dump process state (which requires the freezer, but 
> doesn't require freezing the entire system),

Once you've frozen processes, the freezing of the rest of the system is
pretty straightforward.  Currently, we do a bit too much for that, because we
suspend devices instead of just quiescing them before creating the image,
bu that's going to change (I hope).

> shut down, get the system back into approximately the correct state (remount
> filesystems, start X, whatever) and then restore the processes.
>
> Now, obviously, there's actually quite a lot of complexity here that I'm 
> neatly eliding :) The biggest issue is restoring hardware state. We'd 
> require quite a different model to the existing one, but I think there 
> are arguments there for it being helpful anywy. Keeping state in the 
> midlevels rather than the low-level drivers would give us much more 
> ability to deal with hardware issues, and potentially allow the 
> replacement of faulty hardware without userspace caring (freeze your 
> mission-critical application, hotplug the network card, let the kernel 
> restore state and resume it)

Sounds neat, but what about the processes that depend on the hardware
(like hal)?

> There's other advantages to this. As long as the kernel hasn't changed 
> too much it would be possible to restore userspace across kernel 
> security upgrades. You end up saving less to disk so performance should 
> be better.

Well, not that much less. :-)

> Touching filesystems between suspend and resume doesn't result in the entire
> world ending.

Yes, that would be an advantege.

> I've mocked up a basic implementation using cryopid, but it's somewhat 
> limited by the lack of support for sockets. I'd like to move more of the 
> smarts into the kernel (Hurray, checkpointing!) and then see how much 
> hardware support ends up horifically broken.

There's one more thing, I'm not sure if that's possible to separate the kernel
state from the processes state entirely (think shared memory, LRU lists,
situations in which the application has been frozen while waiting for an
event in the kernel space etc.).

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Matthew Garrett]

On Thu, Jul 05, 2007 at 07:46:01AM -0700, Ray Lee wrote:

> Hmm, careful. There are a bunch of people who use suspend2 exactly
> because it saves and restores the page cache, leaving the system in a
> usable state without waiting for the universe to swap back in from
> disk. It makes a big difference on older laptops with slow drives.
> While the other advantages you list for process cryogenics are pretty
> neat, let's remember that the 99% use case for STD is laptops.

Saving the processes means that you're implicitly saving the interesting 
chunks of the page cache. Removing the need for the atomic copy saves 
you from pushing a pile of stuff out to swap in the first place.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Jeremy Maitin-Shepard]

Matthew Garrett <mjg59@srcf.ucam.org> writes:

> On Thu, Jul 05, 2007 at 04:09:24PM +0200, Rafael J. Wysocki wrote:
>> On Thursday, 5 July 2007 15:46, Matthew Garrett wrote:
>> > I have a model for STD that avoids the need to freeze the entirity of 
>> > userspace, but I need to find some more time to flesh it out.
>> 
>> You can just describe it, as far as I'm concerned. :-)

[snip: new hibernate idea]

I think my kexec-based hibernate idea is simpler and more feasible than
this approach, and also avoids the freezer.

-- 
Jeremy Maitin-Shepard

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Daniel Pittman]

Matthew Garrett <mjg59@srcf.ucam.org> writes:
> On Thu, Jul 05, 2007 at 04:09:24PM +0200, Rafael J. Wysocki wrote:
>> On Thursday, 5 July 2007 15:46, Matthew Garrett wrote:
>> > I have a model for STD that avoids the need to freeze the entirity of 
>> > userspace, but I need to find some more time to flesh it out.
>> 
>> You can just describe it, as far as I'm concerned. :-)
>
> The basic model is that nobody's really described a use-case where we 
> actually care about restoring system state. What people want is to be 
> able to restore application state. So, arguably, what we want isn't to 
> save the entire kernel state and application state in one go because we 
> can reconstruct a huge amount of that afterwards.

[...]

> I've mocked up a basic implementation using cryopid, but it's somewhat
> limited by the lack of support for sockets. I'd like to move more of
> the smarts into the kernel (Hurray, checkpointing!) and then see how
> much hardware support ends up horifically broken.

You might want to look at the checkpoint / migration support in the
OpenVZ kernel in relation to this.  That does work to dump the state of
a running "virtual environment" complete with applications to disk, move
it to another running kernel and restore the content.

That might, perhaps, help with the prototype of this?

Regards,
	Daniel

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Benjamin Herrenschmidt]

> There is that.
> 
> OK, bite the bullet. Tasks involved in fuse are special. Give them a flag
> and teach the freezer to put them on ice only after all other task are
> frozen. In a way they are kernel, there's no use denying that.

Yet another ugly hack to work around the fact that the freezer cannot
work reliably ... yuck

Why not spend that energy fixing drivers to properly block requests
instead ?

Ben.

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Rafael J. Wysocki]

On Friday, 6 July 2007 00:38, Benjamin Herrenschmidt wrote:
> 
> > There is that.
> > 
> > OK, bite the bullet. Tasks involved in fuse are special. Give them a flag
> > and teach the freezer to put them on ice only after all other task are
> > frozen. In a way they are kernel, there's no use denying that.
> 
> Yet another ugly hack to work around the fact that the freezer cannot
> work reliably ... yuck
> 
> Why not spend that energy fixing drivers to properly block requests
> instead ?

Because it's more difficult? :-)

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Oliver Neukum]

Am Freitag, 6. Juli 2007 schrieb Benjamin Herrenschmidt:
> 
> > There is that.
> > 
> > OK, bite the bullet. Tasks involved in fuse are special. Give them a flag
> > and teach the freezer to put them on ice only after all other task are
> > frozen. In a way they are kernel, there's no use denying that.
> 
> Yet another ugly hack to work around the fact that the freezer cannot
> work reliably ... yuck
> 
> Why not spend that energy fixing drivers to properly block requests
> instead ?

Because we will be unable to escape that job. Let's assume that
we remove the freezer from the STR path. The next complaint would
be that we cannot do STD with fuse. "Then don't do that" would not
be taken kindly as answer.

So we would be under pressure to remove the freezer from STD, too,
or find a way to make the freezer work. If we'd have to make the freezer
work anyway, we can do it right now.

	Regards
		Oliver

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Pavel Machek]

On Thu 2007-07-05 10:17:17, Miklos Szeredi wrote:
> > > > I have discussed the benefits elsewhere.  As for the deadlocks -- do 
> > > > you still observe them if you use the version of the freezer which 
> > > > doesn't freeze kernel threads?
> > > 
> > > In general the only way to guarantee there are no deadlocks is to
> > > construct the graph of dependencies between tasks.  Those dependencies
> > > are not in practice observable from outside the tasks, so it is
> > > virtually impossible to construct the graph.
> > 
> > In which way can user space tasks depend on each other in a way that
> > allows a them members of that cycle to be in uninterruptible sleep?
> 
>  - process A calls rename() on a fuse fs
>  - process B, the fuse server, starts to process the rename request
>  - process B is frozen before it can reply
> 
> Now process A is unfreezable.  We cannot make rename() restartable,
> hence it cannot be interruptible.

Yes, we are claiming fuse is very special in this regard, and perhaps
even broken.

Let's see. If I SIGSTOP the fuse server, I can get unrelated tasks
unkillable (even for SIGKILL!) forever. That's very special, and maybe
even a FUSE bug. And that is also what makes FUSE special
w.r.t. s2ram.

So no, you can't claim "FUSE is just IPC". It is very special IPC.
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Miklos Szeredi]

> > > > > I have discussed the benefits elsewhere.  As for the deadlocks -- do 
> > > > > you still observe them if you use the version of the freezer which 
> > > > > doesn't freeze kernel threads?
> > > > 
> > > > In general the only way to guarantee there are no deadlocks is to
> > > > construct the graph of dependencies between tasks.  Those dependencies
> > > > are not in practice observable from outside the tasks, so it is
> > > > virtually impossible to construct the graph.
> > > 
> > > In which way can user space tasks depend on each other in a way that
> > > allows a them members of that cycle to be in uninterruptible sleep?
> > 
> >  - process A calls rename() on a fuse fs
> >  - process B, the fuse server, starts to process the rename request
> >  - process B is frozen before it can reply
> > 
> > Now process A is unfreezable.  We cannot make rename() restartable,
> > hence it cannot be interruptible.
> 
> Yes, we are claiming fuse is very special in this regard, and perhaps
> even broken.
> 
> Let's see. If I SIGSTOP the fuse server, I can get unrelated tasks
> unkillable (even for SIGKILL!) forever.

Actually fuse allows SIGKILL, because it's always fatal, and the
syscall may not be restarted.

> That's very special, and maybe even a FUSE bug. And that is also
> what makes FUSE special w.r.t. s2ram.

What makes fuse special is that some file operations are synchronous
and non-restartable.  That's just how the UNIX filesystem API works
and is hardly a bug in fuse.

> So no, you can't claim "FUSE is just IPC". It is very special IPC.

I did say it's special.  Sure, it has some "interesting" properties,
and with a bit of malice you can do very ugly things with it.  If you
are interested, read Documentation/filesystems/fuse.txt, especially
the "Tricky deadlock" section ;)

Miklos

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Pavel Machek]

Hi!

> > > > In which way can user space tasks depend on each other in a way that
> > > > allows a them members of that cycle to be in uninterruptible sleep?
> > > 
> > >  - process A calls rename() on a fuse fs
> > >  - process B, the fuse server, starts to process the rename request
> > >  - process B is frozen before it can reply
> > > 
> > > Now process A is unfreezable.  We cannot make rename() restartable,
> > > hence it cannot be interruptible.
> > 
> > Yes, we are claiming fuse is very special in this regard, and perhaps
> > even broken.
> > 
> > Let's see. If I SIGSTOP the fuse server, I can get unrelated tasks
> > unkillable (even for SIGKILL!) forever.
> 
> Actually fuse allows SIGKILL, because it's always fatal, and the
> syscall may not be restarted.

I think you want to stick try_to_freeze() at the same places where you
do SIGKILL handling. That should solve the 'syslogd is unfreezeable'
problem.

Plus, it would be nice to find out where suspend/hibernation is
triggering fuse activity. We can then decide where to fix it -- in
fuse or in suspend parts. You said sys_sync is not implemented... so
where is the problem?

> > That's very special, and maybe even a FUSE bug. And that is also
> > what makes FUSE special w.r.t. s2ram.
> 
> What makes fuse special is that some file operations are synchronous
> and non-restartable.  That's just how the UNIX filesystem API works
> and is hardly a bug in fuse.

Well, unix is not plan9, and maybe userland filesystems are impossible
in unix. But that is hardly a bug in unix :-).

							Pavel

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

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Rafael J. Wysocki]

On Thursday, 5 July 2007 11:31, Miklos Szeredi wrote:
> > > > > > I have discussed the benefits elsewhere.  As for the deadlocks -- do 
> > > > > > you still observe them if you use the version of the freezer which 
> > > > > > doesn't freeze kernel threads?
> > > > > 
> > > > > In general the only way to guarantee there are no deadlocks is to
> > > > > construct the graph of dependencies between tasks.  Those dependencies
> > > > > are not in practice observable from outside the tasks, so it is
> > > > > virtually impossible to construct the graph.
> > > > 
> > > > In which way can user space tasks depend on each other in a way that
> > > > allows a them members of that cycle to be in uninterruptible sleep?
> > > 
> > >  - process A calls rename() on a fuse fs
> > >  - process B, the fuse server, starts to process the rename request
> > >  - process B is frozen before it can reply
> > > 
> > > Now process A is unfreezable.  We cannot make rename() restartable,
> > > hence it cannot be interruptible.
> > 
> > Yes, we are claiming fuse is very special in this regard, and perhaps
> > even broken.
> > 
> > Let's see. If I SIGSTOP the fuse server, I can get unrelated tasks
> > unkillable (even for SIGKILL!) forever.
> 
> Actually fuse allows SIGKILL, because it's always fatal, and the
> syscall may not be restarted.
> 
> > That's very special, and maybe even a FUSE bug. And that is also
> > what makes FUSE special w.r.t. s2ram.
> 
> What makes fuse special is that some file operations are synchronous
> and non-restartable.  That's just how the UNIX filesystem API works
> and is hardly a bug in fuse.
> 
> > So no, you can't claim "FUSE is just IPC". It is very special IPC.
> 
> I did say it's special.  Sure, it has some "interesting" properties,
> and with a bit of malice you can do very ugly things with it.  If you
> are interested, read Documentation/filesystems/fuse.txt, especially
> the "Tricky deadlock" section ;)

Very well.

Don't you think, however, that it can be modified a little to play well,
for example, with the freezer?

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Miklos Szeredi]

> > Actually fuse allows SIGKILL, because it's always fatal, and the
> > syscall may not be restarted.
> 
> I think you want to stick try_to_freeze() at the same places where you
> do SIGKILL handling. That should solve the 'syslogd is unfreezeable'
> problem.

I could, but it would not solve the general problem.  Namely, that the
presence of fuse imposes a certain ordering in which userspace tasks
have to be frozen.  And it is not possible to know this ordering.

And even if the ordering were solved, the freezer would still not work
if the filesystem is not responding due to external events, such as a
lost network (this affects NFS, CIFS, whatever just the same as fuse).

> Plus, it would be nice to find out where suspend/hibernation is
> triggering fuse activity. We can then decide where to fix it -- in
> fuse or in suspend parts. You said sys_sync is not implemented... so
> where is the problem?

I cannot say without having a sysrq-t of the situation.

> > > That's very special, and maybe even a FUSE bug. And that is also
> > > what makes FUSE special w.r.t. s2ram.
> > 
> > What makes fuse special is that some file operations are synchronous
> > and non-restartable.  That's just how the UNIX filesystem API works
> > and is hardly a bug in fuse.
> 
> Well, unix is not plan9, and maybe userland filesystems are impossible
> in unix. But that is hardly a bug in unix :-).

I'd rather say, reliable suspend to ram is impossible in the presense
of userspace filesystems, iff people are too lazy to fix the suspend
framework and the drivers to work without the freezer.

This has nothing to do with unix, if plan9 would need to support STR
or hibernate, it would face exactly the same problems.

Miklos

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Rafael J. Wysocki]

On Thursday, 5 July 2007 14:07, Miklos Szeredi wrote:
> > > Actually fuse allows SIGKILL, because it's always fatal, and the
> > > syscall may not be restarted.
> > 
> > I think you want to stick try_to_freeze() at the same places where you
> > do SIGKILL handling. That should solve the 'syslogd is unfreezeable'
> > problem.
> 
> I could, but it would not solve the general problem.  Namely, that the
> presence of fuse imposes a certain ordering in which userspace tasks
> have to be frozen.  And it is not possible to know this ordering.
> 
> And even if the ordering were solved, the freezer would still not work
> if the filesystem is not responding due to external events, such as a
> lost network (this affects NFS, CIFS, whatever just the same as fuse).
> 
> > Plus, it would be nice to find out where suspend/hibernation is
> > triggering fuse activity. We can then decide where to fix it -- in
> > fuse or in suspend parts. You said sys_sync is not implemented... so
> > where is the problem?
> 
> I cannot say without having a sysrq-t of the situation.
> 
> > > > That's very special, and maybe even a FUSE bug. And that is also
> > > > what makes FUSE special w.r.t. s2ram.
> > > 
> > > What makes fuse special is that some file operations are synchronous
> > > and non-restartable.  That's just how the UNIX filesystem API works
> > > and is hardly a bug in fuse.
> > 
> > Well, unix is not plan9, and maybe userland filesystems are impossible
> > in unix. But that is hardly a bug in unix :-).
> 
> I'd rather say, reliable suspend to ram is impossible in the presense
> of userspace filesystems, iff people are too lazy to fix the suspend
> framework and the drivers to work without the freezer.

Well, I don't think it has anything to do with laziness or things like that.
Rather, people have limited time and this requires some knowledge about
drivers you're modifying, so not many people really can do that.

Also, you've already assumed that there's no other solution, but I'm not
convinced about that yet.

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Oliver Neukum]

Am Donnerstag, 5. Juli 2007 schrieb Miklos Szeredi:
> > > Actually fuse allows SIGKILL, because it's always fatal, and the
> > > syscall may not be restarted.
> > 
> > I think you want to stick try_to_freeze() at the same places where you
> > do SIGKILL handling. That should solve the 'syslogd is unfreezeable'
> > problem.
> 
> I could, but it would not solve the general problem.  Namely, that the
> presence of fuse imposes a certain ordering in which userspace tasks
> have to be frozen.  And it is not possible to know this ordering.

Actually, why do you need this? There is no absolute need that you
finish the request. You must either finish the request or let yourself
be frozen.

A quick look through fuse reveals principally request_wait_answer()
And maybe a few other places. Is there some hidden reason you cannot
handle being frozen here?

	Regards
		Oliver

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Miklos Szeredi]

> Am Donnerstag, 5. Juli 2007 schrieb Miklos Szeredi:
> > > > Actually fuse allows SIGKILL, because it's always fatal, and the
> > > > syscall may not be restarted.
> > > 
> > > I think you want to stick try_to_freeze() at the same places where you
> > > do SIGKILL handling. That should solve the 'syslogd is unfreezeable'
> > > problem.
> > 
> > I could, but it would not solve the general problem.  Namely, that the
> > presence of fuse imposes a certain ordering in which userspace tasks
> > have to be frozen.  And it is not possible to know this ordering.
> 
> Actually, why do you need this? There is no absolute need that you
> finish the request. You must either finish the request or let yourself
> be frozen.
> 
> A quick look through fuse reveals principally request_wait_answer()
> And maybe a few other places. Is there some hidden reason you cannot
> handle being frozen here?

Yes, fuse could handle being frozen there.  However that would only
solve part of the problem: an operation waiting for a reply could be
holding a VFS mutex and some other task may be blocked on that mutex.

How would you solve freezing those tasks?

Miklos

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Rafael J. Wysocki]

On Thursday, 5 July 2007 21:44, Miklos Szeredi wrote:
> > Am Donnerstag, 5. Juli 2007 schrieb Miklos Szeredi:
> > > > > Actually fuse allows SIGKILL, because it's always fatal, and the
> > > > > syscall may not be restarted.
> > > > 
> > > > I think you want to stick try_to_freeze() at the same places where you
> > > > do SIGKILL handling. That should solve the 'syslogd is unfreezeable'
> > > > problem.
> > > 
> > > I could, but it would not solve the general problem.  Namely, that the
> > > presence of fuse imposes a certain ordering in which userspace tasks
> > > have to be frozen.  And it is not possible to know this ordering.
> > 
> > Actually, why do you need this? There is no absolute need that you
> > finish the request. You must either finish the request or let yourself
> > be frozen.
> > 
> > A quick look through fuse reveals principally request_wait_answer()
> > And maybe a few other places. Is there some hidden reason you cannot
> > handle being frozen here?
> 
> Yes, fuse could handle being frozen there.  However that would only
> solve part of the problem: an operation waiting for a reply could be
> holding a VFS mutex and some other task may be blocked on that mutex.
> 
> How would you solve freezing those tasks?

How probable is this situation?

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Oliver Neukum]

Am Donnerstag, 5. Juli 2007 schrieb Miklos Szeredi:
> > Am Donnerstag, 5. Juli 2007 schrieb Miklos Szeredi:
> > > > > Actually fuse allows SIGKILL, because it's always fatal, and the
> > > > > syscall may not be restarted.
> > > > 
> > > > I think you want to stick try_to_freeze() at the same places where you
> > > > do SIGKILL handling. That should solve the 'syslogd is unfreezeable'
> > > > problem.
> > > 
> > > I could, but it would not solve the general problem.  Namely, that the
> > > presence of fuse imposes a certain ordering in which userspace tasks
> > > have to be frozen.  And it is not possible to know this ordering.
> > 
> > Actually, why do you need this? There is no absolute need that you
> > finish the request. You must either finish the request or let yourself
> > be frozen.
> > 
> > A quick look through fuse reveals principally request_wait_answer()
> > And maybe a few other places. Is there some hidden reason you cannot
> > handle being frozen here?
> 
> Yes, fuse could handle being frozen there.  However that would only
> solve part of the problem: an operation waiting for a reply could be
> holding a VFS mutex and some other task may be blocked on that mutex.
> 
> How would you solve freezing those tasks?

OK, you made me reach for literatur on theoretical computer science.

IMHO the range of actions a fuse server is inherently limited.
You must never ever block on a lock one of your clients is holding. In
this case the limitation is not influenced by the freezer.

The freezer introduces a further limitation in that the server can freeze
before the client, which must not be. You can prevent that by freezing
the servers last.

In principle you might have dependencies between servers and you won't
catch that, true. You won't catch servers blocking on IPC, but you are
balancing on the edge of deadlock with fuse anyway.

	Regards
		Oliver

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Miklos Szeredi]

> > Yes, fuse could handle being frozen there.  However that would only
> > solve part of the problem: an operation waiting for a reply could be
> > holding a VFS mutex and some other task may be blocked on that mutex.
> > 
> > How would you solve freezing those tasks?
> 
> OK, you made me reach for literatur on theoretical computer science.
> 
> IMHO the range of actions a fuse server is inherently limited.
> You must never ever block on a lock one of your clients is holding. In
> this case the limitation is not influenced by the freezer.

Obviously.  But I wasn't about the server trying to acquire a lock
held by a client.  I was talking about a client trying to acquire a
lock held by _another_ client.

If this coincides with the server (or some other task which the server
is depending on) being frozen before the clients, the freezer has a
problem.

> The freezer introduces a further limitation in that the server can freeze
> before the client, which must not be. You can prevent that by freezing
> the servers last.
> 
> In principle you might have dependencies between servers and you won't
> catch that, true. You won't catch servers blocking on IPC, but you are
> balancing on the edge of deadlock with fuse anyway.

Huh?

Miklos

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Oliver Neukum]

Am Donnerstag, 5. Juli 2007 schrieb Miklos Szeredi:
> > > Yes, fuse could handle being frozen there.  However that would only
> > > solve part of the problem: an operation waiting for a reply could be
> > > holding a VFS mutex and some other task may be blocked on that mutex.
> > > 
> > > How would you solve freezing those tasks?
> > 
> > OK, you made me reach for literatur on theoretical computer science.
> > 
> > IMHO the range of actions a fuse server is inherently limited.
> > You must never ever block on a lock one of your clients is holding. In
> > this case the limitation is not influenced by the freezer.
> 
> Obviously.  But I wasn't about the server trying to acquire a lock
> held by a client.  I was talking about a client trying to acquire a
> lock held by _another_ client.
> 
> If this coincides with the server (or some other task which the server
> is depending on) being frozen before the clients, the freezer has a
> problem.

True, but that case can only happen if servers are frozen before clients.
You don't need a full dependency graph. A simple set sequence of two
classes of tasks will do.

	Regards
		Oliver

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Oliver Neukum]

Am Donnerstag, 5. Juli 2007 schrieb Oliver Neukum:
> Am Donnerstag, 5. Juli 2007 schrieb Miklos Szeredi:
> > > > Yes, fuse could handle being frozen there.  However that would only
> > > > solve part of the problem: an operation waiting for a reply could be
> > > > holding a VFS mutex and some other task may be blocked on that mutex.
> > > > 
> > > > How would you solve freezing those tasks?
> > > 
> > > OK, you made me reach for literatur on theoretical computer science.
> > > 
> > > IMHO the range of actions a fuse server is inherently limited.
> > > You must never ever block on a lock one of your clients is holding. In
> > > this case the limitation is not influenced by the freezer.
> > 
> > Obviously.  But I wasn't about the server trying to acquire a lock
> > held by a client.  I was talking about a client trying to acquire a
> > lock held by _another_ client.
> > 
> > If this coincides with the server (or some other task which the server
> > is depending on) being frozen before the clients, the freezer has a
> > problem.
> 
> True, but that case can only happen if servers are frozen before clients.
> You don't need a full dependency graph. A simple set sequence of two
> classes of tasks will do.

Any replying to myself. A deadlock here is not fatal. You can and will
timeout in the freezer and can try again.

	REegards
		Oliver

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Alan Stern]

On Thu, 5 Jul 2007, Oliver Neukum wrote:

> > Obviously.  But I wasn't about the server trying to acquire a lock
> > held by a client.  I was talking about a client trying to acquire a
> > lock held by _another_ client.
> > 
> > If this coincides with the server (or some other task which the server
> > is depending on) being frozen before the clients, the freezer has a
> > problem.
> 
> True, but that case can only happen if servers are frozen before clients.
> You don't need a full dependency graph. A simple set sequence of two
> classes of tasks will do.

Just to make things more complicated...  Since a server isn't
restricted in what it can do, what happens when one server depends on
another server?

Alan Stern

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Miklos Szeredi]

> > > > Yes, fuse could handle being frozen there.  However that would only
> > > > solve part of the problem: an operation waiting for a reply could be
> > > > holding a VFS mutex and some other task may be blocked on that mutex.
> > > > 
> > > > How would you solve freezing those tasks?
> > > 
> > > OK, you made me reach for literatur on theoretical computer science.
> > > 
> > > IMHO the range of actions a fuse server is inherently limited.
> > > You must never ever block on a lock one of your clients is holding. In
> > > this case the limitation is not influenced by the freezer.
> > 
> > Obviously.  But I wasn't about the server trying to acquire a lock
> > held by a client.  I was talking about a client trying to acquire a
> > lock held by _another_ client.
> > 
> > If this coincides with the server (or some other task which the server
> > is depending on) being frozen before the clients, the freezer has a
> > problem.
> 
> True, but that case can only happen if servers are frozen before clients.
> You don't need a full dependency graph. A simple set sequence of two
> classes of tasks will do.

Umm, let's take sshfs, it has a separate "reply processing thread",
that reads replies from the sftp server and then wakes up the relevant
request thread.  The reply processing thread has no direct contact
with the fuse kernel module.  How is freezer supposed to know that
that task in fact also belongs to the server and needs to be kept from
freezing?

Given, it's in the same thread group, but using that is a rather weak
heuristic, as it could easily be a separate process, and likely in
some filesystems it is.

I fear, that your efforts to "save" the freezer are in vain.  It is
already moderately hackish with that PF_FREEZER_SKIP and the kernel
dotted randomly with try_to_freeze() calls, but adding bandaids to try
to order freezing userspace processes in the right order would just
make it a horrible mess.

Miklos

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Miklos Szeredi]

> > > > > > Actually fuse allows SIGKILL, because it's always fatal, and the
> > > > > > syscall may not be restarted.
> > > > > 
> > > > > I think you want to stick try_to_freeze() at the same places where you
> > > > > do SIGKILL handling. That should solve the 'syslogd is unfreezeable'
> > > > > problem.
> > > > 
> > > > I could, but it would not solve the general problem.  Namely, that the
> > > > presence of fuse imposes a certain ordering in which userspace tasks
> > > > have to be frozen.  And it is not possible to know this ordering.
> > > 
> > > Actually, why do you need this? There is no absolute need that you
> > > finish the request. You must either finish the request or let yourself
> > > be frozen.
> > > 
> > > A quick look through fuse reveals principally request_wait_answer()
> > > And maybe a few other places. Is there some hidden reason you cannot
> > > handle being frozen here?
> > 
> > Yes, fuse could handle being frozen there.  However that would only
> > solve part of the problem: an operation waiting for a reply could be
> > holding a VFS mutex and some other task may be blocked on that mutex.
> > 
> > How would you solve freezing those tasks?
> 
> How probable is this situation?

I guess it depends on usage patterns.

I don't remember seeing any such cases, even though I have a permanent
fuse mount, and I suspend regularly.  But the fs is probably totally
idle during suspend in my case.

But some people use fuse as a home or root filesystem and in those
cases it can become quite likely to cause problems.

Miklos

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Rafael J. Wysocki]

On Thursday, 5 July 2007 22:38, Miklos Szeredi wrote:
> > > > > > > Actually fuse allows SIGKILL, because it's always fatal, and the
> > > > > > > syscall may not be restarted.
> > > > > > 
> > > > > > I think you want to stick try_to_freeze() at the same places where you
> > > > > > do SIGKILL handling. That should solve the 'syslogd is unfreezeable'
> > > > > > problem.
> > > > > 
> > > > > I could, but it would not solve the general problem.  Namely, that the
> > > > > presence of fuse imposes a certain ordering in which userspace tasks
> > > > > have to be frozen.  And it is not possible to know this ordering.
> > > > 
> > > > Actually, why do you need this? There is no absolute need that you
> > > > finish the request. You must either finish the request or let yourself
> > > > be frozen.
> > > > 
> > > > A quick look through fuse reveals principally request_wait_answer()
> > > > And maybe a few other places. Is there some hidden reason you cannot
> > > > handle being frozen here?
> > > 
> > > Yes, fuse could handle being frozen there.  However that would only
> > > solve part of the problem: an operation waiting for a reply could be
> > > holding a VFS mutex and some other task may be blocked on that mutex.
> > > 
> > > How would you solve freezing those tasks?
> > 
> > How probable is this situation?
> 
> I guess it depends on usage patterns.
> 
> I don't remember seeing any such cases, even though I have a permanent
> fuse mount, and I suspend regularly.  But the fs is probably totally
> idle during suspend in my case.
> 
> But some people use fuse as a home or root filesystem and in those
> cases it can become quite likely to cause problems.

That means we need to prevent the freezer from sending freeze requests FUSE's
filesystem servers too early.

What about this (more or less):
1) We add a list of userland processes to the freezer that should not be frozen
in the first phase (ie. during the freezing of user space).  They will be frozen
anyway along with the freezable kernel threads.
2) FUSE adds its filesystem servers to this list upon connecting to them
3) FUSE removes its filesystem servers from this list upon disconnecting

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Benjamin Herrenschmidt]

> Yes, fuse could handle being frozen there.  However that would only
> solve part of the problem: an operation waiting for a reply could be
> holding a VFS mutex and some other task may be blocked on that mutex.
> 
> How would you solve freezing those tasks?

That task is implicitely frozen... but the kernel doesn't know it and
thus the freezer timeouts or fails or deadlocks or whatever.

The freezer could be made to ignore tasks that are sleeping in the
kernel assuming that if they go out of it, they'll ultimately reach
do_signal and freeze, but that means they can potentially still issues
IOs which is what the freezer tries to avoid ...

Or the kernel could start tracking dependencies, but then, good luck
implementing that crap.

At the end of the day, I stand my ground, the freezer cannot be made
reliable without massive infrastructure changes or giving up on very
useful features such as fuse among others. Besides, it only partially
"hides" the problem of requests going to drivers, thus it's a bad
solutions.

We would be much better off spending time fixing the drivers to properly
block requests after suspended, and that also gives for free the ability
to do dynamic runtime suspend on them.

And for "trivial" drivers where we don't care, using late_suspend to
power the chip off later when IRQs are off is an easy enough way to
solve it with very little code (though won't help with dynamic PM but
that's not necessarily an issue). No need for a freezer either way.

Ben.

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Jeremy Maitin-Shepard]

Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:

[snip]

> At the end of the day, I stand my ground, the freezer cannot be made
> reliable without massive infrastructure changes or giving up on very
> useful features such as fuse among others. Besides, it only partially
> "hides" the problem of requests going to drivers, thus it's a bad
> solutions.

I agree that the freezer absolutely should not be used for suspend to
ram ("suspend"), since it is unnecessary with properly written drivers,
which are important to have anyway.  It seems that it is indeed the
consensus that it will be phased out sooner or later.

It does seem that the current device suspend interface does not tell the
drivers enough, since as discussed, they need to know whether to merely
block if they receive a request while suspended (as should be done while
initiating a suspend to ram), or if they should wake up the device (as
should be done if a suspend to ram is not in progress).  Clearly these
two cases need to be addressed by every driver supporting suspend/resume
(but possibly indirectly if the subsystem handles it for them).

The current hibernate approach used by all of the existing
implementations for Linux seems to depend fundamentally on the freezer,
though, in order to actually save the system state.  Thus, it will still
be necessary to fix all of the issues with the freezer, or adopt an
alternate hibernate approach (which is unlikely).  Unfortunately, even
leaving kernel threads and certain drivers running after the snapshot is
taken means that the saved image isn't completely correct, and the
freezer cannot help with these issues.

[snip]

-- 
Jeremy Maitin-Shepard

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Oliver Neukum]

Am Freitag, 6. Juli 2007 schrieb Benjamin Herrenschmidt:
> > Yes, fuse could handle being frozen there.  However that would only
> > solve part of the problem: an operation waiting for a reply could be
> > holding a VFS mutex and some other task may be blocked on that mutex.
> > 
> > How would you solve freezing those tasks?
> 
> That task is implicitely frozen... but the kernel doesn't know it and
> thus the freezer timeouts or fails or deadlocks or whatever.
> 
> The freezer could be made to ignore tasks that are sleeping in the
> kernel assuming that if they go out of it, they'll ultimately reach
> do_signal and freeze, but that means they can potentially still issues
> IOs which is what the freezer tries to avoid ...
> 
> Or the kernel could start tracking dependencies, but then, good luck
> implementing that crap.

Do we need dependencies? Don't we know that fuse can deadlock only
on a limited number of locks in VFS?

	Regards
		Oliver

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Pavel Machek]

Hi!

> > > Actually fuse allows SIGKILL, because it's always fatal, and the
> > > syscall may not be restarted.
> > 
> > I think you want to stick try_to_freeze() at the same places where you
> > do SIGKILL handling. That should solve the 'syslogd is unfreezeable'
> > problem.
> 
> I could, but it would not solve the general problem.  Namely, that the
> presence of fuse imposes a certain ordering in which userspace tasks
> have to be frozen.  And it is not possible to know this ordering.

We can just wait for all fuse requests to be serviced before
proceeding further with freeze, right?

> And even if the ordering were solved, the freezer would still not work
> if the filesystem is not responding due to external events, such as a
> lost network (this affects NFS, CIFS, whatever just the same as
> fuse).

That's ok, you can't suspend if your hdd is dead, and in the same way
you can't suspend if your NFS server is dead. I agree it is ugly, but
we seem to live ok with that.

We could (and should?) handle that, probably by realizing that NFS is
not a disk and using interruptible sleep, but...

> > Plus, it would be nice to find out where suspend/hibernation is
> > triggering fuse activity. We can then decide where to fix it -- in
> > fuse or in suspend parts. You said sys_sync is not implemented... so
> > where is the problem?
> 
> I cannot say without having a sysrq-t of the situation.

Yes please. Can someone affected please produce sysrq-t?
								Pavel

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

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Miklos Szeredi]

> We can just wait for all fuse requests to be serviced before
> proceeding further with freeze, right?

Right.  Nice way to slow down or stop the suspend with an unprivileged
process.  Avoiding that sort of DoS is one of the design goals of
fuse.

Look at it this way: the task of the freezer is to stop new I/O
hitting the hardware.  But it is totally indiscriminate about what it
stops, it tries to stop _everything_ even things which have nothing to
do with hardware.

Not nice.

Miklos

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Miklos Szeredi]

> Don't you think, however, that it can be modified a little to play well,
> for example, with the freezer?

I could stick a couple of try_to_freeze()s into fuse, and that would
make suspend failure less likely.  But making problems less easy to
reproduce is not a good thing.

Miklos

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Rafael J. Wysocki]

On Thursday, 5 July 2007 14:24, Miklos Szeredi wrote:
> > Don't you think, however, that it can be modified a little to play well,
> > for example, with the freezer?
> 
> I could stick a couple of try_to_freeze()s into fuse, and that would
> make suspend failure less likely.  But making problems less easy to
> reproduce is not a good thing.

So, how about eliminating them?

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Miklos Szeredi]

> > > Don't you think, however, that it can be modified a little to play well,
> > > for example, with the freezer?
> > 
> > I could stick a couple of try_to_freeze()s into fuse, and that would
> > make suspend failure less likely.  But making problems less easy to
> > reproduce is not a good thing.
> 
> So, how about eliminating them?

That can't be done just within fuse, a process might be sleeping on a
VFS mutex.  Do we want to hack VFS as well?

I guess I know your answer.  But it ain't gonna work.  Suspend code
really doesn't belong in VFS, and I'm pretty sure the maintainers of
that little piece of code would agree with me on this.

Miklos

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Rafael J. Wysocki]

On Thursday, 5 July 2007 15:50, Miklos Szeredi wrote:
> > > > Don't you think, however, that it can be modified a little to play well,
> > > > for example, with the freezer?
> > > 
> > > I could stick a couple of try_to_freeze()s into fuse, and that would
> > > make suspend failure less likely.  But making problems less easy to
> > > reproduce is not a good thing.
> > 
> > So, how about eliminating them?
> 
> That can't be done just within fuse, a process might be sleeping on a
> VFS mutex.  Do we want to hack VFS as well?

No.

> I guess I know your answer.  But it ain't gonna work.  Suspend code
> really doesn't belong in VFS, and I'm pretty sure the maintainers of
> that little piece of code would agree with me on this.

Surprise, surprise.  Not that I'm scared of the VFS maintainers, though. ;-)

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Miklos Szeredi]

> > I guess I know your answer.  But it ain't gonna work.  Suspend code
> > really doesn't belong in VFS, and I'm pretty sure the maintainers of
> > that little piece of code would agree with me on this.
> 
> Surprise, surprise.  Not that I'm scared of the VFS maintainers, though. ;-)

Probably you haven't had a close encounter with them yet.  They are
pretty much the _most_ dangerous people in kernel land ;)

Miklos

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Pavel Machek]

Hi!

> > > > > > I have discussed the benefits elsewhere.  As for the deadlocks -- do 
> > > > > > you still observe them if you use the version of the freezer which 
> > > > > > doesn't freeze kernel threads?
> > > > > 
> > > > > In general the only way to guarantee there are no deadlocks is to
> > > > > construct the graph of dependencies between tasks.  Those dependencies
> > > > > are not in practice observable from outside the tasks, so it is
> > > > > virtually impossible to construct the graph.
> > > > 
> > > > In which way can user space tasks depend on each other in a way that
> > > > allows a them members of that cycle to be in uninterruptible sleep?
> > > 
> > >  - process A calls rename() on a fuse fs
> > >  - process B, the fuse server, starts to process the rename request
> > >  - process B is frozen before it can reply
> > > 
> > > Now process A is unfreezable.  We cannot make rename() restartable,
> > > hence it cannot be interruptible.
> > 
> > Yes, we are claiming fuse is very special in this regard, and perhaps
> > even broken.
> > 
> > Let's see. If I SIGSTOP the fuse server, I can get unrelated tasks
> > unkillable (even for SIGKILL!) forever.
> 
> Actually fuse allows SIGKILL, because it's always fatal, and the
> syscall may not be restarted.

Okay, and you should handle refrigerator in the same paths where you
handle SIGKILL. Just add try_to_freeze() there...
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Miklos Szeredi]

> > Actually fuse allows SIGKILL, because it's always fatal, and the
> > syscall may not be restarted.
> 
> Okay, and you should handle refrigerator in the same paths where you
> handle SIGKILL. Just add try_to_freeze() there...

It's the fourth time I'm repeating this in this thread:

Yes adding try_to_freeze() there would partially solve the probelem.

But another task can be sleeping on a mutex held by the task waiting
for the reply.  And the freezer won't be able to handle that one.

Generally, calling try_to_freeze() with mutexes held is not a good
idea.

Miklos

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Pavel Machek]

On Fri 2007-07-06 09:07:38, Miklos Szeredi wrote:
> > > Actually fuse allows SIGKILL, because it's always fatal, and the
> > > syscall may not be restarted.
> > 
> > Okay, and you should handle refrigerator in the same paths where you
> > handle SIGKILL. Just add try_to_freeze() there...
> 
> It's the fourth time I'm repeating this in this thread:
> 
> Yes adding try_to_freeze() there would partially solve the probelem.
> 
> But another task can be sleeping on a mutex held by the task waiting
> for the reply.  And the freezer won't be able to handle that one.
> 
> Generally, calling try_to_freeze() with mutexes held is not a good
> idea.

Agreed, calling try_to_freeze() with mutex held is no-no, and it is
even documented somewhere.
								Pavel

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

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Rafael J. Wysocki]

On Friday, 6 July 2007 00:04, Pavel Machek wrote:
> Hi!
> 
> > > > > > > I have discussed the benefits elsewhere.  As for the deadlocks -- do 
> > > > > > > you still observe them if you use the version of the freezer which 
> > > > > > > doesn't freeze kernel threads?
> > > > > > 
> > > > > > In general the only way to guarantee there are no deadlocks is to
> > > > > > construct the graph of dependencies between tasks.  Those dependencies
> > > > > > are not in practice observable from outside the tasks, so it is
> > > > > > virtually impossible to construct the graph.
> > > > > 
> > > > > In which way can user space tasks depend on each other in a way that
> > > > > allows a them members of that cycle to be in uninterruptible sleep?
> > > > 
> > > >  - process A calls rename() on a fuse fs
> > > >  - process B, the fuse server, starts to process the rename request
> > > >  - process B is frozen before it can reply
> > > > 
> > > > Now process A is unfreezable.  We cannot make rename() restartable,
> > > > hence it cannot be interruptible.
> > > 
> > > Yes, we are claiming fuse is very special in this regard, and perhaps
> > > even broken.
> > > 
> > > Let's see. If I SIGSTOP the fuse server, I can get unrelated tasks
> > > unkillable (even for SIGKILL!) forever.
> > 
> > Actually fuse allows SIGKILL, because it's always fatal, and the
> > syscall may not be restarted.
> 
> Okay, and you should handle refrigerator in the same paths where you
> handle SIGKILL. Just add try_to_freeze() there...

In fact the problem is more complicated than that, because some tasks may
be waiting on VFS locks related to FUSE.

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Alan Stern]

On Thu, 5 Jul 2007, Paul Mackerras wrote:

> Alan Stern writes:
> 
> > Let's agree the kernel threads and the freezer are a separate issue.  
> 
> No, I don't think they are a separate issue, because I think the
> distinction the freezer makes between kernel threads and user threads
> is a false and misleading distinction.

That's a little strong.  "Misleading" I could understand, but "false"?  
Isn't the distinction between a kernel thread and a user task pretty 
clear-cut (except for a few borderline cases which aren't at issue just 
now)?

> > I agree the kernel threads which try to do I/O during a suspend will 
> > need extra attention.  However if these threads are necessary for the 
> > suspend procedure, then blocking them (which is how people on this 
> > thread have been saying driver should treat I/O requests during a 
> > suspend) will cause additional problems.  There's no way around it; 
> > these threads _will_ require more work.
> 
> There is a way around it; do the request blocking in the drivers,
> where it belongs.

How will that help?  Block the kernel thread in the freezer or block it 
in the driver -- either way it is blocked.  So how do your deadlocks 
get resolved?

> In general the only way to guarantee there are no deadlocks is to
> construct the graph of dependencies between tasks.  Those dependencies
> are not in practice observable from outside the tasks, so it is
> virtually impossible to construct the graph.
> 
> The "don't freeze kernel threads" thing is an attempt to make a crude
> approximation to the dependency graph (by saying kernel threads only
> depend on other kernel threads), but the approximation breaks down
> when you have FUSE or user-level device drivers.

I disagree with your analysis -- not that it's completely wrong, but it 
points out an existing basic problem in the kernel.  The kernel should 
never depend on userspace!  More correctly, a task executing in the 
kernel should never block with any sort of mutex or other lock held (in 
a way that would preclude it from being frozen, let's say) while 
waiting for a response from userspace.

Then the dependency graph would be easy to construct: User tasks can
depend on whatever they want, and kernel threads never depend on a user
task.

If this contradicts the existing implementations and APIs for userspace 
filesystems, then so be it.  My conclusion would be that the 
implementations and APIs should be changed.

> > There remains the problem of user tasks whose assistance is required to 
> > carry out some I/O (as with FUSE).  If the I/O can be deferred until 
> > after the resume, then there's no problem.  If the I/O can be carried 
> > out before the suspend, then it should be.  And finally, if the I/O 
> > must be done during the suspend, you're in real trouble -- how do you 
> > do I/O to a suspended device?
> 
> So why doesn't that argument apply to kernel threads? :)

It _does_ apply to kernel threads.  That's exactly why I wrote above 
that kernel threads which try to do I/O during a suspend will need 
extra attention.

Alan Stern

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Paul Mackerras]

Alan Stern writes:

> USB already implements runtime PM.  If a device is suspended at runtime
> and a task tries to access it, the device is automatically resumed.  
> No problem there.
> 
> The problem comes when the system is doing a STR.  Right now the code
> doesn't keep track of the difference between a runtime suspend and a
> system suspend -- once the device is suspended, it's suspended, period.  

Whether or not to resume a suspended device when an I/O request comes
in is a policy decision, and there could be cases where the user wants
I/O requests to be blocked, or to fail, or to be dropped while the
device is suspended, even for runtime power management.  For example,
a sound card could be suspended due to a low-battery condition, and in
that case you would want the driver to just drop any data that
userspace tries to write to the soundcard.

> Yes, the code could be changed to keep track of the reason for a device
> suspend.  But that just raises the old problem of what to do when
> there's an I/O request for a suspended device during STR.

Is this actually a real problem?  I would think the policy would be
"block" for block devices (pun not intended :), "drop" for network
devices, etc.

> Consider a particularly troublesome case: During STR, a non-frozen task
> writes to /sys/bus/BBB/drivers/DDD/bind.  The sysfs core grabs the
> device semaphore and calls the driver's probe routine.  If the driver
> isn't PM-aware it simply tries to initialize the device and fails
> because the device is already suspended.  That's no good; it isn't
> transparent.

How did the device get suspended if it didn't have a driver?  If it
did have a driver, why didn't the bind attempt fail?

> So assume the driver is PM-aware.  It tries to resume the device, which
> fails because STR is underway.  Now what can it do?  There's only one 
> possibility: It must block until the resume call can succeed.  But when 
> is that?
> 
> It has to be before the PM core tries to resume the device, because the 
> core will try to acquire the device semaphore and will block waiting 
> for the probe call to complete.  But it has to be after the PM core 
> resumes the device's parent, because obviously the device can't resume 
> until its parent is awake.

Suppose the device-model core code simply blocked all bind and unbind
requests while suspend is under way, until resume is finished.
Wouldn't that solve the problem?

Paul.

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Rafael J. Wysocki]

On Tuesday, 3 July 2007 16:21, Johannes Berg wrote:
> On Tue, 2007-07-03 at 14:56 +0200, Rafael J. Wysocki wrote:
> 
> > Still, can you please read this post from Alan Stern:
> > 
> > https://lists.linux-foundation.org/pipermail/linux-pm/2007-June/012847.html
> > 
> > ?  I don't think I'm able to repeat the arguments given in there in a
> > convincing way.
> 
> As I read it, Alan basically has two objections:
>  (1) drivers shouldn't need to worry about this
>  (2) suspend should be transparent to userspace
> 
> His proposed solution (freezing tasks when they cross the kernel
> boundary) helps for the s-t-r case, but in fact doesn't solve (1)
> because devices can be suspended at runtime

This is a different thing and a different infrastructure is needed for it (not
present at the moment).

> and then you certainly do not want to freeze tasks that try to access the
> device. 
> 
> (2) is related but not identical, what if you have a device suspended at
> runtime and some tasks tries to access it; should the task block until
> you wake up that device?

I think the device should be woken up in that case.

> I think the core of the discussion isn't appreciated by everybody here
> yet---we need to solve both run-time and suspend-to-ram-time device
> suspend, not just one of them.

For now, we're discussing the suspend-to-ram-time suspend only, for which
we have (some) infrastrcuture (and which should be supported by all drivers,
IMO).

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Johannes Berg]

[-- Attachment #1.1: Type: text/plain, Size: 1367 bytes --]

On Tue, 2007-07-03 at 16:51 +0200, Rafael J. Wysocki wrote:

> > His proposed solution (freezing tasks when they cross the kernel
> > boundary) helps for the s-t-r case, but in fact doesn't solve (1)
> > because devices can be suspended at runtime
> 
> This is a different thing and a different infrastructure is needed for it (not
> present at the moment).

Yeah I should've said "any of his proposed solutions"

> > and then you certainly do not want to freeze tasks that try to access the
> > device. 
> > 
> > (2) is related but not identical, what if you have a device suspended at
> > runtime and some tasks tries to access it; should the task block until
> > you wake up that device?
> 
> I think the device should be woken up in that case.

Ah, but that also means the device has to actually know about it.

> > I think the core of the discussion isn't appreciated by everybody here
> > yet---we need to solve both run-time and suspend-to-ram-time device
> > suspend, not just one of them.
> 
> For now, we're discussing the suspend-to-ram-time suspend only, for which
> we have (some) infrastrcuture (and which should be supported by all drivers,
> IMO).

Right but if we solve the run-time suspend case in favour of having the
device driver know about it then the suspend-to-ram-time suspend case
solves itself.

johannes

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

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

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Paul Mackerras]

Rafael J. Wysocki writes:

> Still, do you really think that we're ready to drop it _right_ _now_ (I'm
> referring to suspend only) and if so than on what basis (except that you
> don't like it, which falls short of being a techical argument)?

The basis is that it (the freezer) causes more deadlocks and other
problems than it avoids, so it's a net win to remove it.

Paul.

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Rafael J. Wysocki]

On Wednesday, 4 July 2007 05:29, Paul Mackerras wrote:
> Rafael J. Wysocki writes:
> 
> > Still, do you really think that we're ready to drop it _right_ _now_ (I'm
> > referring to suspend only) and if so than on what basis (except that you
> > don't like it, which falls short of being a techical argument)?
> 
> The basis is that it (the freezer) causes more deadlocks and other
> problems than it avoids, so it's a net win to remove it.

So, I gather, you're volunteering to handle suspend-related bug reports
from the point in which we drop the freezer from the suspend code path?

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Paul Mackerras]

Rafael J. Wysocki writes:

> So, I gather, you're volunteering to handle suspend-related bug reports
> from the point in which we drop the freezer from the suspend code path?

Ben and I are happy to handle all the ones for the platform we
maintain, which currently does suspend without freezing processes. :)

Paul.

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Rafael J. Wysocki]

On Wednesday, 4 July 2007 12:48, Paul Mackerras wrote:
> Rafael J. Wysocki writes:
> 
> > So, I gather, you're volunteering to handle suspend-related bug reports
> > from the point in which we drop the freezer from the suspend code path?
> 
> Ben and I are happy to handle all the ones for the platform we
> maintain, which currently does suspend without freezing processes. :)

I mean all platforms.  After all, the $subject change won't affect yours.

BTW, does your platform's suspend work on SMP systems?

Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Paul Mackerras]

Rafael J. Wysocki writes:

> BTW, does your platform's suspend work on SMP systems?

Yes; currently we require userspace to offline all cpus other than the
boot cpu before initiating the suspend.

The main difficulty is actually that SMP powermacs that can suspend
tend to have video cards that get powered off in suspend.  We know how
to re-initialize one (the Radeon RV100 QW) but not others.  That's an
orthogonal issue to the issues we have been discussing, though.

Paul.

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Rafael J. Wysocki]

On Wednesday, 4 July 2007 13:24, Paul Mackerras wrote:
> Rafael J. Wysocki writes:
> 
> > BTW, does your platform's suspend work on SMP systems?
> 
> Yes; currently we require userspace to offline all cpus other than the
> boot cpu before initiating the suspend.

This is incompatible with the code in kernel/power/main.c, since we only
disable the nonboot CPUs after devices have been suspended.  Do you think that
your framework can be modified to work without disabling the nonboot CPUs
by the user space?

> The main difficulty is actually that SMP powermacs that can suspend
> tend to have video cards that get powered off in suspend.  We know how
> to re-initialize one (the Radeon RV100 QW) but not others.  That's an
> orthogonal issue to the issues we have been discussing, though.

Sure.

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Paul Mackerras]

Rafael J. Wysocki writes:

> This is incompatible with the code in kernel/power/main.c, since we only
> disable the nonboot CPUs after devices have been suspended.  Do you think that
> your framework can be modified to work without disabling the nonboot CPUs
> by the user space?

Sure.  It was a "if it can be done in userspace, do it in userspace"
kind of decision, but I'm not wedded to it.

I actually do want to converge to using the generic suspend-to-ram
code on powerbooks.  I just want to avoid causing regressions for
powerbook users, including myself. :)

Paul.

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Rafael J. Wysocki]

On Thursday, 5 July 2007 02:15, Paul Mackerras wrote:
> Rafael J. Wysocki writes:
> 
> > This is incompatible with the code in kernel/power/main.c, since we only
> > disable the nonboot CPUs after devices have been suspended.  Do you think that
> > your framework can be modified to work without disabling the nonboot CPUs
> > by the user space?
> 
> Sure.  It was a "if it can be done in userspace, do it in userspace"
> kind of decision, but I'm not wedded to it.
> 
> I actually do want to converge to using the generic suspend-to-ram
> code on powerbooks.  I just want to avoid causing regressions for
> powerbook users, including myself. :)

Okay, but my question is this: Would that be possible, within your framework,
to disable the nonboot CPUs _after_ suspending devices?

Can you please point me to your high-level suspend code?

Greetings,
Rafael


-- 
"Premature optimization is the root of all evil." - Donald Knuth

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Pavel Machek]

On Thu 2007-07-05 10:15:01, Paul Mackerras wrote:
> Rafael J. Wysocki writes:
> 
> > This is incompatible with the code in kernel/power/main.c, since we only
> > disable the nonboot CPUs after devices have been suspended.  Do you think that
> > your framework can be modified to work without disabling the nonboot CPUs
> > by the user space?
> 
> Sure.  It was a "if it can be done in userspace, do it in userspace"
> kind of decision, but I'm not wedded to it.
> 
> I actually do want to converge to using the generic suspend-to-ram
> code on powerbooks.  I just want to avoid causing regressions for
> powerbook users, including myself. :)

Curious, do you actually use fuse? Can you try it _with_ freezer and
produce sysrq-t trace of deadlock?
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Paul Mackerras]

Rafael J. Wysocki writes:

> > > So, I gather, you're volunteering to handle suspend-related bug reports
> > > from the point in which we drop the freezer from the suspend code path?
> > 
> > Ben and I are happy to handle all the ones for the platform we
> > maintain, which currently does suspend without freezing processes. :)
> 
> I mean all platforms.  After all, the $subject change won't affect yours.

Well, I can't commit to handling all bug reports, but I am happy to
help out where I can...

Paul.

Re: Re: [PATCH] Remove process freezer from suspend to RAM pathway

[Paul Mackerras]

Pavel Machek writes:

> How well does it work on SMP PPC?

Just fine, on those machines where we know how to reinitialize the
video card.  We currently require userspace to offline all except the
boot cpu before suspending, but that could be moved into the kernel.
I have no particular attachment to that way of doing it; it was just a
"don't do things in the kernel that can be reasonably be done in
userspace" kind of thing.

Paul.