* Nigel Cunningham <[email protected]> wrote:
> From subsequent emails, I think you already got your answer, but just
> in case...
>
> Yes, if you enabled "Replace swsusp by default" and you already had it
> set up for getting swsusp to resume. If not, and you're using an
> initrd/ramfs, you'll need to modify it to echo
> > /sys/power/suspend2/do_resume after /sys and /proc are mounted but
> prior to mounting / and so on.
yeah, went with the default suggested by your patch:
CONFIG_SUSPEND2_REPLACE_SWSUSP=y
and it was pretty easy to set things up. I used "echo disk >
/sys/power/state" to trigger it.
In hindsight it was all pretty straightforward and suspend2 worked
beautifully on an UP and on an SMP system i tried. So in exchange for
suspend2 folks debugging a bug in CFS here's some suspend2 review
feedback ;) Any plans about moving suspend2 to the upstream kernel? It
should be pretty easy for it to co-exist with the current swsuspend
code.
The patch has quite some size:
89 files changed, 16452 insertions(+), 69 deletions(-)
that should obviously be split up into more than a dozen sub-patches,
and fed to lkml with the small ones first. (unless it already is split
up?)
i cannot comment on the kernel/power/ bits (they are way too large
anyway), other than that they look pretty clean visually, but the
lowlevel arch and generic kernel bits look sane in detail too, sans a
few mostly trivial cleanliness issues:
+int suspend2_faulted = 0;
+EXPORT_SYMBOL(suspend2_faulted);
should be done via the pagefault notifier chain mechanism. Also, all the
exports you added should be EXPORT_SYMBOL_GPL().
this:
- ClearPageReserved(virt_to_page(addr));
- init_page_count(virt_to_page(addr));
+ //ClearPageReserved(virt_to_page(addr));
+ //init_page_count(virt_to_page(addr));
looks like there's a buglet in there still somewhere?
+ if(PageHighMem(page))
+ return 0;
coding style.
+ BUG_ON( test_suspend_state(SUSPEND_RUNNING) && /* Suspend2, that is */
make this a WARN_ON() or a WARN_ON_ONCE() - that way you have a chance
to even get feedback from users, instead of a 'uhm, X froze' report.
+#define FREEZER_OFF 0
+#define FREEZER_USERSPACE_FROZEN 1
+#define FREEZER_FULLY_ON 2
should be:
+#define FREEZER_OFF 0
+#define FREEZER_USERSPACE_FROZEN 1
+#define FREEZER_FULLY_ON 2
(you want your reviewers have an pleasant time reading your code :)
+#define NETLINK_SUSPEND2_USERUI 20 /* For suspend2's userui */
IIRC userui was at the center of suspend2 merge flames, right? So you
might want to layer it ontop a less flashy suspend2-core and thus get
90% of your patch upstream?
+++ linux/mm/vmscan.c
the MM impact looks quite nontrivial. But i suspect this is unavoidable,
because you zap portions of the pagecache on the way to disk, so when it
comes back it results in a different pagecache (new lru lists, etc.),
right?
+++ linux/lib/dyn_pageflags.c
shouldnt this be in mm/dyn_pageflags.c? Plus it would be nice to use
some other core kernel user for this infrastructure. (but it's not a
necessity i guess)
but ... again, the patch looks sane all around.
Ingo
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
