-test7: /sys/power/disk not reading right data?

-test7: /sys/power/disk not reading right data?

[Pavel Machek]

Hi!

I'm seeing this on -test7 (tainted:pavel, but I did not touch this
area).
								Pavel


root@amd:~# echo -n platform > /sys/power/disk
root@amd:~# dmesg | tail -1
PM: suspend-to-disk mode set to 'platform'
root@amd:~# cat /sys/power/disk
firmware
root@amd:~#

-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: -test7: /sys/power/disk not reading right data?

[Pavel Machek]

Hi!

> I'm seeing this on -test7 (tainted:pavel, but I did not touch this
> area).

Reproduced on -test7-bk, vanilla.

I really do not think that 2 separate files (disk, state) are good
idea. It bitten me, again. I did echo -n platform > disk, echo -n disk
> state. Unfortunately kernel somehow decided that platform suspend is
not supported, but honoured second request. Result is crash, of
course. This error would be avoided if I said echo -n swsusp > state
(or similar), because unrecognized/unsupported request would be simply
not done.

What advantages does disk/ state separation have? I believe that echo
swsusp > state (or echo s4bios > state or echo disk-firmware > state)
is the right interface, and that we want to do echo "s4bios" >
on_battery_low or similar interface. (echo "mem" > on_battery_low
makes sense, too, toshiba notebooks do that for example).

> root@amd:~# echo -n platform > /sys/power/disk
> root@amd:~# dmesg | tail -1
> PM: suspend-to-disk mode set to 'platform'
> root@amd:~# cat /sys/power/disk
> firmware
> root@amd:~#

								Pavel
-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]

Re: -test7: /sys/power/disk not reading right data?

[Patrick Mochel]

> > I'm seeing this on -test7 (tainted:pavel, but I did not touch this
> > area).
> 
> Reproduced on -test7-bk, vanilla.

It's a known bug. If your system supports both S4 and S4bios, only the 
latter may be selected. 

> What advantages does disk/ state separation have? I believe that echo
> swsusp > state (or echo s4bios > state or echo disk-firmware > state)
> is the right interface, and that we want to do echo "s4bios" >
> on_battery_low or similar interface. (echo "mem" > on_battery_low
> makes sense, too, toshiba notebooks do that for example).

The string 's4bios' only makes sense on systems that support it, while 
'disk' is concise and obvious, regardless on how your platform implements 
it. I don't want to have options that are only relevant on a subset of the 
systems out there. 



	Pat

Re: -test7: /sys/power/disk not reading right data?

[Pavel Machek]

Hi!

> > What advantages does disk/ state separation have? I believe that echo
> > swsusp > state (or echo s4bios > state or echo disk-firmware > state)
> > is the right interface, and that we want to do echo "s4bios" >
> > on_battery_low or similar interface. (echo "mem" > on_battery_low
> > makes sense, too, toshiba notebooks do that for example).
> 
> The string 's4bios' only makes sense on systems that support it, while 
> 'disk' is concise and obvious, regardless on how your platform implements 
> it. I don't want to have options that are only relevant on a subset of the 
> systems out there. 

So call it disk-firmware.

The two-files interface is accident waiting for happen. (And accident
that already happened, it crashed my machine at least twice).

Assume your user has 'echo platform > /sys/power/disk', and has
suspend partition configured for BIOS.

He's used to suspending by echo disk > /sys/power/state. One day, for
whatever reason (new kernel does not recognize s4bios?) platform
suspend is unavailable. echo > /sys/power/disk is going to fail, but
user is very unlikely to notice that. He is also very unlikely to cat
/sys/power/disk before doing suspend. He does 'echo disk >
/sys/power/disk', machine does swsusp (ouch). He can still save his
data by adding resume=... parameter on next boot, but I guess it is
unlikely to happen.

This looks very much like "trap for user" to me, and that trap already
got me twice (s4bios does not really work here now, and I was trying
to test pmdisk. It took me two fsck-s before I found out whats
happening).

If you want to have concise and obvious interface, do echo
'disk-platform' vs. 'disk-firmware', or whatever, but please do not
use two files.
								Pavel
-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]