userland swsusp
Subsystems (4)
RFC 1/1 userland swsusp
Hi!
This is prototype of userland swsusp. I'd like kernel parts to go in, probably for 2.6.16. Now, I'm not sure about the interface, ioctls are slightly ugly, OTOH it would be probably overkill to introduce syscalls just for this. (I'll need to add an ioctl for freeing memory in future).
Small question is where should userspace parts go. In-kernel kernel/power/swsusp.c is basically replaced by usr/swsusp.c....
[Of course, I'll need to fix the patch up so that it does not modify existing behaviour].
Pavel
drivers/char/mem.c +41 -0
@@ -27,6 +27,7 @@#include <linux/crash_dump.h> #include <linux/backing-dev.h> #include <linux/bootmem.h> +#include <linux/suspend.h> #include <asm/uaccess.h> #include <asm/io.h>
@@ -559,6 +561,45 @@ static ssize_t write_port(struct file * }#endif +static int +ioctl_kmem (struct inode *inode,struct file *file,unsigned int cmd,unsigned long arg) +{ +int retval =0 ; + +switch (cmd) { +case IOCTL_FREEZE : + retval =sys_freeze (); +break ; +case IOCTL_UNFREEZE : + retval =sys_unfreeze (); +break ; +case IOCTL_ATOMIC_SNAPSHOT : + retval =sys_atomic_snapshot (arg); +break ; +case IOCTL_ATOMIC_RESTORE : + { +int pages; +void *pgdir; +get_user (pages, (long *) arg); +get_user (pgdir, (void **) (arg +4 )); + retval =sys_atomic_restore (pgdir, pages); + } +break ; +case IOCTL_KMALLOC : + retval =get_zeroed_page (GFP_KERNEL ); +break ; +case IOCTL_KFREE : +free_page (arg); +break ; +default : + retval = -ENOTTY ; +break ; + } + +return retval; +} + +static ssize_t read_null (struct file * file,char __user * buf,size_t count,loff_t *ppos) {
@@ -769,6 +810,7 @@ static struct file_operations mem_fops =static struct file_operations kmem_fops = { .llseek = memory_lseek, .read = read_kmem, + .ioctl = ioctl_kmem, .write = write_kmem, .mmap = mmap_kmem, .open = open_kmem,
include/linux/suspend.h +23 -0
@@ -1,6 +1,7 @@#ifndef _LINUX_SWSUSP_H#define _LINUX_SWSUSP_H +#ifdef __KERNEL__#if defined(CONFIG_X86 ) || defined(CONFIG_FRV ) || defined(CONFIG_PPC32 )#include <asm/suspend.h> #endif
@@ -9,6 +10,7 @@#include <linux/config.h> #include <linux/init.h> #include <linux/pm.h> +#endif /* page backup entry */ typedef struct pbe {
@@ -30,6 +32,7 @@ typedef struct pbe {#define for_each_pb_page (pbe, pblist) \ for (pbe = pblist ; pbe ; pbe = (pbe+PB_PAGE_SKIP)->next) +#ifdef __KERNEL__#define SWAP_FILENAME_MAXLENGTH 32
@@ -79,4 +82,24 @@ unsigned long get_safe_page(gfp_t gfp_ma*/ #define PAGES_FOR_IO 512 +#endif + +struct restore_ioctl { +void *pgdir; +int nr_pages; +}; + +#define IOCTL_FREEZE _IO('3', 1) +#define IOCTL_UNFREEZE _IO('3', 2) +#define IOCTL_ATOMIC_SNAPSHOT _IOW('3', 3, void **) +#define IOCTL_ATOMIC_RESTORE _IOR('3', 4, struct restore_ioctl) +#define IOCTL_KMALLOC _IO('3', 5) +#define IOCTL_KFREE _IOR('3', 6, void *) + +extern int sys_freeze (void ); +extern int sys_unfreeze (void ); +extern int sys_atomic_snapshot (void **pgdir); +extern int sys_atomic_restore (void *pgdir,int pages); + +#endif /* _LINUX_SWSUSP_H */
kernel/power/console.c +1 -0
@@ -9,6 +9,7 @@#include <linux/console.h> #include "power.h" +#undef SUSPEND_CONSOLEstatic int new_loglevel =10 ;static int orig_loglevel;#ifdef SUSPEND_CONSOLE
kernel/power/disk.c +17 -1
@@ -143,12 +144,25 @@ thaw:static void unprepare_processes (void ) { -platform_finish ();thaw_processes ();enable_nonboot_cpus ();pm_restore_console (); } + +int sys_freeze (void ) +{ +return prepare_processes (); +} + +int sys_unfreeze (void ) +{ +thaw_processes (); +enable_nonboot_cpus (); +pm_restore_console (); +return 0 ; +} +/** * pm_suspend_disk - The granpappy of power management. *
@@ -243,6 +257,9 @@ static int software_resume(void)if ((error =swsusp_check ())) goto Done; +/* Prepare processes only after swsusp_check; we could do it before, +but it would mean an ugly console switch even in case of normal boot. +*/ pr_debug ("PM: Preparing processes for restore.\n" );if ((error =prepare_processes ())) {
kernel/power/swsusp.c +57 -0
73 lines not on this page · raw message
usr/swsusp-init new file +9 -0
@@ -0,0 +1,9 @@ +#!/bin/bash +# +# swapoff /dev/hda1; cat /dev/zero | head -c 4096 > /dev/hda1 +# /tmp/swsusp /dev/hda1 -s -b +/tmp/swsusp /dev/hda1 -r +exec /sbin/init +exit + +
usr/swsusp.c new file +529 -0
529 lines not on this page · raw message
Signature · 1 line
Thanks, Sharp!
Quoted 7 lines · Pavel Machek
Hi!
This is prototype of userland swsusp. I'd like kernel parts to go in, probably for 2.6.16. Now, I'm not sure about the interface, ioctls are slightly ugly, OTOH it would be probably overkill to introduce syscalls just for this. (I'll need to add an ioctl for freeing memory in future).
What's wrong with 4 new syscalls? It seems the cleanest way.
thanks,
greg k-h
Hi!
Quoted 7 lines
Quoted 5 lines
This is prototype of userland swsusp. I'd like kernel parts to go in, probably for 2.6.16. Now, I'm not sure about the interface, ioctls are slightly ugly, OTOH it would be probably overkill to introduce syscalls just for this. (I'll need to add an ioctl for freeing memory in future).
What's wrong with 4 new syscalls? It seems the cleanest way.
I'd need about 7 of them, and that is on at least 3 architectures (i386, x86-64, ppc, not sure about ppc64/arm). And it does not fix the interface -- userland parts will still need to read/write /dev/kmem :-(.
Yep, I can do it...
Pavel
Signature · 1 line
Thanks, Sharp!
Quoted 7 lines · Pavel Machek
Hi!
This is prototype of userland swsusp. I'd like kernel parts to go in, probably for 2.6.16. Now, I'm not sure about the interface, ioctls are slightly ugly, OTOH it would be probably overkill to introduce syscalls just for this. (I'll need to add an ioctl for freeing memory in future).
Just for info: If this goes in, Red Hat/Fedora kernels will fork swsusp development, as this method just will not work there. (We have a restricted /dev/mem that prevents writes to arbitary
memory regions, as part of a patchset to prevent rootkits)
Even it were not for this, the whole idea seems misconcieved to me anyway.
Dave
Hi!
Quoted 10 lines
Quoted 5 lines
This is prototype of userland swsusp. I'd like kernel parts to go in, probably for 2.6.16. Now, I'm not sure about the interface, ioctls are slightly ugly, OTOH it would be probably overkill to introduce syscalls just for this. (I'll need to add an ioctl for freeing memory in future).
Just for info: If this goes in, Red Hat/Fedora kernels will fork swsusp development, as this method just will not work there. (We have a restricted
/dev/memthat prevents writes to arbitary
memory regions, as part of a patchset to prevent rootkits)
If this goes in, you can still keep using old method... I'll not remove it anytime soon.
Quoted 2 lines
Even it were not for this, the whole idea seems misconcieved to me anyway.
...but how do you provide nice, graphical progress bar for swsusp without this? People want that, and "esc to abort", compression, encryption. Too much to be done in kernel space, IMNSHO.
Pavel
Signature · 1 line
Thanks, Sharp!
Quoted 2 lines · Pavel Machek
If this goes in, you can still keep using old method... I'll not remove it anytime soon.
Ok.
Quoted 6 lines
Quoted 2 lines
Even it were not for this, the whole idea seems misconcieved to me anyway.
...but how do you provide nice, graphical progress bar for swsusp without this? People want that, and "esc to abort", compression, encryption. Too much to be done in kernel space, IMNSHO.
I'll take "rootkit doesnt work" over "bells and whistles".
I think most users actually care more about "works" than "looks pretty, and then fails spectacularly".
Dave
Hi!
Quoted 8 lines
Quoted 6 lines
Quoted 2 lines
Even it were not for this, the whole idea seems misconcieved to me anyway.
...but how do you provide nice, graphical progress bar for swsusp without this? People want that, and "esc to abort", compression, encryption. Too much to be done in kernel space, IMNSHO.
I'll take "rootkit doesnt work" over "bells and whistles".
It moves bunch of code from kernelspace to userspace. You don't have to add bells and whistles at the same time. That's normally called good thing. If Fedora has special needs, fine.
Pavel
Signature · 1 line
Thanks, Sharp!
Hi,
Quoted 10 lines · Dave Jones
Quoted 2 lines · Pavel Machek
If this goes in, you can still keep using old method... I'll not remove it anytime soon.
Ok.
Quoted 3 lines
Quoted 2 lines
Even it were not for this, the whole idea seems misconcieved to me anyway.
Quoted 8 lines
Quoted 3 lines
...but how do you provide nice, graphical progress bar for swsusp without this? People want that, and "esc to abort", compression, encryption. Too much to be done in kernel space, IMNSHO.
I'll take "rootkit doesnt work" over "bells and whistles".
I think most users actually care more about "works" than "looks pretty, and then fails spectacularly".
I've been discussing this with Pavel for quite some time and my opinion is that moving the image-writing and reading functionality of swsusp to the user space makes sense from the technical point of view.
For example it would allow us to add the image encryption (real, eg. with a passphrase-protected key), image compression, and image verification in a rather straightforward way. These are important functionalities, at least for some users.
However, I think we should not try to read and/or set up kernel data structures from the users space. Instead, we can create an interface that will allow us to convey the image data and metadata from the kernel to the user space and vice versa.
Greetings,
Rafael
În data de Mi, 16-11-2005 la 00:32 +0100, Pavel Machek a scris:
Quoted 3 lines
...but how do you provide nice, graphical progress bar for swsusp without this? People want that, and "esc to abort", compression, encryption. Too much to be done in kernel space, IMNSHO.
Pavel, you really should _listen_ when someone else is talking about the same things in different implementations. suspend2 has this feature (nice graphical progress bars in userspace) for a long time now and it's compatible with the fedora kernels.
Why don't you and Nigel (of suspend2) can just work together on this ? It's a shame that much work is wasted in duplicated effort.
Signature · 1 line
Cioby
Quoted 9 lines · Dumitru Ciobarcianu
??n data de Mi, 16-11-2005 la 00:32 +0100, Pavel Machek a scris:
Quoted 3 lines
...but how do you provide nice, graphical progress bar for swsusp without this? People want that, and "esc to abort", compression, encryption. Too much to be done in kernel space, IMNSHO.
Pavel, you really should _listen_ when someone else is talking about the same things in different implementations. suspend2 has this feature (nice graphical progress bars in userspace) for a long time now and it's compatible with the fedora kernels.
It's also implemented in the kernel, which is exactly the wrong place for this. Pavel is doing this properly, why do you doubt him?
Quoted 2 lines
Why don't you and Nigel (of suspend2) can just work together on this ? It's a shame that much work is wasted in duplicated effort.
It's not duplicated, Nigel knows what need to be done to work together, if he so desires.
thanks,
greg k-h
Hi.
Quoted 6 lines · Greg KH
Quoted 5 lines · Dumitru Ciobarcianu
??n data de Mi, 16-11-2005 la 00:32 +0100, Pavel Machek a scris:
Quoted 3 lines
...but how do you provide nice, graphical progress bar for swsusp without this? People want that, and "esc to abort", compression, encryption. Too much to be done in kernel space, IMNSHO.
Quoted 7 lines
Quoted 4 lines
Pavel, you really should _listen_ when someone else is talking about the same things in different implementations. suspend2 has this feature (nice graphical progress bars in userspace) for a long time now and it's compatible with the fedora kernels.
It's also implemented in the kernel, which is exactly the wrong place for this. Pavel is doing this properly, why do you doubt him?
You yourself called it a hack not long ago. I'm not sure why you think the userspace is the right place for suspending. It seems to me that the very fact that it requires access to structures that are normally only visible to the kernel is pretty telling. To be fair, it is true at the same time that graphical interfaces don't belong in the kernel - but the vast majority of it - calculating what to write and doing the writing does. It's only by hamstringing himself and the user - limiting the image to half of memory that Pavel (and dropping support for writing to swap) that Pavel can make this work.
Quoted 5 lines
Quoted 2 lines
Why don't you and Nigel (of suspend2) can just work together on this ? It's a shame that much work is wasted in duplicated effort.
It's not duplicated, Nigel knows what need to be done to work together, if he so desires.
I know that Pavel and I have such different ideas about what should be done that it's not worth the effort.
Regards,
Nigel
Quoted 9 lines
thanks,
greg k-h
______________________________________________________________________
_______________________________________________
linux-pm mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/linux-pm
Quoted 14 lines · Nigel Cunningham
Hi.
Quoted 11 lines · Greg KH
Quoted 9 lines · Dumitru Ciobarcianu
??n data de Mi, 16-11-2005 la 00:32 +0100, Pavel Machek a scris:
Quoted 3 lines
...but how do you provide nice, graphical progress bar for swsusp without this? People want that, and "esc to abort", compression, encryption. Too much to be done in kernel space, IMNSHO.
Pavel, you really should _listen_ when someone else is talking about the same things in different implementations. suspend2 has this feature (nice graphical progress bars in userspace) for a long time now and it's compatible with the fedora kernels.
Quoted 4 lines
Quoted 2 lines
It's also implemented in the kernel, which is exactly the wrong place for this. Pavel is doing this properly, why do you doubt him?
You yourself called it a hack not long ago.
I did, in the proud tradition of neat hacks. It's a very nice accomplishment that this even works, and I'm impressed.
Quoted 2 lines
I'm not sure why you think the userspace is the right place for suspending.
If he can come up with an implementation that works, and puts stuff like the pretty spinning wheels and progress bars and encryption in userspace, that's great. That stuff doesn't belong in the kerenel if we can possibly help it.
Quoted 3 lines
It seems to me that the very fact that it requires access to structures that are normally only visible to the kernel is pretty telling.
So it needs some work :)
Quoted 6 lines
To be fair, it is true at the same time that graphical interfaces don't belong in the kernel - but the vast majority of it - calculating what to write and doing the writing does. It's only by hamstringing himself and the user - limiting the image to half of memory that Pavel (and dropping support for writing to swap) that Pavel can make this work.
Then propose a better way to do this, if you can see one.
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
Why don't you and Nigel (of suspend2) can just work together on this ? It's a shame that much work is wasted in duplicated effort.
It's not duplicated, Nigel knows what need to be done to work together, if he so desires.
I know that Pavel and I have such different ideas about what should be done that it's not worth the effort.
I'm sorry that you feel this way. I thought that after our meeting in July that things were different.
thanks,
greg k-h
Hi Greg.
Quoted 23 lines · Greg KH
Quoted 14 lines · Nigel Cunningham
Hi.
Quoted 11 lines · Greg KH
Quoted 9 lines · Dumitru Ciobarcianu
??n data de Mi, 16-11-2005 la 00:32 +0100, Pavel Machek a scris:
Quoted 3 lines
...but how do you provide nice, graphical progress bar for swsusp without this? People want that, and "esc to abort", compression, encryption. Too much to be done in kernel space, IMNSHO.
Pavel, you really should _listen_ when someone else is talking about the same things in different implementations. suspend2 has this feature (nice graphical progress bars in userspace) for a long time now and it's compatible with the fedora kernels.
Quoted 4 lines
Quoted 2 lines
It's also implemented in the kernel, which is exactly the wrong place for this. Pavel is doing this properly, why do you doubt him?
You yourself called it a hack not long ago.
I did, in the proud tradition of neat hacks. It's a very nice accomplishment that this even works, and I'm impressed.
Quoted 7 lines
Quoted 2 lines
I'm not sure why you think the userspace is the right place for suspending.
If he can come up with an implementation that works, and puts stuff like the pretty spinning wheels and progress bars and encryption in userspace, that's great. That stuff doesn't belong in the kerenel if we can possibly help it.
I can agree with putting splash screens and userspace stuff in userspace. Suspend2 has had that too, since March. But the guts of the code is a different thing. Encryption - well, I think we're both using cryptoapi now, so that's more easily done in the kernel.
Quoted 5 lines
Quoted 3 lines
It seems to me that the very fact that it requires access to structures that are normally only visible to the kernel is pretty telling.
So it needs some work :)
rm :)
Quoted 8 lines
Quoted 6 lines
To be fair, it is true at the same time that graphical interfaces don't belong in the kernel - but the vast majority of it - calculating what to write and doing the writing does. It's only by hamstringing himself and the user - limiting the image to half of memory that Pavel (and dropping support for writing to swap) that Pavel can make this work.
Then propose a better way to do this, if you can see one.
We've done the user interface in userspace using netlink to communication.
We've done storing a full image of memory by storing the page cache separately to the rest of the image, so that it doesn't need to have an atomic copy made. (Nothing that uses the page cache is running anyway). Having done this, we can use the memory occupied by the page cache for our atomic copy, and just reread the overwritten page cache pages if we need to cancel the suspend. Suspend2 has done this since... beta18 I think.
Quoted 11 lines
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
Why don't you and Nigel (of suspend2) can just work together on this ? It's a shame that much work is wasted in duplicated effort.
It's not duplicated, Nigel knows what need to be done to work together, if he so desires.
I know that Pavel and I have such different ideas about what should be done that it's not worth the effort.
I'm sorry that you feel this way. I thought that after our meeting in July that things were different.
I'm sorry you came away with that impression. I want to work together, but I'm not willing to settle for a minimalist implementation. Pavel, on the other hand, wanted a minimalist implementation at first. He seems to be changing his mind a bit now, but I'm not sure how far that will go.
Regards,
Nigel
Quoted 3 lines
thanks,
greg k-h
Hi, all!
Quoted 11 lines
Quoted 11 lines
Quoted 4 lines
Quoted 2 lines
It's also implemented in the kernel, which is exactly the wrong place for this. Pavel is doing this properly, why do you doubt him?
You yourself called it a hack not long ago.
I did, in the proud tradition of neat hacks. It's a very nice accomplishment that this even works, and I'm impressed.
Quoted 2 lines
I'm not sure why you think the userspace is the right place for suspending.
Quoted 8 lines
Quoted 4 lines
If he can come up with an implementation that works, and puts stuff like the pretty spinning wheels and progress bars and encryption in userspace, that's great. That stuff doesn't belong in the kerenel if we can possibly help it.
I can agree with putting splash screens and userspace stuff in userspace. Suspend2 has had that too, since March. But the guts of the
Well, I'd say that having to resort to netlink is ... not quite nice. You get all the complexity of having userspace running during suspend, and get very little benefit.
Quoted 2 lines
code is a different thing. Encryption - well, I think we're both using cryptoapi now, so that's more easily done in the kernel.
Its not only encryption. It is encryption, compression, support for suspend over network, support for suspend into file. That's quite a lot of stuff.
Quoted 5 lines
Quoted 1 line
Then propose a better way to do this, if you can see one.
We've done the user interface in userspace using netlink to communication.
Quoted 7 lines
We've done storing a full image of memory by storing the page cache separately to the rest of the image, so that it doesn't need to have an atomic copy made. (Nothing that uses the page cache is running anyway). Having done this, we can use the memory occupied by the page cache for our atomic copy, and just reread the overwritten page cache pages if we need to cancel the suspend. Suspend2 has done this since... beta18 I think.
...at expense of complexity, and hooks all over the kernel. Yes, if you modify kernel a bit, nothing will use the page cache.
Anyway, I believe we have solution for that one. See Rafael's recent patches -- "only free as much memory as neccessary" should do the trick, without excessive complexity.
Quoted 10 lines
Quoted 5 lines
Quoted 2 lines
I know that Pavel and I have such different ideas about what should be done that it's not worth the effort.
I'm sorry that you feel this way. I thought that after our meeting in July that things were different.
I'm sorry you came away with that impression. I want to work together, but I'm not willing to settle for a minimalist implementation. Pavel, on the other hand, wanted a minimalist implementation at first. He seems to be changing his mind a bit now, but I'm not sure how far that will go.
Well, I do not want the complexity of two page sets. I think Rafael's patches will provide almost equivalent functionality. Other than that, all your features should be doable. I'm not saying I'm going to write those patches myself, but I'll certainly not reject them just because they are too big.
Pavel
Signature · 1 line
Thanks, Sharp!
Hi
Quoted 18 lines · Pavel Machek
Hi, all!
Quoted 16 lines
Quoted 15 lines
Quoted 4 lines
Quoted 2 lines
It's also implemented in the kernel, which is exactly the wrong place for this. Pavel is doing this properly, why do you doubt him?
You yourself called it a hack not long ago.
I did, in the proud tradition of neat hacks. It's a very nice accomplishment that this even works, and I'm impressed.
Quoted 2 lines
I'm not sure why you think the userspace is the right place for suspending.
If he can come up with an implementation that works, and puts stuff like the pretty spinning wheels and progress bars and encryption in userspace, that's great. That stuff doesn't belong in the kerenel if we can possibly help it.
Quoted 7 lines
Quoted 3 lines
I can agree with putting splash screens and userspace stuff in userspace. Suspend2 has had that too, since March. But the guts of the
Well, I'd say that having to resort to netlink is ... not quite nice. You get all the complexity of having userspace running during suspend, and get very little benefit.
Mmm, but less complexity than with trying to do the whole suspend from userspace. (I don't need to export pageflags, bio routines etc or work around it by using /dev/kmem).
Quoted 20 lines
Quoted 2 lines
code is a different thing. Encryption - well, I think we're both using cryptoapi now, so that's more easily done in the kernel.
Its not only encryption. It is encryption, compression, support for suspend over network, support for suspend into file. That's quite a lot of stuff.
Quoted 5 lines
Quoted 1 line
Then propose a better way to do this, if you can see one.
We've done the user interface in userspace using netlink to communication.
Quoted 7 lines
We've done storing a full image of memory by storing the page cache separately to the rest of the image, so that it doesn't need to have an atomic copy made. (Nothing that uses the page cache is running anyway). Having done this, we can use the memory occupied by the page cache for our atomic copy, and just reread the overwritten page cache pages if we need to cancel the suspend. Suspend2 has done this since... beta18 I think.
Quoted 2 lines
...at expense of complexity, and hooks all over the kernel. Yes, if you modify kernel a bit, nothing will use the page cache.
Could you back your "hooks all over the kernel" statement up? I do have some BUG_ON()s aimed at double checking that nothing bad happens, but they never get hit and obviously aren't required to stop processes using the page cache. All that's really required is to freeze processes.
Quoted 3 lines
Anyway, I believe we have solution for that one. See Rafael's recent patches -- "only free as much memory as neccessary" should do the trick, without excessive complexity.
That's still imposing a 1/2 of memory limit, though.
Quoted 11 lines
Quoted 10 lines
Quoted 5 lines
Quoted 2 lines
I know that Pavel and I have such different ideas about what should be done that it's not worth the effort.
I'm sorry that you feel this way. I thought that after our meeting in July that things were different.
I'm sorry you came away with that impression. I want to work together, but I'm not willing to settle for a minimalist implementation. Pavel, on the other hand, wanted a minimalist implementation at first. He seems to be changing his mind a bit now, but I'm not sure how far that will go.
Quoted 5 lines
Well, I do not want the complexity of two page sets. I think Rafael's patches will provide almost equivalent functionality. Other than that, all your features should be doable. I'm not saying I'm going to write those patches myself, but I'll certainly not reject them just because they are too big.
I'm sorry for making you think that having two pagesets is a complex issues. I know that when I first did it, I put tight restrictions on memory usage while the first pageset was written and used a separate memory pool. Since then, I've realised a far simpler way of handling this, and the code has been greatly simplified. In essence, all you need to do is make your I/O code generic enough that it can be passed a list of pages to write and put page cache pages in a separate list when figuring out what pages need to be saved. Then you save those pages before doing your atomic copy of the other pages, and reload them after restoring the atomic copy at resume time.
Regards,
Nigel
Hi!
Quoted 11 lines
Quoted 7 lines
Quoted 3 lines
I can agree with putting splash screens and userspace stuff in userspace. Suspend2 has had that too, since March. But the guts of the
Well, I'd say that having to resort to netlink is ... not quite nice. You get all the complexity of having userspace running during suspend, and get very little benefit.
Mmm, but less complexity than with trying to do the whole suspend from userspace. (I don't need to export pageflags, bio routines etc or work around it by using
/dev/kmem).
Well, userland swsusp has pretty low impact on kernel code -- it adds something like 150 lines:
drivers/char/mem.c | 42 + include/linux/suspend.h | 23 kernel/power/console.c | 1 kernel/power/disk.c | 19 kernel/power/swsusp.c | 78 + usr/swsusp-init | 9 8 files changed, 2696 insertions(+), 4 deletions(-)
i don't think you can do much better than that...
Quoted 7 lines
Quoted 2 lines
...at expense of complexity, and hooks all over the kernel. Yes, if you modify kernel a bit, nothing will use the page cache.
Could you back your "hooks all over the kernel" statement up? I do have some BUG_ON()s aimed at double checking that nothing bad happens, but they never get hit and obviously aren't required to stop processes using the page cache. All that's really required is to freeze processes.
Are you willing to merge the code without BUG_ONs?
Quoted 5 lines
Quoted 3 lines
Anyway, I believe we have solution for that one. See Rafael's recent patches -- "only free as much memory as neccessary" should do the trick, without excessive complexity.
That's still imposing a 1/2 of memory limit, though.
Yes, hopefully users will not notice.
Quoted 6 lines
Quoted 5 lines
Well, I do not want the complexity of two page sets. I think Rafael's patches will provide almost equivalent functionality. Other than that, all your features should be doable. I'm not saying I'm going to write those patches myself, but I'll certainly not reject them just because they are too big.
Quoted 10 lines
I'm sorry for making you think that having two pagesets is a complex issues. I know that when I first did it, I put tight restrictions on memory usage while the first pageset was written and used a separate memory pool. Since then, I've realised a far simpler way of handling this, and the code has been greatly simplified. In essence, all you need to do is make your I/O code generic enough that it can be passed a list of pages to write and put page cache pages in a separate list when figuring out what pages need to be saved. Then you save those pages before doing your atomic copy of the other pages, and reload them after restoring the atomic copy at resume time.
Okay, it may have gotten better. Anyway, this is the only part that really needs to be in-kernel. Saving 50% of memory is still going to produce *way* more responsive system than "save as little as possible", and I hope it will be good enough.
Pavel
Signature · 1 line
Thanks, Sharp!
Hi.
Quoted 30 lines · Pavel Machek
Hi!
Quoted 11 lines
Quoted 7 lines
Quoted 3 lines
I can agree with putting splash screens and userspace stuff in userspace. Suspend2 has had that too, since March. But the guts of the
Well, I'd say that having to resort to netlink is ... not quite nice. You get all the complexity of having userspace running during suspend, and get very little benefit.
Mmm, but less complexity than with trying to do the whole suspend from userspace. (I don't need to export pageflags, bio routines etc or work around it by using
/dev/kmem).Well, userland swsusp has pretty low impact on kernel code -- it adds something like 150 lines:
drivers/char/mem.c | 42 + include/linux/suspend.h | 23 kernel/power/console.c | 1 kernel/power/disk.c | 19 kernel/power/swsusp.c | 78 + usr/swsusp-init | 9 8 files changed, 2696 insertions(+), 4 deletions(-)i don't think you can do much better than that...
Quoted 3 lines
Quoted 2 lines
...at expense of complexity, and hooks all over the kernel. Yes, if you modify kernel a bit, nothing will use the page cache.
Quoted 6 lines
Quoted 4 lines
Could you back your "hooks all over the kernel" statement up? I do have some BUG_ON()s aimed at double checking that nothing bad happens, but they never get hit and obviously aren't required to stop processes using the page cache. All that's really required is to freeze processes.
Are you willing to merge the code without
BUG_ONs?
Yes.
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
Anyway, I believe we have solution for that one. See Rafael's recent patches -- "only free as much memory as neccessary" should do the trick, without excessive complexity.
That's still imposing a 1/2 of memory limit, though.
Yes, hopefully users will not notice.
Users with more memory probably won't care so much, depending on what apps they want to run and how responsive they want the system to be post-resume.
Quoted 17 lines
Quoted 6 lines
Quoted 5 lines
Well, I do not want the complexity of two page sets. I think Rafael's patches will provide almost equivalent functionality. Other than that, all your features should be doable. I'm not saying I'm going to write those patches myself, but I'll certainly not reject them just because they are too big.
Quoted 10 lines
I'm sorry for making you think that having two pagesets is a complex issues. I know that when I first did it, I put tight restrictions on memory usage while the first pageset was written and used a separate memory pool. Since then, I've realised a far simpler way of handling this, and the code has been greatly simplified. In essence, all you need to do is make your I/O code generic enough that it can be passed a list of pages to write and put page cache pages in a separate list when figuring out what pages need to be saved. Then you save those pages before doing your atomic copy of the other pages, and reload them after restoring the atomic copy at resume time.
Quoted 4 lines
Okay, it may have gotten better. Anyway, this is the only part that really needs to be in-kernel. Saving 50% of memory is still going to produce *way* more responsive system than "save as little as possible", and I hope it will be good enough.
I agree about the 'way more responsive system'. Good enough will depend on the user and which way the wind is blowing at the time. I guess if that's the only option they have, it's still better than rebooting.
Regards,
Nigel
Quoted 1 line · Pavel Machek
Yes, hopefully users will not notice.
? So the idea is to merge inferior code and "hope users won't notice"?
Users might not notice that half their memory is gone, but they *will* notice that their system is dog slow when it resumes because all their caches are gone and a most of their stuff is swapped out.
Non-responsive system on resume is one of the main reasons that swsusp2 is much better than swsusp1, and yes, users *do* notice (I was one of them, as I pointed out a while back). Yes, 50% is better than nothing, but it's still a pretty poor show.
Seen from the perspective of a user, the situation is simple: suspend2 works, it's fast, and it's rock-solid. Just use it.
Regards,
Lorenzo
P.S. Don't "show me the code" me. I can't write the code. :-) But based on what I see of how well suspend2 works, I think Nigel can...
Quoted 5 lines · Lorenzo Colitti
Quoted 1 line · Pavel Machek
Yes, hopefully users will not notice.
? So the idea is to merge inferior code and "hope users won't notice"?
Quoted 8 lines
Users might not notice that half their memory is gone, but they *will* notice that their system is dog slow when it resumes because all their caches are gone and a most of their stuff is swapped out.
Non-responsive system on resume is one of the main reasons that swsusp2 is much better than swsusp1, and yes, users *do* notice (I was one of them, as I pointed out a while back). Yes, 50% is better than nothing, but it's still a pretty poor show.
Did you actually benchmark it?
Quoted 2 lines
Seen from the perspective of a user, the situation is simple: suspend2 works, it's fast, and it's rock-solid. Just use it.
About as helpful as "Windows XP works, it's fast, and it's rock-solid".
Pavel
Signature · 1 line
Thanks, Sharp!
Quoted 5 lines · Nigel Cunningham
Quoted 2 lines
It's not duplicated, Nigel knows what need to be done to work together, if he so desires.
I know that Pavel and I have such different ideas about what should be done that it's not worth the effort.
So first it was Pavel and Patrick Mochel...
Then Pavel and Nigel...
Recently Dave Jones rumbled about a suspend fork...
You sure you software suspend guys haven't been hanging out with the IDE maintainers?
Rob
Hi!
Quoted 11 lines
Quoted 5 lines
Quoted 2 lines
It's not duplicated, Nigel knows what need to be done to work together, if he so desires.
I know that Pavel and I have such different ideas about what should be done that it's not worth the effort.
So first it was Pavel and Patrick Mochel...
Then Pavel and Nigel...
Recently Dave Jones rumbled about a suspend fork...
Pavel and Patrick is solved, and there's no Pavel and Nigel... Its just Pavel vs. way too much code. See my reply to Dave.
Quoted 2 lines
You sure you software suspend guys haven't been hanging out with the IDE maintainers?
:-)
Signature · 1 line
64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms
Quoted 4 lines · Dave Jones
Just for info: If this goes in, Red Hat/Fedora kernels will fork swsusp development, as this method just will not work there. (We have a restricted
/dev/memthat prevents writes to arbitary
memory regions, as part of a patchset to prevent rootkits)
Perhaps it is trying to tell you that you should be using SELinux rules not kernel hacks for this purpose ?
Quoted 2 lines
Even it were not for this, the whole idea seems misconcieved to me anyway.
I'm sceptical too but several Win9x BIOS vendor suspend paths were implemented in roughly this way. I don't however see how you can co-ordinate the freeze with outstanding O_DIRECT DMA to user pages for one item.
Quoted 8 lines · Alan Cox
Quoted 4 lines · Dave Jones
Just for info: If this goes in, Red Hat/Fedora kernels will fork swsusp development, as this method just will not work there. (We have a restricted
/dev/memthat prevents writes to arbitary
memory regions, as part of a patchset to prevent rootkits)Perhaps it is trying to tell you that you should be using SELinux rules not kernel hacks for this purpose ?
I don't think selinux can give you the granularity to say "process can access this bit of the file only", at least not yet.
Even if that was capable however, it still doesn't solve the problem. Pavel's implementation wants to write to arbitary address spaces, which is what we're trying to prevent. The two are at odds with each other.
Dave
Quoted 3 lines · Dave Jones
Even if that was capable however, it still doesn't solve the problem. Pavel's implementation wants to write to arbitary address spaces, which is what we're trying to prevent. The two are at odds with each other.
I agree, he needs to find a different way to get that information into and out of the kernel than that device node for it to be accepted into mainline. But for now, it's a nice way to mock up the fuctionality needed.
thanks,
greg k-h
Hi!
Quoted 8 lines
Quoted 7 lines
Quoted 4 lines
Just for info: If this goes in, Red Hat/Fedora kernels will fork swsusp development, as this method just will not work there. (We have a restricted
/dev/memthat prevents writes to arbitary
memory regions, as part of a patchset to prevent rootkits)Perhaps it is trying to tell you that you should be using SELinux rules not kernel hacks for this purpose ?
Quoted 6 lines
I don't think selinux can give you the granularity to say "process can access this bit of the file only", at least not yet.
Even if that was capable however, it still doesn't solve the problem. Pavel's implementation wants to write to arbitary address spaces, which is what we're trying to prevent. The two are at odds with each other.
I do not think thats a security problem. By definition, suspending code can change arbitrary things in memory -- it could just write image with changes it desires, then resume from it. Whether this code is in kernel or not, it has to be trusted.
Signature · 1 line
64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms
Quoted 13 lines · Pavel Machek
Hi!
Quoted 11 lines
Quoted 7 lines
Quoted 4 lines
Just for info: If this goes in, Red Hat/Fedora kernels will fork swsusp development, as this method just will not work there. (We have a restricted
/dev/memthat prevents writes to arbitary
memory regions, as part of a patchset to prevent rootkits)Perhaps it is trying to tell you that you should be using SELinux rules not kernel hacks for this purpose ?
I don't think selinux can give you the granularity to say "process can access this bit of the file only", at least not yet.
Quoted 8 lines
Quoted 3 lines
Even if that was capable however, it still doesn't solve the problem. Pavel's implementation wants to write to arbitary address spaces, which is what we're trying to prevent. The two are at odds with each other.
I do not think thats a security problem. By definition, suspending code can change arbitrary things in memory -- it could just write image with changes it desires, then resume from it. Whether this code is in kernel or not, it has to be trusted.
Stop thinking about the suspend usage case for a minute.
With your proposed changes, an attacker can scribble over random bits of /dev/mem without suspending in order to do whatever he wants.
With what we have in-kernel, and a restricted /dev/mem, achieving the attack you mention is a lot less feasible, as the attacker has no access to the memory being written out to the suspend partition, even as root. Even if they did, people tend to notice boxes shutting down pretty quickly making this a not-very-stealthy attack.
Dave
Hi!
Quoted 20 lines
Quoted 11 lines
Quoted 11 lines
Quoted 7 lines
Quoted 4 lines
Just for info: If this goes in, Red Hat/Fedora kernels will fork swsusp development, as this method just will not work there. (We have a restricted
/dev/memthat prevents writes to arbitary
memory regions, as part of a patchset to prevent rootkits)Perhaps it is trying to tell you that you should be using SELinux rules not kernel hacks for this purpose ?
I don't think selinux can give you the granularity to say "process can access this bit of the file only", at least not yet.
Quoted 8 lines
Quoted 3 lines
Even if that was capable however, it still doesn't solve the problem. Pavel's implementation wants to write to arbitary address spaces, which is what we're trying to prevent. The two are at odds with each other.
I do not think thats a security problem. By definition, suspending code can change arbitrary things in memory -- it could just write image with changes it desires, then resume from it. Whether this code is in kernel or not, it has to be trusted.
Quoted 5 lines
Stop thinking about the suspend usage case for a minute.
With your proposed changes, an attacker can scribble over random bits of
/dev/memwithout suspending in order to do whatever he wants.
Well, without my changes, an attacker can scribble over random bits of memory, too; I was not the one that introduced /dev/mem :-).
Quoted 5 lines
With what we have in-kernel, and a restricted
/dev/mem, achieving the attack you mention is a lot less feasible, as the attacker has no access to the memory being written out to the suspend partition, even as root. Even if they did, people tend to notice boxes shutting down pretty quickly making this a not-very-stealthy attack.
Can I read somewhere about security model you are using? Would it be enough to restrict /dev/[k]mem to those people that have right to update kernel anyway? Or your approach is "noone, absolutely noone has right to modify running kernel"? [Do you still use loadable modules?]
Pavel
Signature · 1 line
Thanks, Sharp!
Hi,
On Sunday, 20 of November 2005 23:09, Pavel Machek wrote:
}-- snip --{
Quoted 10 lines
Quoted 5 lines
With what we have in-kernel, and a restricted
/dev/mem, achieving the attack you mention is a lot less feasible, as the attacker has no access to the memory being written out to the suspend partition, even as root. Even if they did, people tend to notice boxes shutting down pretty quickly making this a not-very-stealthy attack.Can I read somewhere about security model you are using? Would it be enough to restrict /dev/[k]mem to those people that have right to update kernel anyway? Or your approach is "noone, absolutely noone has right to modify running kernel"? [Do you still use loadable modules?]
The problem is that, whatever the security model, if you have access to the kernel memory (eg. via /dev/kmem), you can modify the security rules themselves, so this should better be avoided.
Apart from this, IMO, if it's necessary to access the kernel memory directly from a userland process, this means that the process' functionality really belongs to the kernel. Consequently, the code in swsusp that needs to access the kernel memory should stay in the kernel, and the rest can go to the userland.
Greetings,
Rafael
Hi!
Quoted 6 lines
Quoted 6 lines
Quoted 5 lines
With what we have in-kernel, and a restricted
/dev/mem, achieving the attack you mention is a lot less feasible, as the attacker has no access to the memory being written out to the suspend partition, even as root. Even if they did, people tend to notice boxes shutting down pretty quickly making this a not-very-stealthy attack.
Quoted 8 lines
Quoted 4 lines
Can I read somewhere about security model you are using? Would it be enough to restrict /dev/[k]mem to those people that have right to update kernel anyway? Or your approach is "noone, absolutely noone has right to modify running kernel"? [Do you still use loadable modules?]
The problem is that, whatever the security model, if you have access to the kernel memory (eg. via
/dev/kmem), you can modify the security rules themselves, so this should better be avoided.
Well, under current linux security model, root has all permissions, including inserting modifying running kernel, touching hardware directly, and installing rootkits. Fedora may be trying to change that... but if so, I'd like to know what they are planning.
Pavel
Signature · 1 line
Thanks, Sharp!
Quoted 8 lines · Alan Cox
Quoted 4 lines · Dave Jones
Just for info: If this goes in, Red Hat/Fedora kernels will fork swsusp development, as this method just will not work there. (We have a restricted
/dev/memthat prevents writes to arbitary
memory regions, as part of a patchset to prevent rootkits)Perhaps it is trying to tell you that you should be using SELinux rules not kernel hacks for this purpose ?
actually no. SELinux can't work, we've looked at that bigtime. Basically /dev/mem has 3 types in one, and to apply security you need different roles for each in selinux. so the only option to apply selinux *anything* is to first split /dev/mem up.
types:
1) accessing non-ram memory (eg PCI mmio space) by X and the likes (ideally should use sysfs but hey, changing X for this will take forever)
2) accessing bios memory in the lower 1Gb for various emulation like purposes (including vbetool and X mode setting)
3) accessing things the kernel sees as RAM
they are very distinct security wise.
Quoted 3 lines · Arjan van de Ven
1) accessing non-ram memory (eg PCI mmio space) by X and the likes (ideally should use sysfs but hey, changing X for this will take forever)
Once sysfs supports the relevant capabilities fixing X actually doesn't look too horrible, the PCI mapping routines are abstracted and done by PCITAG (ie PCI device). You would need the ISA hole too in some cases.
Quoted 5 lines
2) accessing bios memory in the lower 1Gb for various emulation like purposes (including vbetool and X mode setting)
3) accessing things the kernel sees as RAMthey are very distinct security wise.
Agreed.
Quoted 9 lines · Alan Cox
Quoted 3 lines · Arjan van de Ven
1) accessing non-ram memory (eg PCI mmio space) by X and the likes (ideally should use sysfs but hey, changing X for this will take forever)
Once sysfs supports the relevant capabilities fixing X actually doesn't look too horrible, the PCI mapping routines are abstracted and done by PCITAG (ie PCI device). You would need the ISA hole too in some cases.
It's actually partly done already (at least for ia64, but the code I put together works on x86 too, iirc, and should work elsewhere). The ISA stuff is exported on a per-bus basis in legacy_io and legacy_mem files.
If vbetool and friends want to get at the ROM, they can use the sysfs rom file like everyone else. There are problems with this however, on systems where the ROM is unpacked at 0xc0000 or something, especially if the unpacked version is modified by the BIOS at startup time, not sure how to address that reliably.
Jesse
Quoted 7 lines · Alan Cox
Quoted 3 lines · Arjan van de Ven
1) accessing non-ram memory (eg PCI mmio space) by X and the likes (ideally should use sysfs but hey, changing X for this will take forever)
Once sysfs supports the relevant capabilities fixing X actually doesn't look too horrible
I think the kernel already supports this since at least july if not earlier. If there's something missing... someone needs to speak up..
(yes vga arbitrage is missing but well that's not there today by any means either so not a regression)
Quoted 2 lines
, the PCI mapping routines are abstracted and done by PCITAG (ie PCI device). You would need the ISA hole too in some cases.
this may need /dev/mem a bit longer, but hopefully is rarer. Once the pci side is fixed I bet this only is easy to do as well
Hi!
Quoted 7 lines
Quoted 2 lines
Even it were not for this, the whole idea seems misconcieved to me anyway.
I'm sceptical too but several Win9x BIOS vendor suspend paths were implemented in roughly this way. I don't however see how you can co-ordinate the freeze with outstanding
O_DIRECTDMA to user pages for one item.
I do not see a problem. swsusp process stops all other processes, freezes the drivers, then asks for system snapshot. It certainly does *not* ask for O_DIRECT........
Signature · 1 line
64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms