Date: Tue, 27 Jul 2010 20:13:30 +0530 From: "Jayachandran C." <c.jayachandran@gmail.com> To: Alexander Motin <mav@freebsd.org> Cc: Randall Stewart <rrs@freebsd.org>, Neel Natu <neel@freebsd.org>, freebsd-mips@freebsd.org Subject: Re: [RFC] Event timers on MIPS Message-ID: <AANLkTiktMt87V5jXV0%2BnagHjpfTkBQ8Fu6CK7HqNXff3@mail.gmail.com> In-Reply-To: <4C4ED247.80701@FreeBSD.org> References: <4C41A248.8090605@FreeBSD.org> <AANLkTilKYw4UqmfEee9zHGosEDzy4hiFob1d8R9jcB25@mail.gmail.com> <4C41B4CF.6080409@FreeBSD.org> <AANLkTik8_NGm7nKYXT1d1E4Vj6vYQPWHnnLDi78YnvQD@mail.gmail.com> <4C4205CC.6080700@FreeBSD.org> <AANLkTikUpqLeogkqxqWzzejp=7FstHX2wVRWNrYoWGCp@mail.gmail.com> <4C4ED247.80701@FreeBSD.org>
index | next in thread | previous in thread | raw e-mail
[-- Attachment #1 --]
On Tue, Jul 27, 2010 at 6:04 PM, Alexander Motin <mav@freebsd.org> wrote:
> Jayachandran C. wrote:
>> On Sun, Jul 18, 2010 at 1:04 AM, Alexander Motin <mav@freebsd.org> wrote:
>>> Jayachandran C. wrote:
>>>> On XLR we would like to use the count/compare which is faster but less
>>>> accurate on all cpus - we can have upto 32 cpus now. We also have a
>>>> PIC which can provide a better timestamp and timer interrupts. This
>>>> PIC timestamp can be read from all CPUs but the timer interrupt can be
>>>> delivered to just one CPU at a time. I think this is how we ended up
>>>> with the current implementation, but any suggestions on how to improve
>>>> this is welcome.
>>
>> As a first step, I have copied the count /compare code from mips with
>> minor modifications into mips/rmi, this lets me boot up (checked in as
>> r210528).
>>
>> I would like to add the PIC based clock next.
>
> Thanks.
>
>>> I would prefer to not mix the things.
>>>
>>> I think:
>>> - PIC timestamp looks like the best candidate for system timecounter.
>>> - per-CPU counters could be registered as per-CPU timecounters with
>>> set_cputicker() - the main criteria there is a speed.
>>> - if per-CPU counters are synchronized between CPUs - they could be
>>> registered as alternative timecounter for people who wish fastest
>>> timecounting; if they are not - they are useless in that role.
>>> - both PIC timer and per-CPU comparators should be independently
>>> registered as eventtimers - it is better to have two of them to from
>>> accounting correctness PoV, and it will allow user to experiment which
>>> one he likes more.
>>> - if there is any other timer hardware - it also should be registered -
>>> it will give additional flexibility.
>>
>> The per-cpu count/compare counters are not synchronized on XLR.
>
> Then tick_ticker() function looks broken. counter_lower_last and
> counter_upper should be tracked per-CPU. Otherwise you will have huge
> forward jumps due to false overflows.
>
>> So your suggestion would be to add a PIC based clock which calls
>> tc_init() and et_register(), and to leave the set_cputicker() to be
>> the count/compare?
>
> Yes. And I would leave count/compare also calling tc_init() and
> et_register() as it is now. It won't hurt.
>
>> Also, with just the count/compare, I get these print on early mutiuser bootup.
>> ---
>> calcru: runtime went backwards from 85936878 usec to 236488 usec for
>> pid 1286 (rpcbind)
>> calcru: runtime went backwards from 7158742 usec to 19700 usec for pid
>> 1285 (nfsiod 0)
>> calcru: runtime went backwards from 111005442 usec to 305474 usec for
>> pid 1257 (syslogd)
>> calcru: runtime went backwards from 10740196 usec to 29555 usec for
>> pid 1048 (devd)
>> --
>> Did not get much time to investigate, any idea what the cause can be?
>
> I think it can easily be result of broken tick_ticker().
I'm planning to check-in the attached patch for mips/rmi, I think
mips/mips would need something similar.
JC.
[-- Attachment #2 --]
Index: sys/mips/rmi/tick.c
===================================================================
--- sys/mips/rmi/tick.c (revision 210534)
+++ sys/mips/rmi/tick.c (working copy)
@@ -62,9 +62,8 @@
static DPCPU_DEFINE(uint32_t, cycles_per_tick);
static uint32_t cycles_per_usec;
-static u_int32_t counter_upper = 0;
-static u_int32_t counter_lower_last = 0;
-
+static DPCPU_DEFINE(uint32_t, counter_upper);
+static DPCPU_DEFINE(uint32_t, counter_lower_last);
static DPCPU_DEFINE(uint32_t, compare_ticks);
static DPCPU_DEFINE(uint32_t, lost_ticks);
@@ -106,6 +105,7 @@
{
uint64_t ret;
uint32_t ticktock;
+ uint32_t t_lower_last, t_upper;
/*
* XXX: MIPS64 platforms can read 64-bits of counter directly.
@@ -115,12 +115,16 @@
*/
ticktock = mips_rd_count();
critical_enter();
- if (ticktock < counter_lower_last)
- counter_upper++;
- counter_lower_last = ticktock;
+ t_lower_last = DPCPU_GET(counter_lower_last);
+ t_upper = DPCPU_GET(counter_upper);
+ if (ticktock < t_lower_last)
+ t_upper++;
+ t_lower_last = ticktock;
critical_exit();
- ret = ((uint64_t) counter_upper << 32) | counter_lower_last;
+ DPCPU_SET(counter_upper, t_upper);
+ DPCPU_SET(counter_lower_last, t_lower_last);
+ ret = ((uint64_t)t_upper << 32) | t_lower_last;
return (ret);
}
@@ -265,9 +269,9 @@
mips_wr_compare(0xffffffff);
critical_enter();
- if (count < counter_lower_last) {
- counter_upper++;
- counter_lower_last = count;
+ if (count < DPCPU_GET(counter_lower_last)) {
+ DPCPU_SET(counter_upper, DPCPU_GET(counter_upper) + 1);
+ DPCPU_SET(counter_lower_last, count);
}
if (cycles_per_tick > 0) {
help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTiktMt87V5jXV0%2BnagHjpfTkBQ8Fu6CK7HqNXff3>