linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* pmdisk suspend patch
@ 2004-03-14 15:19 Giridhar Pemmasani
  2004-03-15  6:35 ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 4+ messages in thread
From: Giridhar Pemmasani @ 2004-03-14 15:19 UTC (permalink / raw)
  To: linux-kernel; +Cc: swsusp-devel

[-- Attachment #1: Type: text/plain, Size: 6474 bytes --]

Hello

I have been trying to get a reliable way to suspend-to-disk. For me
swsusp was taking more than a few minutes and swsusp2 was not
reliable. So I decided to give pmdisk a chance. Since my laptop
supported S4bios, pmdisk wouldn't let me change it to S4 platform
based suspend. Even after fixing it, I didn't have much luck. I dug
deeper and found another problem. pmdisk resumes all devices while
suspending. So I patched the pmdisk to resume only the device on which
suspend image is to be stored. With these patches, I have been able to
use pmdisk reliably (tested with two laptops over 100 times).

I am not subscribed to the mailing list, so if you have problems or
feedback, CC to me.

diff -Nur linux-2.6.4.orig/kernel/power/disk.c
linux-2.6.4/kernel/power/disk.c
--- linux-2.6.4.orig/kernel/power/disk.c        2004-03-13 10:38:32.000000000 -0500
+++ linux-2.6.4/kernel/power/disk.c     2004-03-14 01:27:33.000000000 -0500
@@ -280,14 +280,13 @@
        return sprintf(buf,"%s\n",pm_disk_modes[pm_disk_mode]);
 }
 
-
+extern int acpi_pm_disk_mode(int);
 static ssize_t disk_store(struct subsystem * s, const char * buf, size_t n)
 {
        int error = 0;
        int i;
        u32 mode = 0;
 
-       down(&pm_sem);
        for (i = PM_DISK_FIRMWARE; i < PM_DISK_MAX; i++) {
                if (!strcmp(buf,pm_disk_modes[i])) {
                        mode = i;
@@ -295,21 +294,18 @@
                }
        }
        if (mode) {
-               if (mode == PM_DISK_SHUTDOWN || mode == PM_DISK_REBOOT)
+               if (mode == PM_DISK_SHUTDOWN || mode == PM_DISK_REBOOT ||
+                               !acpi_pm_disk_mode(mode)) {
+                       down(&pm_sem);
                        pm_disk_mode = mode;
-               else {
-                       if (pm_ops && pm_ops->enter &&
-                           (mode == pm_ops->pm_disk_mode))
-                               pm_disk_mode = mode;
-                       else
-                               error = -EINVAL;
-               }
+                       up(&pm_sem);
+               } else
+                       error = -EINVAL;
        } else
                error = -EINVAL;
 
        pr_debug("PM: suspend-to-disk mode set to '%s'\n",
                 pm_disk_modes[mode]);
-       up(&pm_sem);
        return error ? error : n;
 }
 
diff -Nur linux-2.6.4.orig/kernel/power/pmdisk.c
linux-2.6.4/kernel/power/pmdisk.c
--- linux-2.6.4.orig/kernel/power/pmdisk.c      2004-03-13 10:38:32.000000000
-0500
+++ linux-2.6.4/kernel/power/pmdisk.c   2004-03-14 01:34:07.000000000 -0500
@@ -20,6 +20,7 @@
 
 #undef DEBUG
 
+#include <linux/fs.h>
 #include <linux/mm.h>
 #include <linux/bio.h>
 #include <linux/suspend.h>
@@ -29,6 +30,7 @@
 #include <linux/swapops.h>
 #include <linux/bootmem.h>
 #include <linux/utsname.h>
+#include <linux/ide.h>
 
 #include <asm/mmu_context.h>
 
@@ -53,7 +55,7 @@
 /* For resume= kernel option */
 static char resume_file[256] = CONFIG_PM_DISK_PARTITION;
 
-static dev_t resume_device;
+static dev_t resume_dev;
 /* Local variables that should not be affected by save */
 unsigned int pmdisk_pages __nosavedata = 0;
 
@@ -158,7 +160,7 @@
                        } else {
                                /* we ignore all swap devices that are not the resume_file */
                                if (1) {
-// FIXME                               if(resume_device == swap_info[i].swap_device) {
+// FIXME                               if(resume_dev == swap_info[i].swap_device) {
                                        swapfile_used[i] = SWAPFILE_SUSPEND;
                                        root_swap = i;
                                } else
@@ -578,8 +580,8 @@
 static int enough_free_mem(void)
 {
        if(nr_free_pages() < (pmdisk_pages + PAGES_FOR_IO)) {
-               pr_debug("pmdisk: Not enough free pages: Have %d\n",
-                        nr_free_pages());
+               pr_debug("Not enough free pages: need %d, have %d\n",
+                        pmdisk_pages + PAGES_FOR_IO, nr_free_pages());
                return 0;
        }
        return 1;
@@ -593,7 +595,7 @@
  *     space avaiable.
  *
  *     FIXME: si_swapinfo(&i) returns all swap devices information.
- *     We should only consider resume_device. 
+ *     We should only consider resume_dev. 
  */
 
 static int enough_swap(void)
@@ -689,10 +691,24 @@
  *     correctly, we'll mark system clean, anyway.)
  */
 
+extern int resume_device(struct device *);
 static int suspend_save_image(void)
 {
        int error;
-       device_resume();
+       struct block_device *bdev;
+       struct device *dev;
+
+       /* resume the device on which suspend image is stored */
+       bdev = lookup_bdev(resume_file);
+       if (!bdev)
+               return -EINVAL;
+       dev = bdev->bd_disk->driverfs_dev;
+       if (!dev)
+               return -EINVAL;
+
+       error = resume_device(dev);
+       if (error)
+               return -EINVAL;
        lock_swapdevices();
        error = write_suspend_image();
        lock_swapdevices();
@@ -1118,10 +1134,10 @@
        if (!strlen(resume_file))
                return -ENOENT;
 
-       resume_device = name_to_dev_t(resume_file);
+       resume_dev = name_to_dev_t(resume_file);
        pr_debug("pmdisk: Resume From Partition: %s\n", resume_file);
 
-       resume_bdev = open_by_devnum(resume_device, FMODE_READ);
+       resume_bdev = open_by_devnum(resume_dev, FMODE_READ);
        if (!IS_ERR(resume_bdev)) {
                set_blocksize(resume_bdev, PAGE_SIZE);
                error = read_suspend_image();
diff -Nur linux-2.6.4.orig/drivers/acpi/sleep/main.c
linux-2.6.4/drivers/acpi/sleep/main.c
--- linux-2.6.4.orig/drivers/acpi/sleep/main.c  2004-01-09 01:59:04.000000000
-0500
+++ linux-2.6.4/drivers/acpi/sleep/main.c       2004-03-14 01:46:42.000000000
-0500
@@ -166,6 +166,22 @@
        .finish         = acpi_pm_finish,
 };
 
+int acpi_pm_disk_mode(int mode)
+{
+
+       if (sleep_states[ACPI_STATE_S4])
+               if (mode == PM_DISK_PLATFORM ||
+                       (mode == PM_DISK_FIRMWARE && acpi_gbl_FACS->S4bios_f)) {
+                       acpi_pm_ops.pm_disk_mode = mode;
+                       pm_set_ops(&acpi_pm_ops);
+                       return 0;
+               }
+               else
+                       return -EINVAL;
+       else
+               return -EINVAL;
+}
+
 static int __init acpi_sleep_init(void)
 {
        int                     i = 0;


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: pmdisk_patch --]
[-- Type: text/x-diff; name="pmdisk_patch", Size: 4579 bytes --]

diff -Nur linux-2.6.4.orig/kernel/power/disk.c linux-2.6.4/kernel/power/disk.c
--- linux-2.6.4.orig/kernel/power/disk.c	2004-03-13 10:38:32.000000000 -0500
+++ linux-2.6.4/kernel/power/disk.c	2004-03-14 01:27:33.000000000 -0500
@@ -280,14 +280,13 @@
 	return sprintf(buf,"%s\n",pm_disk_modes[pm_disk_mode]);
 }
 
-
+extern int acpi_pm_disk_mode(int);
 static ssize_t disk_store(struct subsystem * s, const char * buf, size_t n)
 {
 	int error = 0;
 	int i;
 	u32 mode = 0;
 
-	down(&pm_sem);
 	for (i = PM_DISK_FIRMWARE; i < PM_DISK_MAX; i++) {
 		if (!strcmp(buf,pm_disk_modes[i])) {
 			mode = i;
@@ -295,21 +294,18 @@
 		}
 	}
 	if (mode) {
-		if (mode == PM_DISK_SHUTDOWN || mode == PM_DISK_REBOOT)
+		if (mode == PM_DISK_SHUTDOWN || mode == PM_DISK_REBOOT ||
+				!acpi_pm_disk_mode(mode)) {
+			down(&pm_sem);
 			pm_disk_mode = mode;
-		else {
-			if (pm_ops && pm_ops->enter &&
-			    (mode == pm_ops->pm_disk_mode))
-				pm_disk_mode = mode;
-			else
-				error = -EINVAL;
-		}
+			up(&pm_sem);
+		} else
+			error = -EINVAL;
 	} else
 		error = -EINVAL;
 
 	pr_debug("PM: suspend-to-disk mode set to '%s'\n",
 		 pm_disk_modes[mode]);
-	up(&pm_sem);
 	return error ? error : n;
 }
 
diff -Nur linux-2.6.4.orig/kernel/power/pmdisk.c linux-2.6.4/kernel/power/pmdisk.c
--- linux-2.6.4.orig/kernel/power/pmdisk.c	2004-03-13 10:38:32.000000000 -0500
+++ linux-2.6.4/kernel/power/pmdisk.c	2004-03-14 01:34:07.000000000 -0500
@@ -20,6 +20,7 @@
 
 #undef DEBUG
 
+#include <linux/fs.h>
 #include <linux/mm.h>
 #include <linux/bio.h>
 #include <linux/suspend.h>
@@ -29,6 +30,7 @@
 #include <linux/swapops.h>
 #include <linux/bootmem.h>
 #include <linux/utsname.h>
+#include <linux/ide.h>
 
 #include <asm/mmu_context.h>
 
@@ -53,7 +55,7 @@
 /* For resume= kernel option */
 static char resume_file[256] = CONFIG_PM_DISK_PARTITION;
 
-static dev_t resume_device;
+static dev_t resume_dev;
 /* Local variables that should not be affected by save */
 unsigned int pmdisk_pages __nosavedata = 0;
 
@@ -158,7 +160,7 @@
 			} else {
 	  			/* we ignore all swap devices that are not the resume_file */
 				if (1) {
-// FIXME				if(resume_device == swap_info[i].swap_device) {
+// FIXME				if(resume_dev == swap_info[i].swap_device) {
 					swapfile_used[i] = SWAPFILE_SUSPEND;
 					root_swap = i;
 				} else
@@ -578,8 +580,8 @@
 static int enough_free_mem(void)
 {
 	if(nr_free_pages() < (pmdisk_pages + PAGES_FOR_IO)) {
-		pr_debug("pmdisk: Not enough free pages: Have %d\n",
-			 nr_free_pages());
+		pr_debug("Not enough free pages: need %d, have %d\n",
+			 pmdisk_pages + PAGES_FOR_IO, nr_free_pages());
 		return 0;
 	}
 	return 1;
@@ -593,7 +595,7 @@
  *	space avaiable.
  *
  *	FIXME: si_swapinfo(&i) returns all swap devices information.
- *	We should only consider resume_device. 
+ *	We should only consider resume_dev. 
  */
 
 static int enough_swap(void)
@@ -689,10 +691,24 @@
  *	correctly, we'll mark system clean, anyway.)
  */
 
+extern int resume_device(struct device *);
 static int suspend_save_image(void)
 {
 	int error;
-	device_resume();
+	struct block_device *bdev;
+	struct device *dev;
+
+	/* resume the device on which suspend image is stored */
+	bdev = lookup_bdev(resume_file);
+	if (!bdev)
+		return -EINVAL;
+	dev = bdev->bd_disk->driverfs_dev;
+	if (!dev)
+		return -EINVAL;
+
+	error = resume_device(dev);
+	if (error)
+		return -EINVAL;
 	lock_swapdevices();
 	error = write_suspend_image();
 	lock_swapdevices();
@@ -1118,10 +1134,10 @@
 	if (!strlen(resume_file))
 		return -ENOENT;
 
-	resume_device = name_to_dev_t(resume_file);
+	resume_dev = name_to_dev_t(resume_file);
 	pr_debug("pmdisk: Resume From Partition: %s\n", resume_file);
 
-	resume_bdev = open_by_devnum(resume_device, FMODE_READ);
+	resume_bdev = open_by_devnum(resume_dev, FMODE_READ);
 	if (!IS_ERR(resume_bdev)) {
 		set_blocksize(resume_bdev, PAGE_SIZE);
 		error = read_suspend_image();
diff -Nur linux-2.6.4.orig/drivers/acpi/sleep/main.c linux-2.6.4/drivers/acpi/sleep/main.c
--- linux-2.6.4.orig/drivers/acpi/sleep/main.c	2004-01-09 01:59:04.000000000 -0500
+++ linux-2.6.4/drivers/acpi/sleep/main.c	2004-03-14 01:46:42.000000000 -0500
@@ -166,6 +166,22 @@
 	.finish		= acpi_pm_finish,
 };
 
+int acpi_pm_disk_mode(int mode)
+{
+
+	if (sleep_states[ACPI_STATE_S4])
+		if (mode == PM_DISK_PLATFORM ||
+			(mode == PM_DISK_FIRMWARE && acpi_gbl_FACS->S4bios_f)) {
+			acpi_pm_ops.pm_disk_mode = mode;
+			pm_set_ops(&acpi_pm_ops);
+			return 0;
+		}
+		else
+			return -EINVAL;
+	else
+		return -EINVAL;
+}
+
 static int __init acpi_sleep_init(void)
 {
 	int			i = 0;

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: pmdisk suspend patch
  2004-03-14 15:19 pmdisk suspend patch Giridhar Pemmasani
@ 2004-03-15  6:35 ` Benjamin Herrenschmidt
  2004-03-15  8:13   ` Giridhar Pemmasani
  0 siblings, 1 reply; 4+ messages in thread
From: Benjamin Herrenschmidt @ 2004-03-15  6:35 UTC (permalink / raw)
  To: Giridhar Pemmasani; +Cc: Linux Kernel list, swsusp-devel

On Mon, 2004-03-15 at 02:19, Giridhar Pemmasani wrote:
> Hello
> 
> I have been trying to get a reliable way to suspend-to-disk. For me
> swsusp was taking more than a few minutes and swsusp2 was not
> reliable. So I decided to give pmdisk a chance. Since my laptop
> supported S4bios, pmdisk wouldn't let me change it to S4 platform
> based suspend. Even after fixing it, I didn't have much luck. I dug
> deeper and found another problem. pmdisk resumes all devices while
> suspending. So I patched the pmdisk to resume only the device on which
> suspend image is to be stored. With these patches, I have been able to
> use pmdisk reliably (tested with two laptops over 100 times).
> 
> I am not subscribed to the mailing list, so if you have problems or
> feedback, CC to me.

That's broken. You cannot call resume_device for a single device
like that. You must properly (and in order) resume the parents of
this device first. Your patch, on some controllers, will cause the
IDE layer to try to wake up the drive without waking up the IDE
controller itself.

I agree that it would be nice to be able to resume only one "branch"
of the PM tree like that, it's just not possible at this point.

Ben



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: pmdisk suspend patch
  2004-03-15  6:35 ` Benjamin Herrenschmidt
@ 2004-03-15  8:13   ` Giridhar Pemmasani
  2004-03-15  9:36     ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 4+ messages in thread
From: Giridhar Pemmasani @ 2004-03-15  8:13 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: Linux Kernel list, swsusp-devel

On Mon, 15 Mar 2004 17:35:07 +1100, Benjamin Herrenschmidt <benh@kernel.crashing.org> said:

  Benjamin> I agree that it would be nice to be able to resume only
  Benjamin> one "branch" of the PM tree like that, it's just not
  Benjamin> possible at this point.

Thanks for your reply.

My understanding was that although it is a specific device that is
being resumed, actual resume method is for the bus and that should
power up the whole IDE subsystem. Am I wrong?

I see that there is dependency branch information in
dpm_active/dpm_off/dpm_off_irq. Isn't it possible to traverse these
lists and power up parents for a device? What are the issues involved?

Cheers,
Giri

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: pmdisk suspend patch
  2004-03-15  8:13   ` Giridhar Pemmasani
@ 2004-03-15  9:36     ` Benjamin Herrenschmidt
  0 siblings, 0 replies; 4+ messages in thread
From: Benjamin Herrenschmidt @ 2004-03-15  9:36 UTC (permalink / raw)
  To: Giridhar Pemmasani; +Cc: Linux Kernel list, swsusp-devel

On Mon, 2004-03-15 at 19:13, Giridhar Pemmasani wrote:
> On Mon, 15 Mar 2004 17:35:07 +1100, Benjamin Herrenschmidt <benh@kernel.crashing.org> said:
> 
>   Benjamin> I agree that it would be nice to be able to resume only
>   Benjamin> one "branch" of the PM tree like that, it's just not
>   Benjamin> possible at this point.
> 
> Thanks for your reply.
> 
> My understanding was that although it is a specific device that is
> being resumed, actual resume method is for the bus and that should
> power up the whole IDE subsystem. Am I wrong?

No. The bus "type" handler gets the resume and passes it along to the
device straight. You really need to walk the PM tree and wake up all
parents first, starting from the top of tree.

> I see that there is dependency branch information in
> dpm_active/dpm_off/dpm_off_irq. Isn't it possible to traverse these
> lists and power up parents for a device? What are the issues involved?
> 
> Cheers,
> Giri
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
-- 
Benjamin Herrenschmidt <benh@kernel.crashing.org>


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2004-03-15  9:41 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2004-03-14 15:19 pmdisk suspend patch Giridhar Pemmasani
2004-03-15  6:35 ` Benjamin Herrenschmidt
2004-03-15  8:13   ` Giridhar Pemmasani
2004-03-15  9:36     ` Benjamin Herrenschmidt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).