Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: frank.hofmann@tomtom.com (Frank Hofmann)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC PATCH] Current status, suspend-to-disk support on ARM
Date: Tue, 12 Apr 2011 17:30:27 +0100 (BST)	[thread overview]
Message-ID: <alpine.DEB.2.00.1104121636460.29883@localhost6.localdomain6> (raw)
In-Reply-To: <20110412055958.GA3507@ucw.cz>

On Tue, 12 Apr 2011, Pavel Machek wrote:

> Hi!
>
>> There's one ugly thing in this set - I've changed a generic kernel
>> header, <linux/suspend.h> to #define save/restore_processor_state()
>> on ARM so that it only does preempt_disable/enable(). It's
>> surprising that this isn't the default behaviour; all platforms need
>> swsusp_arch_suspend/resume anyway so why force the existance of
>> _two_ arch-specific hooks ?
>
> Can you submit separate patch cleaning it up?

How about the attached ?

It's for discussion, and hence not tested (admittedly, I need an x86 test 
system ...).

The diff is against RMK's devel-stable tree, as of commit 
5f183860d5007ec76ea36bfa6c36d66e37f0dbcf


There's a few hurdles here:

a) who knows all assembly calling conventions for all architectures
    supported by linux ?

    This applies to SH and S390; save/restore_processor_state on those
    are primitive and should be inlined within swsusp_arch_suspend/resume,
    just like the attached patch does for the powerpc variants.

    I've done a bounce call within S390 but don't know enough about SH3
    for arch/sh/kernel/cpu/sh3/swsusp.S - i.e. add a call in assembly to
    init_cpu(current), and then ditch save_processor_state.

b) some architectures do things _beyond_ register/state saving in those
    two functions, and hence rely on them being called paired.

c) CONFIG_KEXEC_JUMP (ab-)uses them. I don't know that subsystem at all,
    but this sort of re-use means the symbols can't just be ditched.
    They become arch-dependent though, no non-arch/ code calls them
    anymore.

What I've attached might break SH3 swsusp, as save_processor_state 
is no longer called there (it stores FPU state).




I've also been wondering:
The comments in kernel/power/hibernate.c mention the need to get preempt 
counts "right" as major motivator to call save/restore_processor_state.
effect" of save/restore_processor_state).

But then on all architectures in kernels as far back as 2.6.32 it doesn't 
look like save/restore_processor state are actually adjusting the preempt 
count; nowhere do they call preempt_enable/disable.

What is and what is not necessary, really ?


Finally, on things like e.g. the floating point context saves: Wouldn't 
this happen as part of freezing processes (in the early stages of 
suspend), and/or as part of device quiesce ?
I wonder why e.g. powerpc does it explicitly; not doing so on ARM doesn't 
seem to cause problems.



>
>> @@ -195,6 +195,14 @@ config VECTORS_BASE
>>  	help
>>  	  The base address of exception vectors.
>>
>> +config ARCH_HIBERNATION_POSSIBLE
>> +	bool
>> +	help
>> +	  If the machine architecture supports suspend-to-disk
>> +	  it should select this automatically for you.
>> +	  Otherwise, say 'Y' at your own peril.
>> +
>
> Good for debugging, but not good for mainline.

How would this be done better ?

FrankH.

>
> -- 
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: remove-save_processor_state.patch
Type: text/x-diff
Size: 10417 bytes
Desc: 
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110412/6de02f2b/attachment.bin>

  reply	other threads:[~2011-04-12 16:30 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-04-04 14:47 [RFC PATCH] Current status, suspend-to-disk support on ARM Frank Hofmann
2011-04-04 15:06 ` Frank Hofmann
2011-04-05 12:22 ` [RFC PATCH v2] " Frank Hofmann
2011-04-12  5:59 ` [RFC PATCH] " Pavel Machek
2011-04-12 16:30   ` Frank Hofmann [this message]
2011-04-12 18:32     ` Pavel Machek
2011-04-13 13:36       ` Frank Hofmann
2011-04-13 21:38         ` Rafael J. Wysocki
2011-04-14 17:06           ` Frank Hofmann
2011-04-18  5:48             ` [TuxOnIce-devel] " Matt Hsu
2011-04-19 18:13               ` Frank Hofmann
2011-04-20  7:14                 ` Matt Hsu
2011-04-21  4:55                   ` [linux-pm] " MyungJoo Ham

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.00.1104121636460.29883@localhost6.localdomain6 \
    --to=frank.hofmann@tomtom.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox