2007-02-07 13:35:20

by Milan Svoboda

[permalink] [raw]
Subject: posix-timers overrun broken?

Hi,

I'm creating a driver for timer on ixp4xx (there are two hardware timers).
It shall use posix api
so I looked at the only driver that implements it in kernel:
drivers/char/mmtimer.c. My driver uses almost the
same logic that handles overruns.

However, I found that receiving of number of overruns in waitsiginfo is
somewhat broken.

Linux-2.6.20:

Look at posix_timer_event - this is called when interrupt wants to signal
the thread that
event happend. The very first operation there is
memset(&timr->sigq->info, 0, sizeof(siginfo_t));
this clears all info members - si.overrun included.
Then send_sigqueue or send_group_sigqueue is called. They both check if
it's
SI_TIMER and if so, they increase si.overrun if the signal is already
queued.

But if the next interrupt arrives before function collect_signal is called
to actually deliver the
siginfo_t to userspace, the si.overrun is cleared in posix_timer_event and
we have just forgotten
one overrun...

Am I wrong?

Patch follows, but please note that it is more to describe the problem
than solve the problem in proper
way...

Please, CC me, I'm not subscribed on the list.

Signed-off-by: Milan Svoboda <[email protected]>

Index: linux/kernel/posix-timers.c
===================================================================
--- linux.orig/kernel/posix-timers.c
+++ linux/kernel/posix-timers.c
@@ -298,7 +298,10 @@ void do_schedule_next_timer(struct sigin

int posix_timer_event(struct k_itimer *timr,int si_private)
{
+ int overrun = timr->sigq->info.si_overrun;
memset(&timr->sigq->info, 0, sizeof(siginfo_t));
+ timr->sigq->info.si_overrun = overrun;
+
timr->sigq->info.si_sys_private = si_private;
/* Send signal to the process that owns this timer.*/

@@ -407,6 +410,7 @@ static struct k_itimer * alloc_posix_tim
kmem_cache_free(posix_timers_cache, tmr);
tmr = NULL;
}
+ tmr->sigq->info.si_overrun = 0;
return tmr;
}

Index: linux/kernel/signal.c
===================================================================
--- linux.orig/kernel/signal.c
+++ linux/kernel/signal.c
@@ -402,6 +402,7 @@ static int collect_signal(int sig, struc
if (first) {
list_del_init(&first->list);
copy_siginfo(info, &first->info);
+ first->info.si_overrun = 0;
__sigqueue_free(first);
if (!still_pending)
sigdelset(&list->signal, sig);


2007-02-07 16:14:06

by Thomas Gleixner

[permalink] [raw]
Subject: Re: posix-timers overrun broken?

On Wed, 2007-02-07 at 14:36 +0100, Milan Svoboda wrote:
> But if the next interrupt arrives before function collect_signal is called
> to actually deliver the
> siginfo_t to userspace, the si.overrun is cleared in posix_timer_event and
> we have just forgotten
> one overrun...
>
> Am I wrong?

Yes. posix_timer_event() is only called when the timer expires the first
time. When the signal has been queued, the timer is not rearmed and the
overrun is calculated in the actual signal delivery path, which calls
do_schedule_next_timer().

tglx


2007-02-08 07:54:07

by Milan Svoboda

[permalink] [raw]
Subject: Re: posix-timers overrun broken?

> On Wed, 2007-02-07 at 14:36 +0100, Milan Svoboda wrote:
> > But if the next interrupt arrives before function collect_signal is
called
> > to actually deliver the
> > siginfo_t to userspace, the si.overrun is cleared in posix_timer_event
and
> > we have just forgotten
> > one overrun...
> >
> > Am I wrong?
>
> Yes. posix_timer_event() is only called when the timer expires the first
> time. When the signal has been queued, the timer is not rearmed and the
> overrun is calculated in the actual signal delivery path, which calls
> do_schedule_next_timer().

Ok understand.

So it seems to me that drivers/char/mmtimer.c is wrong. It is
driver to additional hardware timers and mmtimer exports them as a posix
timer.
When interrupt happens, they call posix_timer_event and set hardware
interrupt
to the next tick. I use the same practise in my driver and found
that overruns are reported wrongly.

How to plug "external" source of ticks to the posix timers?

As far as I know current HRT doesn't allow us to use more sources of
ticks together, only one is selected, correct?

Milan