Messages in this thread | | | Date | Wed, 31 Jan 2007 18:15:32 +0100 | From | Ingo Molnar <> | Subject | Re: [PATCH 11/23] clocksource: atomic signals |
| |
* Daniel Walker <dwalker@mvista.com> wrote:
> > I see little difference between your and John's code: both poll > > something - you poll an atomic "did a new clocksource arrive" flag > > in the timer interrupt, John takes the clocksource_lock spinlock and > > checks a "did a new clocksource arrive" variable. Both are global > > atomic variables in essence. > > The original version has more operations on every timer interrupt. > Also changing the spinlock to an atomic eliminates the possibility of > contention in the timer interrupt ..
there is precisely /zero/ contention on the clocksource_lock! It is a very short-held lock, and it's only held by the timer interrupt and some really rare operations like 'clocksource register' or 'show clocksources'.
> > what i'd see as a real cleanup here would be to get away from this > > 'poll whether there's any clocksource update' model, and to just > > ensure that a running timer irq will always see the latest > > clocksource. I.e. to run the change_clocksource() logic (and the > > following updates) when a new clock source is selected - not when > > the next timer interrupt runs. That would propagate all effects of a > > new clock source immediately. > > You could reduce the code in the interrupt handler (which is good), > but I think you'll end up with a polling model regardless.. If you add > some locking between the interrupt handler and something else you may > as well add the run time of that new critical section to the timer > latency . So I'm not sure it would be a outright win ..
I think you didnt understand what i said: the point is to /remove/ the polling, and to replace it with a natural lock that is held anyway: xtime_lock or whatever other exclusion mechanism. Again, there is almost /never/ any contention on this lock so there's no 'latency to add'. But the polling overhead in every timer irq, even if it's just a single atomic flag, does add up in every timer tick.
you also didnt seem to understand my other point:
> > I.e. to run the change_clocksource() logic (and the following > > updates) when a new clock source is selected - not when the next > > timer interrupt runs. That would propagate all effects of a new > > clock source immediately.
that is actually more important from a design cleanliness POV than the basic avoidance of some polling overhead.
Ingo - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |