Re: Wrong use of MAX_JIFFY_OFFSET?

From: Andrew Morton
Date: Fri Apr 06 2012 - 15:05:27 EST


On Wed, 28 Mar 2012 15:12:24 +0800
"Liu Yu" <liuyums@xxxxxxxxxxxxx> wrote:

> Hi guys,
>
> I saw a couple of places in current kernel have this kind of code:
>
> > static inline unsigned int elapsed_jiffies_msecs(unsigned long start)
> > {
> > unsigned long end = jiffies;
> >
> > if (end >= start)
> > return jiffies_to_msecs(end - start);
> >
> > return jiffies_to_msecs(end + (MAX_JIFFY_OFFSET - start) + 1);
> > }
>
> As you know, jiffies has a type of unsigned long, so if we know which is the
> end and
> which is the start, then (end - start) can simply figure out how much
> jiffies flies,
> without worry about the overflow.
>
> Look at the code above, assume that there is just an overflow happening on
> jiffies: end=0 and start=~0UL.
> Since end < start, then the return value of the function is
> jiffies_to_msecs(MAX_JIFFY_OFFSET+2).
> But shouldn't the correct value be jiffies_to_msecs(1)?
>
> could someone tell me that am I missing anything?
>

Seems right. The code should be

static inline unsigned long elapsed_jiffies_msecs(unsigned long start)
{
return jiffies_to_msecs(jiffies - start);
}

Note the return type. jiffies_to_msecs() currently returns unsigned
int. I think it should return unsigned long. Even then, it can still
overflow with valid inputs on HZ=100 32-bit machines.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/