Skip to contentSkip to conversation

: jiffies wrap in ll_rw_blk.c

PATCH In review Luben Tuikov 4 participants 8 messages Nov 20, 2002

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 : jiffies wrap in ll_rw_blk.c

ll_rw_blk.c Wed Nov 20 15:33:06 2002 renamed from ll_rw_blk.c.old Wed Nov 20 15:32:50 2002 +1 -1
@@ -2092,7 +2092,7 @@
                complete(req->waiting);
 
        if (disk) {
-               unsigned long duration = jiffies - req->start_time;
+               unsigned long duration = (signed) jiffies - (signed) req->start_time;
                switch (rq_data_dir(req)) {
                    case WRITE:
                        disk->writes++;
#1 lore raw

ll_rw_blk.c Wed Nov 20 15:33:06 2002 line 2096

Quoted 9 lines · Luben Tuikov
ll_rw_blk.c Wed Nov 20 15:33:06 2002 renamed from ll_rw_blk.c.old Wed Nov 20 15:32:50 2002 +1 -1
@@ -2092,7 +2092,7 @@
                complete(req->waiting);
 
        if (disk) {
-               unsigned long duration = jiffies - req->start_time;
+               unsigned long duration = (signed) jiffies - (signed) req->start_time;
                switch (rq_data_dir(req)) {

It was right before. Your patch breaks it. Think about it in unsigned maths

              0x00000002 - 0xFFFFFFFF = 0x00000003

Alan

#2 lore raw
Quoted 15 lines · Alan Cox

ll_rw_blk.c Wed Nov 20 15:33:06 2002 line 2096

Quoted 9 lines · Luben Tuikov
ll_rw_blk.c Wed Nov 20 15:33:06 2002 renamed from ll_rw_blk.c.old Wed Nov 20 15:32:50 2002 +1 -1
@@ -2092,7 +2092,7 @@
                complete(req->waiting);
 
        if (disk) {
-               unsigned long duration = jiffies - req->start_time;
+               unsigned long duration = (signed) jiffies - (signed) req->start_time;
                switch (rq_data_dir(req)) {

It was right before. Your patch breaks it. Think about it in unsigned maths

Quoted 1 line
              0x00000002 - 0xFFFFFFFF = 0x00000003

0x2 - (-0x1) = 0x2 + 0x1 = 0x3

Right! I thought (signed) does the job. I actually tried it both ways and works all right. I guess either way works fine.

Signature · 1 line
Luben
#3 lore raw

On Wed, 2002-11-20 at 21:00, Luben Tuikov wrote:sk) {

Quoted 7 lines
Quoted 7 lines

ll_rw_blk.c Wed Nov 20 15:33:06 2002 line 2096

Quoted 3 lines
ll_rw_blk.c Wed Nov 20 15:33:06 2002 renamed from ll_rw_blk.c.old Wed Nov 20 15:32:50 2002 +1 -1
-               unsigned long duration = jiffies - req->start_time;
+               unsigned long duration = (signed) jiffies - (signed) req->start_time;
                switch (rq_data_dir(req)) {

It was right before. Your patch breaks it. Think about it in unsigned maths

Quoted 6 lines
Quoted 1 line
              0x00000002 - 0xFFFFFFFF = 0x00000003

0x2 - (-0x1) = 0x2 + 0x1 = 0x3

Right! I thought (signed) does the job. I actually tried it both ways and works all right. I guess either way works fine.

(signed long) maybe - but not signed - long is 64bit on Alpha, (signed) is 32

#4 lore raw
Quoted 11 lines · Alan Cox

On Wed, 2002-11-20 at 21:00, Luben Tuikov wrote:sk) {

Quoted 9 lines
Quoted 8 lines

ll_rw_blk.c Wed Nov 20 15:33:06 2002 line 2096

Quoted 3 lines
ll_rw_blk.c Wed Nov 20 15:33:06 2002 renamed from ll_rw_blk.c.old Wed Nov 20 15:32:50 2002 +1 -1
-               unsigned long duration = jiffies - req->start_time;
+               unsigned long duration = (signed) jiffies - (signed) req->start_time;
                switch (rq_data_dir(req)) {

It was right before. Your patch breaks it. Think about it in unsigned maths

              0x00000002 - 0xFFFFFFFF = 0x00000003
Quoted 7 lines
Quoted 4 lines

0x2 - (-0x1) = 0x2 + 0x1 = 0x3

Right! I thought (signed) does the job. I actually tried it both ways and works all right. I guess either way works fine.

(signed long) maybe - but not signed - long is 64bit on Alpha, (signed) is 32

Aaaah, I see where you're coming from.

I basically tried to stay away from knowing _what_ actual type it is and just to make it signed and do the arithmetic, but didn't know the specific for the Alpha.

Shouldn't this be (symbolically:) (signed (typeof(jiffes)) jiffies - (signed (typeof(start)) start -- you know what I mean.

But yes both ways should work:
a, b are both unsigned, then a-b = a+(-b) and there's just no other way to compute the result.

Anyway, doesn't matter,

Signature · 1 line
Luben
#5 lore raw
In reply to Alan Cox
Quoted 14 lines · Alan Cox

ll_rw_blk.c Wed Nov 20 15:33:06 2002 line 2096

Quoted 9 lines · Luben Tuikov
ll_rw_blk.c Wed Nov 20 15:33:06 2002 renamed from ll_rw_blk.c.old Wed Nov 20 15:32:50 2002 +1 -1
@@ -2092,7 +2092,7 @@
                complete(req->waiting);
 
        if (disk) {
-               unsigned long duration = jiffies - req->start_time;
+               unsigned long duration = (signed) jiffies - (signed) req->start_time;
                switch (rq_data_dir(req)) {

It was right before. Your patch breaks it. Think about it in unsigned maths

Quoted 1 line
              0x00000002 - 0xFFFFFFFF = 0x00000003

Signed vs. unsigned is actually irrelevant in two-complement systems as long as you don't compare. Only the int/long issue has an actual effect.

OG.

#6 lore raw
In reply to Luben Tuikov

Erm, you just truncated a long to an int.

-ben

Quoted 16 lines · Luben Tuikov
ll_rw_blk.c Wed Nov 20 15:33:06 2002 renamed from ll_rw_blk.c.old Wed Nov 20 15:32:50 2002 +1 -2
@@ -2092,7 +2092,7 @@
                complete(req->waiting);
 
        if (disk) {
-               unsigned long duration = jiffies - req->start_time;
+               unsigned long duration = (signed) jiffies - (signed) req->start_time;
                switch (rq_data_dir(req)) {
                    case WRITE:
                        disk->writes++;
-

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/

Signature · 1 line
"Do you seek knowledge in time travel?"
#7 lore raw
Quoted 2 lines · Benjamin LaHaise

Erm, you just truncated a long to an int.

Yep, totally forgot about no type == int, hehehhee.

Boy, last time I thought about that _rule_ was over 12 years ago...

Signature · 1 line
Luben