On Wed, Mar 16, 2016 at 14:42 +0900, Masao Uebayashi wrote:
> This doesn't use atomic operation, because:
>
> - hardclock() is the only writer
> - Clock interrupt doesn't run simultaneously
> - Reading int should be atomic on all architectures
>
Well, in reality both the new location and timeout_hardclock_update
are only executed by the primary CPU so there's no concurrent
modification happening with or without your diff.
FWIW, I'm fine with moving it into the kern_clock.c. I assume that
this will make even more sense with some upcoming diffs.
> On Wed, Mar 16, 2016 at 02:13:13PM +0900, Masao Uebayashi wrote:
> > This clarifies that the single, global `ticks' is owned by kern_clock.c.
> > timeout(9) is only one of users of `ticks', even though its handler,
> > timeout_hardclock_update(), is called from hardclock() after update of
> > `ticks' every time.
> >
> > Theoretically timecounter(9)'s tick, tc_ticktock(), is independent of
> > the value of `ticks'. Then it must be safe to update `ticks' first then
> > tick timecounter(9). I leave this order as is for now.
> >
> > Index: sys/kern/kern_clock.c
> > ===================================================================
> > RCS file: /cvs/src/sys/kern/kern_clock.c,v
> > retrieving revision 1.88
> > diff -u -p -r1.88 kern_clock.c
> > --- sys/kern/kern_clock.c 11 Jun 2015 16:03:04 -0000 1.88
> > +++ sys/kern/kern_clock.c 16 Mar 2016 04:30:42 -0000
> > @@ -191,6 +191,7 @@ hardclock(struct clockframe *frame)
> > return;
> >
> > tc_ticktock();
> > + ticks++;
> >
> > /*
> > * Update real-time timeout queue.
> > Index: sys/kern/kern_timeout.c
> > ===================================================================
> > RCS file: /cvs/src/sys/kern/kern_timeout.c,v
> > retrieving revision 1.43
> > diff -u -p -r1.43 kern_timeout.c
> > --- sys/kern/kern_timeout.c 20 Jul 2015 23:47:20 -0000 1.43
> > +++ sys/kern/kern_timeout.c 16 Mar 2016 04:30:42 -0000
> > @@ -304,8 +304,6 @@ timeout_hardclock_update(void)
> >
> > mtx_enter(&timeout_mutex);
> >
> > - ticks++;
> > -
> > MOVEBUCKET(0, ticks);
> > if (MASKWHEEL(0, ticks) == 0) {
> > MOVEBUCKET(1, ticks);
>
