All of lore.kernel.org
 help / color / mirror / Atom feed
From: Grant Grundler <grundler@parisc-linux.org>
To: Carlos O'Donell <carlos@systemhalted.org>
Cc: parisc-linux <parisc-linux@lists.parisc-linux.org>
Subject: Re: [parisc-linux] [PATCH] timer_interrupt and gettimeoffset.
Date: Sun, 3 Sep 2006 13:55:53 -0600	[thread overview]
Message-ID: <20060903195553.GC20217@colo.lackof.org> (raw)
In-Reply-To: <119aab440609030754t5e0f869bq10b5c10770f14105@mail.gmail.com>

On Sun, Sep 03, 2006 at 10:54:18AM -0400, Carlos O'Donell wrote:
> >Yes. I'm now pretty sure the fix for that will be in gettimeoffset()
> >and not in timer_interrupt(). But I agree the two functions are very
> >similar but have to handle slightly different corner cases.
> 
> I still disagree, if you see James most recent patch which disabled
> IRQ's while we handle the timer interrupt, you will see that both
> functions are closer to being similar :)

I'm not sure about that. James might be wrong in this case.
See do_cpu_irq_mask().
It clears EIEM and thus disables all external interrupts
including the interval timer. The comment is wrong in that
we disable all EIEM bits, not just TIMER and IPI.

> I will assert that at the end of the day we will be writing
> "cr16_offset" which is a generic function that can be called with
> interrupts disabled and *that* will handle all the corner cases and be
> called by "timer_interrupt" and "gettimeoffset".

hrm ok. I'll keep that in mind. I'm not convinced of that
yet since those are both performance critical piecves of code.

...
> >Sorry - this didn't make sense. Can you provide an example?
> >For simplicity assume a 5 bit counter and pick the values
> >that illustrate your case.
> 
> 5 bit counter.
> it_value = 5
> cr16 = ... free running.
> clocktick = 5
> halftick = 2
> 
> a. Counter goes off at 8. Expected to go off at 5.

Itimer will interrupt at 5 if it was expect to go off at 5.
timer_interrupt() will never get invoked until 8 becuase of
interrupt handling overhead.

> b. Since (8 - 5) == 3 > 2 or a halftick, we push forward 2 ticks
> c. Set it_value to 15.

Ok. I really don't like the business with halftick pushing
system time "into the future".  Maybe we can just drop one tick
on the floor. Ie skip it and handle it on the next "full" tick?

> 1. User calls gettimeofday.
> 2. cr16 says time is 9
> 3. The it_value is 15, subtract a clocktick and you get 10.
> 
> *boom*
> 
> The value of cr16 is < the value of a rolled back clocktick.
> In counting the halftick, we need to rollback the halftick in gettimeoffset.

No. We need to just drop the next tick and not count it until
the next round.

> >Division is much more expensive than integer multiplication.
> >Is there a way to do the division once _and_ get the remainder?
> >
> >My assumption that integer multiplication is "cheaper"
> >could be wrong.
> 
> I timed this for some reasonable values of dividend and divisor.
> The division and modulo operation is ~156 cycles.
> The integer multiplication is ~73 cycles.
> This is the average cr16 tick over 1000 runs.
> The integer multiplication is "cheaper"

ok - many thanks for doing that.

...
> You say the following equations are correct:
> 
> ticks_elapsed = (now - next_tick) / clocktick;
> remainder = now - (ticks_elapsed * clocktick);
> 
> The ticks_elapsed equation *is* correct.
> 
> The remainder equation is wrong. The value of "now" could be 4GB, the
> value of "next_tick" could be "4GB - clocktick", the ticks_elapsed
> value is 1. This is correct. However, the remainder is going to be
> "4GB - 1 * clocktick" which is incorrect. The remainder in this case
> is zero.

> 
> To get the remainder you *must* do:
> 
> remainder = (now - next_tick) - (ticks_elapsed * clocktick)
> 
> This gives you the left over ticks you didn't count. I have added
> brackets here to clarify the math.

Yes - got it now. thanks again!

> I have come to the conclusion that adding the halftick is conceptually 
> wrong.
> 
> Follow me on this.
> 
> 1. By adding the halftick, as a real tick, the system time moves into
> the future.

I'm already convinced this is wrong and leads to too many complications.

> 2. A system with future time means we need negative gettimeoffset 
> adjustments.
> 3. Negative gettimeoffset adjustment complicate things needlessly.
> 4. Halfticks should not be counted, and should be ignored.
> 5. Ignoring halfticks is not a problem, when the late timer interrupt
> arrives we will accrue all full ticks anyway.
> 
> What do you think?

total agreement.
I'll rework my code later today to try to match the discussion so far.

thanks,
grant
_______________________________________________
parisc-linux mailing list
parisc-linux@lists.parisc-linux.org
http://lists.parisc-linux.org/mailman/listinfo/parisc-linux

  reply	other threads:[~2006-09-03 19:55 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-08-30  4:48 [parisc-linux] [PATCH] timer_interrupt and gettimeoffset Carlos O'Donell
2006-08-31  6:53 ` Grant Grundler
2006-08-31 18:52   ` Carlos O'Donell
2006-08-31 21:46     ` Grant Grundler
2006-09-03 14:54       ` Carlos O'Donell
2006-09-03 19:55         ` Grant Grundler [this message]
2006-09-03 20:29           ` James Bottomley
2006-09-03 20:30           ` Carlos O'Donell
2006-09-03 21:22             ` John David Anglin
2006-09-01 22:48   ` Grant Grundler
2006-09-01 22:59     ` Grant Grundler
2006-09-06  6:29     ` [parisc-linux] [PATCH] timer_interrupt #5 Grant Grundler
     [not found]       ` <44FF2A69.5040102@scarlet.be>
     [not found]         ` <20060907001944.GA4407@colo.lackof.org>
     [not found]           ` <450188D8.5000501@scarlet.be>
2006-09-08 18:49             ` Grant Grundler
2006-09-23  0:41               ` John David Anglin
  -- strict thread matches above, loose matches on Subject: below --
2006-08-30 16:38 Re:[parisc-linux] [PATCH] timer_interrupt and gettimeoffset Joel Soete
2006-08-30 16:52 ` [parisc-linux] " Grant Grundler
2006-08-30 20:23   ` Carlos O'Donell
2006-09-02 15:52     ` James Bottomley
2006-09-03 15:00       ` Carlos O'Donell
2006-09-03 15:17         ` James Bottomley
2006-09-03 16:14         ` James Bottomley
2006-09-03 19:31           ` Grant Grundler
2006-09-04 15:51             ` James Bottomley
2006-09-04 16:57               ` John David Anglin
2006-09-04 17:23                 ` James Bottomley
2006-09-04 19:12                   ` John David Anglin
2006-09-04 21:21                 ` Grant Grundler
2006-09-04 22:47                   ` James Bottomley
2006-09-04 23:52                     ` John David Anglin
2006-09-05  5:12                       ` Grant Grundler
2006-09-05 15:53                         ` James Bottomley
2006-09-05 22:49                           ` Grant Grundler
2006-09-05 22:59                             ` James Bottomley
2006-09-05 23:41                             ` John David Anglin
2006-09-06  0:24                         ` John David Anglin
2006-09-03 19:38       ` Grant Grundler
2006-09-03 19:59         ` John David Anglin
2006-09-04  0:09           ` Grant Grundler
2006-09-04  0:58             ` John David Anglin
2006-09-04  3:51             ` John David Anglin
2006-09-04 15:49         ` James Bottomley
     [not found] <44FDE867.3090204@scarlet.be>
2006-09-05 21:35 ` John David Anglin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20060903195553.GC20217@colo.lackof.org \
    --to=grundler@parisc-linux.org \
    --cc=carlos@systemhalted.org \
    --cc=parisc-linux@lists.parisc-linux.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.