public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* Jiffies wraparound is not treated in the schedstats
@ 2006-11-08 18:05 Mauricio Lin
  2006-11-09  6:29 ` Balbir Singh
  0 siblings, 1 reply; 6+ messages in thread
From: Mauricio Lin @ 2006-11-08 18:05 UTC (permalink / raw)
  To: balbir; +Cc: linux-kernel

Hi Balbir,

Do you know why in the sched_info_arrive() and sched_info_depart()
functions the calculation of delta_jiffies does not use the time_after
or time_before macro to prevent  the miscalculation when jiffies
overflow?

For instance the delta_jiffies variable is simply calculated as:

delta_jiffies = now - t->sched_info.last_queued;

Do not you think the more logical way should be

if (time_after(now, t->sched_info.last_queued))
   delta_jiffies = now - t->sched_info.last_queued;
else
   delta_jiffies = (MAX_JIFFIES - t->sched_info.last_queued) + now

I have included more variables to measure some issues of schedule in
the kernel (following schedstat idea) and I noticed that jiffies
wraparound has led to wrong values, since the user space tool when
collecting the values is producing negative values.

Any comments?

Can I provide a patch for that?

BR,

Mauricio Lin.

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: Jiffies wraparound is not treated in the schedstats
@ 2006-11-08 19:01 Kaz Kylheku
  0 siblings, 0 replies; 6+ messages in thread
From: Kaz Kylheku @ 2006-11-08 19:01 UTC (permalink / raw)
  To: linux-kernel

Mauricio Lin wrote:
> For instance the delta_jiffies variable is simply calculated as:
> 
> delta_jiffies = now - t->sched_info.last_queued;
>
> Do not you think the more logical way should be

No. Learn about the modulo arithmetic properties of the unsigned types.
 
> if (time_after(now, t->sched_info.last_queued))

Have you looked at time_after? It just performs a subtraction, after
casting both operands to long. It does not care which one is bigger than
the other. Basically, it performs the same calculation as the
delta_jiffies above, and then checks the sign bit.

>    delta_jiffies = now - t->sched_info.last_queued;
> else
>    delta_jiffies = (MAX_JIFFIES - t->sched_info.last_queued) + now

I'm looking at a 2.6.17 kernel, and I can't find MAX_JIFFIES, or
JIFFIES_MAX. There is a MAX_JIFFY_OFFSET, which is something else. It's
the biggest delta that you can add to a value X so that time_after(X, X
+ delta) is still true. If you try to schedule something beyond now +
MAX_JIFFY_OFFSET, it will be put back in time.

Also, your calculation is buggy. MAX_JIFFIES would correspond to
ULONG_MAX. What you want is:

  (ULONG_MAX + 1 - last_queued + now)

This expression is now mathematically congruent to 

  -last_queued + now (mod ULONG_MAX + 1)

or

   now - last_queued (mod ULONG_MAX + 1).

The arithmetic of the unsigned long type is /already/ carried out modulo
ULONG_MAX + 1, so the ULONG_MAX + 1 term can be dropped from your
expression, leaving it as

  now - last_queued

> I have included more variables to measure some issues of schedule in
> the kernel (following schedstat idea) and I noticed that jiffies
> wraparound has led to wrong values, since the user space tool when
> collecting the values is producing negative values.

What user space tool?

> Any comments?

The user space tool is perhaps buggy, or you are misinterpreting its
output.
 
> Can I provide a patch for that?

Some day, when you learn how to program.

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

end of thread, other threads:[~2007-05-30  4:10 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-11-08 18:05 Jiffies wraparound is not treated in the schedstats Mauricio Lin
2006-11-09  6:29 ` Balbir Singh
2006-11-09  6:45   ` Balbir Singh
2006-11-09 14:35   ` Mauricio Lin
2007-05-30  4:03   ` Oleg Verych
  -- strict thread matches above, loose matches on Subject: below --
2006-11-08 19:01 Kaz Kylheku

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox