Re: Linus rollup

Andrew Morton
2003-01-29T10:16:36Z

Russell King <rmk@arm.linux.org.uk> wrote:
>
> On Tue, Jan 28, 2003 at 10:07:29PM -0800, Andrew Morton wrote:
> > Possible breakage would be in the new frlock-for-xtime_lock code and the
> > get_order() cleanup.
> >
> > The frlock code is showing nice speedups, but I think the main reason we want
> > this is to fix the problem wherein an application spinning on gettimeofday()
> > can make time stop.
>
> I'm slightly concerned about this. With this patch, we generally seem
> to do:
>
> do {
> seq = fr_read_begin(&xtime_lock);
> *tv = xtime;
> tv->tv_usec += do_fast_gettimeoffset();
> tv->tv_usec += lost_ticks;
> } while (seq != fr_read_end(&xtime_lock));
>
> This is fine when considering xtime. However, considering the
> implementation of do_fast_gettimeoffset(). Notice that
> linux/arch/i386/kernel/timers/timer_pit.c:do_offset_pit specifically
> makes the comment:
>
> /* This function must be called with interrupts disabled
>
> which hasn't been true for some time, and is even less true now that
> local IRQs don't get disabled. Does this matter... for UP?
>
> The same is true for other architectures; their gettimeoffset
> implementations need to be audited by the architecture maintainers
> to ensure that they are safe to run with (local) interrupts enabled.

Thanks for spotting that. Bug.

How about I make all instances of the above be:

do {
seq = fr_read_begin_irqsave(&xtime_lock, flags);
*tv = xtime;
tv->tv_usec += do_fast_gettimeoffset();
tv->tv_usec += lost_ticks;
} while (seq != fr_read_end_irqrestore(&xtime_lock, flags));

?