PM / Hibernate : Add wait for disk detection if resume_file is MAJOR:MINOR style

RFC/PATCH Minho Ban 3 participants 7 messages May 7, 2012 – May 9, 2012

Subsystems (2)

Suspend to RAM Hibernation

lore

Share & tools

lore

Minho Ban

author

2012-05-07 08:26 UTC #0 lore raw

name_to_dev_t is not enough to check device detection when the resume_file is MAJOR:MINOR format. get_gendisk is needed in that case.

Signed-off-by Minho Ban <[email protected]>

---
 kernel/power/hibernate.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

kernel/power/hibernate.c +10 -0

index e09dfbf..27efaef 100644
@@ -25,6 +25,8 @@
 #include <linux/freezer.h>
 #include <linux/gfp.h>
 #include <linux/syscore_ops.h>
+#include <linux/ctype.h>
+#include <linux/genhd.h>
 #include <scsi/scsi_scan.h>
 
 #include "power.h"

@@ -749,6 +751,14 @@ static int software_resume(void)
 		}
 	}
 
+	/* name_to_dev_t is ineffective if resume_file comes in major:minor
+	 * format */
+	if (isdigit(resume_file[0]) && resume_wait) {
+		int partno;
+		while (!get_gendisk(swsusp_resume_device, &partno))
+			msleep(10);
+	}
+
  Check_image:
 	pr_debug("PM: Hibernation image partition %d:%d present\n",
 		MAJOR(swsusp_resume_device), MINOR(swsusp_resume_device));

Signature · 1 line

1.7.5.4

Rafael J. Wysocki

2012-05-07 19:34 UTC #1 lore raw

Quoted 2 lines · Minho Ban

name_to_dev_t is not enough to check device detection when the resume_file is MAJOR:MINOR format.

It's not assumed to be in that format, although I suppose we can add the possibility to specify it this way.

Quoted 21 lines

get_gendisk is needed in that case.

Signed-off-by Minho Ban <[email protected]>

---
 kernel/power/hibernate.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

kernel/power/hibernate.c +2 -0

index e09dfbf..27efaef 100644
@@ -25,6 +25,8 @@
 #include <linux/freezer.h>
 #include <linux/gfp.h>
 #include <linux/syscore_ops.h>
+#include <linux/ctype.h>
+#include <linux/genhd.h>
 #include <scsi/scsi_scan.h>
 
 #include "power.h"

kernel/power/hibernate.c static int software_resume(void) · line 756

Quoted 6 lines

kernel/power/hibernate.c +3 -0

@@ -749,6 +751,14 @@ static int software_resume(void)
 		}
 	}
 
+	/* name_to_dev_t is ineffective if resume_file comes in major:minor
+	 * format */
+	if (isdigit(resume_file[0]) && resume_wait) {

The check here is too late. It should be done before name_to_dev_t() is used on resume_file for the first time.

kernel/power/hibernate.c static int software_resume(void) · line 764

Quoted 8 lines

kernel/power/hibernate.c +5 -0

+		int partno;
+		while (!get_gendisk(swsusp_resume_device, &partno))
+			msleep(10);
+	}
+
  Check_image:
 	pr_debug("PM: Hibernation image partition %d:%d present\n",
 		MAJOR(swsusp_resume_device), MINOR(swsusp_resume_device));

Besides, I'd like the documentation to be updated to reflect this change too.

Thanks,
Rafael

Minho Ban

author

2012-05-07 23:56 UTC #2 lore raw

Quoted 7 lines · Rafael J. Wysocki

kernel/power/hibernate.c static int software_resume(void) · line 756
Quoted 3 lines
kernel/power/hibernate.c +3 -0
+	/* name_to_dev_t is ineffective if resume_file comes in major:minor
+	 * format */
+	if (isdigit(resume_file[0]) && resume_wait) {
The check here is too late. It should be done before name_to_dev_t() is used on resume_file for the first time.

Thanks for comment, I'll pull it up.

Quoted 10 lines

kernel/power/hibernate.c static int software_resume(void) · line 764
Quoted 8 lines
kernel/power/hibernate.c +5 -0
+		int partno;
+		while (!get_gendisk(swsusp_resume_device, &partno))
+			msleep(10);
+	}
+
  Check_image:
 	pr_debug("PM: Hibernation image partition %d:%d present\n",
 		MAJOR(swsusp_resume_device), MINOR(swsusp_resume_device));
Besides, I'd like the documentation to be updated to reflect this change too.

Do you mean resumewait is needed to be updated in kernel-parameters.txt?

Rafael J. Wysocki

2012-05-08 21:24 UTC #3 lore raw

Quoted 22 lines · Minho Ban

Quoted 7 lines · Rafael J. Wysocki
kernel/power/hibernate.c static int software_resume(void) · line 756
Quoted 3 lines
kernel/power/hibernate.c +3 -0
+	/* name_to_dev_t is ineffective if resume_file comes in major:minor
+	 * format */
+	if (isdigit(resume_file[0]) && resume_wait) {
The check here is too late. It should be done before name_to_dev_t() is used on resume_file for the first time.
Thanks for comment, I'll pull it up.
Quoted 10 lines
kernel/power/hibernate.c static int software_resume(void) · line 764
Quoted 8 lines
kernel/power/hibernate.c +5 -0
+		int partno;
+		while (!get_gendisk(swsusp_resume_device, &partno))
+			msleep(10);
+	}
+
  Check_image:
 	pr_debug("PM: Hibernation image partition %d:%d present\n",
 		MAJOR(swsusp_resume_device), MINOR(swsusp_resume_device));
Besides, I'd like the documentation to be updated to reflect this change too.

Quoted 1 line

Do you mean resumewait is needed to be updated in kernel-parameters.txt?

No, but it should be decumented what exactly is to be passed in the resume= command line option (in kernel-parameters.txt). Please do that when you're at it.

Thanks,
Rafael

Srivatsa S. Bhat

2012-05-09 06:13 UTC #5 lore raw

In reply to Minho Ban

Quoted 22 lines · Minho Ban

name_to_dev_t is not enough to check device detection when the resume_file is MAJOR:MINOR format. get_gendisk is needed in that case.
Signed-off-by Minho Ban <[email protected]>
---
 kernel/power/hibernate.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)
kernel/power/hibernate.c +2 -0
index e09dfbf..27efaef 100644
@@ -25,6 +25,8 @@
 #include <linux/freezer.h>
 #include <linux/gfp.h>
 #include <linux/syscore_ops.h>
+#include <linux/ctype.h>
+#include <linux/genhd.h>
 #include <scsi/scsi_scan.h>
 
 #include "power.h"

kernel/power/hibernate.c static int software_resume(void) · line 755

Quoted 5 lines

kernel/power/hibernate.c +2 -0

@@ -749,6 +751,14 @@ static int software_resume(void)
 		}
 	}
 
+	/* name_to_dev_t is ineffective if resume_file comes in major:minor
+	 * format */

Nitpick: Multi-line comments should be of the form:

/*
* name_to_dev_t is ineffective....
* ....... major:minor format
*/

Regards,
Srivatsa S. Bhat

Quoted 9 lines

kernel/power/hibernate.c +6 -0

+	if (isdigit(resume_file[0]) && resume_wait) {
+		int partno;
+		while (!get_gendisk(swsusp_resume_device, &partno))
+			msleep(10);
+	}
+
  Check_image:
 	pr_debug("PM: Hibernation image partition %d:%d present\n",
 		MAJOR(swsusp_resume_device), MINOR(swsusp_resume_device));

Minho Ban

author

2012-05-09 08:03 UTC #4 lore raw

In reply to Rafael J. Wysocki

Quoted 13 lines · Rafael J. Wysocki

Quoted 12 lines · Minho Ban
Quoted 10 lines · Rafael J. Wysocki
kernel/power/hibernate.c static int software_resume(void) · line 764
Quoted 8 lines
kernel/power/hibernate.c +5 -0
+		int partno;
+		while (!get_gendisk(swsusp_resume_device, &partno))
+			msleep(10);
+	}
+
  Check_image:
 	pr_debug("PM: Hibernation image partition %d:%d present\n",
 		MAJOR(swsusp_resume_device), MINOR(swsusp_resume_device));
Besides, I'd like the documentation to be updated to reflect this change too.

Quoted 9 lines

Quoted 1 line
Do you mean resumewait is needed to be updated in kernel-parameters.txt?
No, but it should be decumented what exactly is to be passed in the resume= command line option (in kernel-parameters.txt). Please do that when you're at it.
Thanks,
Rafael

OK, I'll add some comment on the resume= to clarify what partition format could be used.

Regards,
Minho Ban

Minho Ban

author

2012-05-09 08:05 UTC #6 lore raw

In reply to Srivatsa S. Bhat

Quoted 7 lines · Srivatsa S. Bhat

kernel/power/hibernate.c static int software_resume(void) · line 755
Quoted 2 lines · Minho Ban
kernel/power/hibernate.c +2 -0
+	/* name_to_dev_t is ineffective if resume_file comes in major:minor
+	 * format */
Nitpick: Multi-line comments should be of the form:

Quoted 5 lines

/*
* name_to_dev_t is ineffective....
* ....... major:minor format
*/

Thank you for pointing this out. I'll fix it.

Regards,
Minho Ban