Skip to contentSkip to conversation

kernel/power/hibernate.c: be sure of 'start' is not bigger than 'stop'

PATCH In review Chen Gang 3 participants 4 messages Apr 21, 2014 – Apr 22, 2014

Subsystems (2)
Suspend to RAM Hibernation
lore
Share & tools
lore

Chen Gang

author
#0 lore raw

PATCH 1/1 kernel/power/hibernate.c: be sure of 'start' is not bigger than 'stop'

For do_div(), it need 'u64' type, which means the outside must be sure of 'start' is not bigger than 'stop', or it will report warning.

The related warning (with allmodconfig for unicore32):

CC kernel/power/hibernate.o
kernel/power/hibernate.c: In function ‘swsusp_show_speed’:
kernel/power/hibernate.c:237: warning: comparison of distinct pointer types lacks a cast

Signed-off-by Chen Gang <[email protected]>
---
 kernel/power/hibernate.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
kernel/power/hibernate.c +6 -1
index f4f2073..0dec9b7 100644
@@ -228,12 +228,17 @@ static void platform_recover(int platform_mode)
 void swsusp_show_speed(struct timeval *start, struct timeval *stop,
 			unsigned nr_pages, char *msg)
 {
-	s64 elapsed_centisecs64;
+	u64 elapsed_centisecs64;
 	int centisecs;
 	int k;
 	int kps;
 
 	elapsed_centisecs64 = timeval_to_ns(stop) - timeval_to_ns(start);
+	if ((s64)elapsed_centisecs64 < 0) {
+		printk(KERN_ERR "PM: stop: %llu < start %llu\n",
+			timeval_to_ns(stop), timeval_to_ns(start));
+		return;
+	}
 	do_div(elapsed_centisecs64, NSEC_PER_SEC / 100);
 	centisecs = elapsed_centisecs64;
 	if (centisecs == 0)
Signature · 1 line
1.9.2.459.g68773ac
#1 lore raw
Quoted 17 lines · Chen Gang

For do_div(), it need 'u64' type, which means the outside must be sure of 'start' is not bigger than 'stop', or it will report warning.

The related warning (with allmodconfig for unicore32):

CC kernel/power/hibernate.o
kernel/power/hibernate.c: In function ‘swsusp_show_speed’:
kernel/power/hibernate.c:237: warning: comparison of distinct pointer types lacks a cast

Signed-off-by Chen Gang <[email protected]>

{

kernel/power/hibernate.c +1 -1
-	s64 elapsed_centisecs64;
+	u64 elapsed_centisecs64;
 	int centisecs;
 	int k;
 	int kps;

kernel/power/hibernate.c static void platform_recover(int platform_mode) · line 244

Quoted 10 lines
kernel/power/hibernate.c +5 -0
 
 	elapsed_centisecs64 = timeval_to_ns(stop) - timeval_to_ns(start);
+	if ((s64)elapsed_centisecs64 < 0) {
+		printk(KERN_ERR "PM: stop: %llu < start %llu\n",
+			timeval_to_ns(stop), timeval_to_ns(start));
+		return;
+	}
 	do_div(elapsed_centisecs64, NSEC_PER_SEC / 100);
 	centisecs = elapsed_centisecs64;
 	if (centisecs == 0)

Quite a lot of code to shut up warning in mostly debugging function... is there alternative method of shutting it up? Have you seen < 0 values in wild? Is it worth handing that situation?
Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#2 lore raw
Quoted 19 lines · Pavel Machek
Quoted 18 lines · Chen Gang

For do_div(), it need 'u64' type, which means the outside must be sure of 'start' is not bigger than 'stop', or it will report warning.

The related warning (with allmodconfig for unicore32):

CC kernel/power/hibernate.o
kernel/power/hibernate.c: In function ‘swsusp_show_speed’:
kernel/power/hibernate.c:237: warning: comparison of distinct pointer types lacks a cast

Signed-off-by Chen Gang <[email protected]>

{

kernel/power/hibernate.c +1 -1
-	s64 elapsed_centisecs64;
+	u64 elapsed_centisecs64;
 	int centisecs;
 	int k;
 	int kps;
Quoted 13 lines

kernel/power/hibernate.c static void platform_recover(int platform_mode) · line 244

Quoted 9 lines
kernel/power/hibernate.c +5 -0
 	elapsed_centisecs64 = timeval_to_ns(stop) - timeval_to_ns(start);
+	if ((s64)elapsed_centisecs64 < 0) {
+		printk(KERN_ERR "PM: stop: %llu < start %llu\n",
+			timeval_to_ns(stop), timeval_to_ns(start));
+		return;
+	}
 	do_div(elapsed_centisecs64, NSEC_PER_SEC / 100);
 	centisecs = elapsed_centisecs64;
 	if (centisecs == 0)

Quite a lot of code to shut up warning in mostly debugging function... is there alternative method of shutting it up? Have you seen < 0 values in wild?

No, I don't think so. This is a purely theoretical problem that never happens in practice, because all of the callers do the right thing.

Rafael

author
#3 lore raw
Quoted 34 lines · Rafael J. Wysocki
Quoted 19 lines · Pavel Machek
Quoted 18 lines · Chen Gang

For do_div(), it need 'u64' type, which means the outside must be sure of 'start' is not bigger than 'stop', or it will report warning.

The related warning (with allmodconfig for unicore32):

CC kernel/power/hibernate.o
kernel/power/hibernate.c: In function ‘swsusp_show_speed’:
kernel/power/hibernate.c:237: warning: comparison of distinct pointer types lacks a cast

Signed-off-by Chen Gang <[email protected]>

{

kernel/power/hibernate.c +1 -1
-	s64 elapsed_centisecs64;
+	u64 elapsed_centisecs64;
 	int centisecs;
 	int k;
 	int kps;
Quoted 13 lines

kernel/power/hibernate.c static void platform_recover(int platform_mode) · line 244

Quoted 9 lines
kernel/power/hibernate.c +5 -0
 	elapsed_centisecs64 = timeval_to_ns(stop) - timeval_to_ns(start);
+	if ((s64)elapsed_centisecs64 < 0) {
+		printk(KERN_ERR "PM: stop: %llu < start %llu\n",
+			timeval_to_ns(stop), timeval_to_ns(start));
+		return;
+	}
 	do_div(elapsed_centisecs64, NSEC_PER_SEC / 100);
 	centisecs = elapsed_centisecs64;
 	if (centisecs == 0)

Quite a lot of code to shut up warning in mostly debugging function... is there alternative method of shutting it up? Have you seen < 0 values in wild?

Quoted 5 lines

No, I don't think so. This is a purely theoretical problem that never happens in practice, because all of the callers do the right thing.

Rafael

OK, it sounds fine to me, I shall send patch v2 for it.

Thanks.

Signature · 3 lines
Chen Gang

Open, share, and attitude like air, water, and life which God blessed