[PATCH] powermac: proper sleep management

[Johannes Berg]

On Mon, 2007-04-30 at 15:31 +1000, Paul Mackerras wrote:

> At a quick look, the generic code is calling
> freeze_processes and shrink_all_memory, and I don't see where it's
> doing a sync.

The sync is done after userspace has been frozen, in freeze_processes. I
agree that shrink_all_memory is bogus, I'll talk to Rafael about it;
we're working on splitting out the hibernate and suspend code paths
anyway so it should be possible to remove this cleanly. It should also
be easy to invoke it only for suspend to disk for now.

> Also, you're now calling pbook_alloc_pci_save after interrupts are
> disabled, and it does a kmalloc(..., GFP_KERNEL).  Oops.

Oops, yes, thanks for pointing that out, it needs to go into the prepare
callback. I just sent an updated patch that moves it there and verified
that no other code was misplaced.

> Part of the problem is exactly what Linus pointed out: that the
> generic code tries to use the same code paths for suspend to RAM and
> suspend to disk, but they are two totally different things.

Actually, the stuff Linus pointed out in the recent thread was about
device drivers and the current PMU code uses the same driver/device
suspend routines that the generic code uses. No difference there.

> I have no objection to adding code to enable the generic code to do
> the (mostly) right thing when you write "mem" into /sys/power/state.
> I have no objection to code being refactored to eliminate
> duplication.  All I ask is that the PMU ioctls continue to do
> essentially the same things in the same order.

I disagree with that, it'll get us no closer to keeping the generic code
working for us. It does in fact work now, I've been using it for a long
time and a few other people have also tried on older powerbooks. It's
also very hard to convince anybody that we need changes in the generic
code if at the same time we do our own stuff because "the generic code
(you wrote) is not good enough for us anyway."

Considering the options from userspace, there are currently 3 programs
(that I know of) that actually use the ioctl:
 * hal via some helper or via pm-utils (almost all gnome/kde programs
   use it)
 * pbbuttonsd
 * pmud (?)

I know that the hal/pm-utils folks would love to get rid of the pmu
specific stuff and I think this and the battery (?) are the last things.
I don't even pretend to know what pbbuttonsd/pmud will do. Regardless of
that, however, the vast majority of users of modern desktop distros will
be using hal and hence the new code simply because that is integrated
into their desktop. If we fragment the user base into these two sets
we're making life more difficult because suddenly "sleep" or "suspend to
RAM" no longer identifies uniquely what the user did. Also, if one of
them breaks people will switch to the other one instead of helping
identify and fix the problem.

johannes
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 190 bytes
Desc: This is a digitally signed message part
URL: <http://lists.ozlabs.org/pipermail/linuxppc-dev/attachments/20070430/11322408/attachment.pgp>