2009-07-29 13:42:47

by Martin Schwidefsky

[permalink] [raw]
Subject: [RFC][patch 00/12] clocksource / timekeeping rework V2

Greetings,
version 2 of the clocksource / timekeeping cleanup patches. The series
has grown quite a bit, what started with a simple idea to replace the
tick based clocksource update with stop_machine is now a full fledged
code rework.

The code is working on s390 and on my Athlon system at home which has
a broken tsc clocksource:

[ 0.000000] Fast TSC calibration using PIT
[ 0.000341] hpet clockevent registered
[ 0.000343] HPET: 3 timers in total, 0 timers will be used for per-cpu timer
[ 0.204021] hpet0: at MMIO 0xfefff000, IRQs 2, 8, 31
[ 0.204027] hpet0: 3 comparators, 32-bit 25.000000 MHz counter
[ 0.208007] Switching to clock hpet
[ 0.211544] Switched to high resolution mode on CPU 0
[ 0.211960] Switched to high resolution mode on CPU 1
[ 8.000020] Clocksource tsc unstable (delta = -172310085 ns)

So the clocksource switch via stop_machine and the clocksource watchdog
are working. I keep the fingers crossed that nothing else breaks.

The patch set is based on todays upstream tree plus the patches from
the tip tree, if anyone wants to try them you need to pull from the
master branch of
git://git.kernel.org/pub/scm/linux/kernel/git/x86/linux-2.6-tip

There is still more room for improvement. Some sore points are:

1) The cycle_last value still is in the struct clocksource. It should
be in the struct timekeeper but the check against cycles_last in the
read function of the TSC clock source makes it hard.
2) read_persistent_clock returns seconds. With a really good initial
time source this is not very precise. read_persistent_clock should
return a struct timespec.
3) xtime, raw_time, total_sleep_time, timekeeping_suspended, jiffies,
the ntp state and probably a few other values may be better located
in the struct timekeeper as well.

and a few more I forgot.

Many thanks to John who pushed me into the right directions.
--
blue skies,
Martin.

"Reality continues to ruin my life." - Calvin.


2009-07-29 15:10:26

by Daniel Walker

[permalink] [raw]
Subject: Re: [RFC][patch 00/12] clocksource / timekeeping rework V2

On Wed, 2009-07-29 at 15:41 +0200, Martin Schwidefsky wrote:
> There is still more room for improvement. Some sore points are:
>
> 1) The cycle_last value still is in the struct clocksource. It should
> be in the struct timekeeper but the check against cycles_last in
> the
> read function of the TSC clock source makes it hard.
> 2) read_persistent_clock returns seconds. With a really good initial
> time source this is not very precise. read_persistent_clock should
> return a struct timespec.
> 3) xtime, raw_time, total_sleep_time, timekeeping_suspended, jiffies,
> the ntp state and probably a few other values may be better located
> in the struct timekeeper as well.


You could also consolidate the clocksource_unregister() path and the
clocksource_change_rating(0) path , both are basically doing the same
thing.. Neither one is heavily used..

Daniel

2009-07-29 16:51:20

by Martin Schwidefsky

[permalink] [raw]
Subject: Re: [RFC][patch 00/12] clocksource / timekeeping rework V2

On Wed, 29 Jul 2009 09:10:31 -0600
[email protected] wrote:

> On Wed, 2009-07-29 at 15:41 +0200, Martin Schwidefsky wrote:
> > There is still more room for improvement. Some sore points are:
> >
> > 1) The cycle_last value still is in the struct clocksource. It should
> > be in the struct timekeeper but the check against cycles_last in
> > the
> > read function of the TSC clock source makes it hard.
> > 2) read_persistent_clock returns seconds. With a really good initial
> > time source this is not very precise. read_persistent_clock should
> > return a struct timespec.
> > 3) xtime, raw_time, total_sleep_time, timekeeping_suspended, jiffies,
> > the ntp state and probably a few other values may be better located
> > in the struct timekeeper as well.
>
>
> You could also consolidate the clocksource_unregister() path and the
> clocksource_change_rating(0) path , both are basically doing the same
> thing.. Neither one is heavily used..

I'm not quite sure I got this. If I look at the code:

/**
* clocksource_change_rating - Change the rating of a registered clocksource
*/
void clocksource_change_rating(struct clocksource *cs, int rating)
{
mutex_lock(&clocksource_mutex);
cs->rating = rating;
clocksource_select();
mutex_unlock(&clocksource_mutex);
}
EXPORT_SYMBOL(clocksource_change_rating);

/**
* clocksource_unregister - remove a registered clocksource
*/
void clocksource_unregister(struct clocksource *cs)
{
mutex_lock(&clocksource_mutex);
clocksource_dequeue_watchdog(cs);
list_del(&cs->list);
clocksource_select();
mutex_unlock(&clocksource_mutex);
}
EXPORT_SYMBOL(clocksource_unregister);

the two functions do different things. What exactly is the idea you've
got in mind?

--
blue skies,
Martin.

"Reality continues to ruin my life." - Calvin.

2009-07-29 17:02:18

by Daniel Walker

[permalink] [raw]
Subject: Re: [RFC][patch 00/12] clocksource / timekeeping rework V2

On Wed, 2009-07-29 at 18:50 +0200, Martin Schwidefsky wrote:

> void clocksource_change_rating(struct clocksource *cs, int rating)

> the two functions do different things. What exactly is the idea you've
> got in mind?

It's only the case when the rating goes to zero .. That makes the
clocksource unusable, which is very much like unregistering it..

Daniel

2009-07-29 17:09:50

by Martin Schwidefsky

[permalink] [raw]
Subject: Re: [RFC][patch 00/12] clocksource / timekeeping rework V2

On Wed, 29 Jul 2009 11:02:21 -0600
[email protected] wrote:

> On Wed, 2009-07-29 at 18:50 +0200, Martin Schwidefsky wrote:
>
> > void clocksource_change_rating(struct clocksource *cs, int rating)
>
> > the two functions do different things. What exactly is the idea you've
> > got in mind?
>
> It's only the case when the rating goes to zero .. That makes the
> clocksource unusable, which is very much like unregistering it..

True, the clocksource code won't pick the clock any more as long as
there is an alternative clock available. It still shows up in the list
of clocks though and you can do an override with it.

--
blue skies,
Martin.

"Reality continues to ruin my life." - Calvin.

2009-07-29 17:17:14

by Daniel Walker

[permalink] [raw]
Subject: Re: [RFC][patch 00/12] clocksource / timekeeping rework V2

On Wed, 2009-07-29 at 19:09 +0200, Martin Schwidefsky wrote:
> On Wed, 29 Jul 2009 11:02:21 -0600
> [email protected] wrote:
>
> > On Wed, 2009-07-29 at 18:50 +0200, Martin Schwidefsky wrote:
> >
> > > void clocksource_change_rating(struct clocksource *cs, int rating)
> >
> > > the two functions do different things. What exactly is the idea you've
> > > got in mind?
> >
> > It's only the case when the rating goes to zero .. That makes the
> > clocksource unusable, which is very much like unregistering it..
>
> True, the clocksource code won't pick the clock any more as long as
> there is an alternative clock available. It still shows up in the list
> of clocks though and you can do an override with it.

I'm not sure allowing that type of override a good idea tho .. I don't
think it's considered a usable clock when the rating goes to zero.

Daniel

2009-07-29 17:34:38

by Daniel Walker

[permalink] [raw]
Subject: Re: [RFC][patch 00/12] clocksource / timekeeping rework V2

On Wed, 2009-07-29 at 19:09 +0200, Martin Schwidefsky wrote:
> On Wed, 29 Jul 2009 11:02:21 -0600
> [email protected] wrote:
>
> > On Wed, 2009-07-29 at 18:50 +0200, Martin Schwidefsky wrote:
> >
> > > void clocksource_change_rating(struct clocksource *cs, int rating)
> >
> > > the two functions do different things. What exactly is the idea you've
> > > got in mind?
> >
> > It's only the case when the rating goes to zero .. That makes the
> > clocksource unusable, which is very much like unregistering it..
>
> True, the clocksource code won't pick the clock any more as long as
> there is an alternative clock available. It still shows up in the list
> of clocks though and you can do an override with it.

Now that you mention it, if the clocks are sorted by rating (something
your removing) then the sysfs override available listing isn't in rated
order .. So if you do have zero rated clocks you wouldn't have any idea
from looking at the available list.. It could be solved just by printing
them in rated order in the sysfs code tho.

2009-07-30 07:43:20

by Martin Schwidefsky

[permalink] [raw]
Subject: Re: [RFC][patch 00/12] clocksource / timekeeping rework V2

On Wed, 29 Jul 2009 10:34:37 -0700
Daniel Walker <[email protected]> wrote:

> On Wed, 2009-07-29 at 19:09 +0200, Martin Schwidefsky wrote:
> > On Wed, 29 Jul 2009 11:02:21 -0600
> > [email protected] wrote:
> >
> > > On Wed, 2009-07-29 at 18:50 +0200, Martin Schwidefsky wrote:
> > >
> > > > void clocksource_change_rating(struct clocksource *cs, int rating)
> > >
> > > > the two functions do different things. What exactly is the idea you've
> > > > got in mind?
> > >
> > > It's only the case when the rating goes to zero .. That makes the
> > > clocksource unusable, which is very much like unregistering it..
> >
> > True, the clocksource code won't pick the clock any more as long as
> > there is an alternative clock available. It still shows up in the list
> > of clocks though and you can do an override with it.
>
> Now that you mention it, if the clocks are sorted by rating (something
> your removing) then the sysfs override available listing isn't in rated
> order .. So if you do have zero rated clocks you wouldn't have any idea
> from looking at the available list.. It could be solved just by printing
> them in rated order in the sysfs code tho.

Ok, I'll readd the sorting. It is a user visible change after all.

--
blue skies,
Martin.

"Reality continues to ruin my life." - Calvin.

2009-07-30 10:53:57

by Martin Schwidefsky

[permalink] [raw]
Subject: Re: [RFC][patch 00/12] clocksource / timekeeping rework V2

On Wed, 29 Jul 2009 11:17:16 -0600
[email protected] wrote:

> On Wed, 2009-07-29 at 19:09 +0200, Martin Schwidefsky wrote:
> > On Wed, 29 Jul 2009 11:02:21 -0600
> > [email protected] wrote:
> >
> > > On Wed, 2009-07-29 at 18:50 +0200, Martin Schwidefsky wrote:
> > >
> > > > void clocksource_change_rating(struct clocksource *cs, int rating)
> > >
> > > > the two functions do different things. What exactly is the idea you've
> > > > got in mind?
> > >
> > > It's only the case when the rating goes to zero .. That makes the
> > > clocksource unusable, which is very much like unregistering it..
> >
> > True, the clocksource code won't pick the clock any more as long as
> > there is an alternative clock available. It still shows up in the list
> > of clocks though and you can do an override with it.
>
> I'm not sure allowing that type of override a good idea tho .. I don't
> think it's considered a usable clock when the rating goes to zero.

Override as the root user -> your foot, no? The whole override stuff is
there for the case that the clocksource selection picked a broken clock
and you want to force the system into a semi-working state. Ideally the
whole override would go away, but that is probably wishful thinking..

--
blue skies,
Martin.

"Reality continues to ruin my life." - Calvin.

2009-07-30 12:49:30

by Daniel Walker

[permalink] [raw]
Subject: Re: [RFC][patch 00/12] clocksource / timekeeping rework V2

On Thu, 2009-07-30 at 12:53 +0200, Martin Schwidefsky wrote:

> > I'm not sure allowing that type of override a good idea tho .. I don't
> > think it's considered a usable clock when the rating goes to zero.
>
> Override as the root user -> your foot, no? The whole override stuff is
> there for the case that the clocksource selection picked a broken clock
> and you want to force the system into a semi-working state. Ideally the
> whole override would go away, but that is probably wishful thinking..

I would agree if the system doesn't crash as a result, if it just starts
to operate funny then that's maybe acceptable. If you keep the change
rating function, you could potentially remove the unregister path..

Daniel

2009-07-30 13:05:12

by Martin Schwidefsky

[permalink] [raw]
Subject: Re: [RFC][patch 00/12] clocksource / timekeeping rework V2

On Thu, 30 Jul 2009 05:49:33 -0700
Daniel Walker <[email protected]> wrote:

> On Thu, 2009-07-30 at 12:53 +0200, Martin Schwidefsky wrote:
>
> > > I'm not sure allowing that type of override a good idea tho .. I don't
> > > think it's considered a usable clock when the rating goes to zero.
> >
> > Override as the root user -> your foot, no? The whole override stuff is
> > there for the case that the clocksource selection picked a broken clock
> > and you want to force the system into a semi-working state. Ideally the
> > whole override would go away, but that is probably wishful thinking..
>
> I would agree if the system doesn't crash as a result, if it just starts
> to operate funny then that's maybe acceptable. If you keep the change
> rating function, you could potentially remove the unregister path..

Why shouldn't it be possible to have a clocksource as a module? I think
that the unregister path should stay. To really make it work we'd need
a function to force the system out of the one-shot mode though.

--
blue skies,
Martin.

"Reality continues to ruin my life." - Calvin.

2009-07-30 13:49:08

by Daniel Walker

[permalink] [raw]
Subject: Re: [RFC][patch 00/12] clocksource / timekeeping rework V2

On Thu, 2009-07-30 at 15:04 +0200, Martin Schwidefsky wrote:
> On Thu, 30 Jul 2009 05:49:33 -0700
> Daniel Walker <[email protected]> wrote:
>
> > On Thu, 2009-07-30 at 12:53 +0200, Martin Schwidefsky wrote:
> >
> > > > I'm not sure allowing that type of override a good idea tho .. I don't
> > > > think it's considered a usable clock when the rating goes to zero.
> > >
> > > Override as the root user -> your foot, no? The whole override stuff is
> > > there for the case that the clocksource selection picked a broken clock
> > > and you want to force the system into a semi-working state. Ideally the
> > > whole override would go away, but that is probably wishful thinking..
> >
> > I would agree if the system doesn't crash as a result, if it just starts
> > to operate funny then that's maybe acceptable. If you keep the change
> > rating function, you could potentially remove the unregister path..
>
> Why shouldn't it be possible to have a clocksource as a module? I think
> that the unregister path should stay. To really make it work we'd need
> a function to force the system out of the one-shot mode though.

Because I don't think there is a sane reason to allow it. It should be
more like if someone has a need for it, then let them add back the
unregister path and explain why they need it.

Daniel

2009-07-30 15:42:55

by Martin Schwidefsky

[permalink] [raw]
Subject: Re: [RFC][patch 00/12] clocksource / timekeeping rework V2

On Thu, 30 Jul 2009 07:49:12 -0600
[email protected] wrote:

> On Thu, 2009-07-30 at 15:04 +0200, Martin Schwidefsky wrote:
> > On Thu, 30 Jul 2009 05:49:33 -0700
> > Daniel Walker <[email protected]> wrote:
> >
> > > On Thu, 2009-07-30 at 12:53 +0200, Martin Schwidefsky wrote:
> > >
> > > > > I'm not sure allowing that type of override a good idea tho .. I don't
> > > > > think it's considered a usable clock when the rating goes to zero.
> > > >
> > > > Override as the root user -> your foot, no? The whole override stuff is
> > > > there for the case that the clocksource selection picked a broken clock
> > > > and you want to force the system into a semi-working state. Ideally the
> > > > whole override would go away, but that is probably wishful thinking..
> > >
> > > I would agree if the system doesn't crash as a result, if it just starts
> > > to operate funny then that's maybe acceptable. If you keep the change
> > > rating function, you could potentially remove the unregister path..
> >
> > Why shouldn't it be possible to have a clocksource as a module? I think
> > that the unregister path should stay. To really make it work we'd need
> > a function to force the system out of the one-shot mode though.
>
> Because I don't think there is a sane reason to allow it. It should be
> more like if someone has a need for it, then let them add back the
> unregister path and explain why they need it.

I hear your. Without the need to unregister a clock the whole watchdog
unregistration can be dropped as well before it ever has been added.

There are currently two clocksources that use clocksource_unregister,
the pit clock and the sgi_rtc clock. The sgi_rtc clock in
arch/x86/kernel/uv_time.c could be modified slightly to avoid the
clocksource_unregister call. The pit clock in arch/x86/kernel/i8253.c
is odd, it uses clocksource_unregister in the set_mode callback if the
mode is set to SHUTDOWN, UNUSED or ONESHOT. Very strange..

--
blue skies,
Martin.

"Reality continues to ruin my life." - Calvin.

2009-07-30 17:14:26

by john stultz

[permalink] [raw]
Subject: Re: [RFC][patch 00/12] clocksource / timekeeping rework V2

On Thu, 2009-07-30 at 12:53 +0200, Martin Schwidefsky wrote:
> On Wed, 29 Jul 2009 11:17:16 -0600
> [email protected] wrote:
>
> > On Wed, 2009-07-29 at 19:09 +0200, Martin Schwidefsky wrote:
> > > On Wed, 29 Jul 2009 11:02:21 -0600
> > > [email protected] wrote:
> > >
> > > > On Wed, 2009-07-29 at 18:50 +0200, Martin Schwidefsky wrote:
> > > >
> > > > > void clocksource_change_rating(struct clocksource *cs, int rating)
> > > >
> > > > > the two functions do different things. What exactly is the idea you've
> > > > > got in mind?
> > > >
> > > > It's only the case when the rating goes to zero .. That makes the
> > > > clocksource unusable, which is very much like unregistering it..
> > >
> > > True, the clocksource code won't pick the clock any more as long as
> > > there is an alternative clock available. It still shows up in the list
> > > of clocks though and you can do an override with it.
> >
> > I'm not sure allowing that type of override a good idea tho .. I don't
> > think it's considered a usable clock when the rating goes to zero.
>
> Override as the root user -> your foot, no? The whole override stuff is
> there for the case that the clocksource selection picked a broken clock
> and you want to force the system into a semi-working state. Ideally the
> whole override would go away, but that is probably wishful thinking..

Its also not only for when a system is broken, but quite often is used
when the system selects a slower clocksource out of caution and the user
wants to override that decision. The kernel really should get it right,
but there is always the case of an old kernel on new hardware that might
require it.

thanks
-john

2009-07-30 17:16:38

by john stultz

[permalink] [raw]
Subject: Re: [RFC][patch 00/12] clocksource / timekeeping rework V2

On Thu, 2009-07-30 at 06:49 -0700, Daniel Walker wrote:
> On Thu, 2009-07-30 at 15:04 +0200, Martin Schwidefsky wrote:
> > On Thu, 30 Jul 2009 05:49:33 -0700
> > Daniel Walker <[email protected]> wrote:
> >
> > > On Thu, 2009-07-30 at 12:53 +0200, Martin Schwidefsky wrote:
> > >
> > > > > I'm not sure allowing that type of override a good idea tho .. I don't
> > > > > think it's considered a usable clock when the rating goes to zero.
> > > >
> > > > Override as the root user -> your foot, no? The whole override stuff is
> > > > there for the case that the clocksource selection picked a broken clock
> > > > and you want to force the system into a semi-working state. Ideally the
> > > > whole override would go away, but that is probably wishful thinking..
> > >
> > > I would agree if the system doesn't crash as a result, if it just starts
> > > to operate funny then that's maybe acceptable. If you keep the change
> > > rating function, you could potentially remove the unregister path..
> >
> > Why shouldn't it be possible to have a clocksource as a module? I think
> > that the unregister path should stay. To really make it work we'd need
> > a function to force the system out of the one-shot mode though.
>
> Because I don't think there is a sane reason to allow it. It should be
> more like if someone has a need for it, then let them add back the
> unregister path and explain why they need it.

Clocksources as modules was one of the initial design goals I had way
back. The benefit being that an older distro kernel could be made to
support newer stranger hardware via a clocksource driver. While the
hardware vendors have for the most part consolidated on HPET/ACPI PM
which has mostly avoided the need, I still think its worth preserving.

thanks
-john

2009-07-30 18:08:36

by Daniel Walker

[permalink] [raw]
Subject: Re: [RFC][patch 00/12] clocksource / timekeeping rework V2

On Thu, 2009-07-30 at 10:16 -0700, john stultz wrote:
> Clocksources as modules was one of the initial design goals I had way
> back. The benefit being that an older distro kernel could be made to
> support newer stranger hardware via a clocksource driver. While the
> hardware vendors have for the most part consolidated on HPET/ACPI PM
> which has mostly avoided the need, I still think its worth preserving.

If the PIT case is a real use case for unregister than we can keep it
around. If not, then that path just becomes unused and all unused code
is open for removal from my perspective.

If the case you describe above is a good one, then someone eventually
will add back the unregister path. Which should come with a good reason
and with an actual user of the code..

Daniel

2009-07-30 20:37:26

by Andreas Mohr

[permalink] [raw]
Subject: Re: [RFC][patch 00/12] clocksource / timekeeping rework V2

Hi,

> On Thu, 2009-07-30 at 10:16 -0700, john stultz wrote:
> > Clocksources as modules was one of the initial design goals I had way
> > back. The benefit being that an older distro kernel could be made to
> > support newer stranger hardware via a clocksource driver. While the
> > hardware vendors have for the most part consolidated on HPET/ACPI PM
> > which has mostly avoided the need, I still think its worth preserving.
>
> If the PIT case is a real use case for unregister than we can keep it
> around. If not, then that path just becomes unused and all unused code
> is open for removal from my perspective.
>
> If the case you describe above is a good one, then someone eventually
> will add back the unregister path. Which should come with a good reason
> and with an actual user of the code..
>
> Daniel

I'm still having a tendency towards unhappiness about my snd-azf3328
PCI soundcard driver and its new use of clock_event_device,
without a proper unload path existing (read: clock_event_device is the sole one
of about 7 different driver components - PCM, I2S, OPL3, MPU401, AC97 mixer,
joystick, ALSA sequencer timer / clock_event_device - which now suddenly
managed to prevent the driver from being unloadable for the first time
in its entire history) and not too many explanations as to how to
get this working optimally...

Would that be enough of a justification? ;-P

Plus [OT], would anyone perhaps have an explanation why I'm getting stalls
every couple seconds (probably a race between set_next_event() programming
the next value already and timer-related interrupt handling causing a
nearby trigger to get lost, but I don't know how to resolve this
- how do other drivers manage to handle this easily racey activity??).
BTW mouse interrupt activity does NOT manage to cancel these stalls.
This is especially noticeable during video playback.

Thanks,

Andreas Mohr

2009-07-30 20:56:53

by john stultz

[permalink] [raw]
Subject: Re: [RFC][patch 00/12] clocksource / timekeeping rework V2

On Thu, 2009-07-30 at 11:08 -0700, Daniel Walker wrote:
> On Thu, 2009-07-30 at 10:16 -0700, john stultz wrote:
> > Clocksources as modules was one of the initial design goals I had way
> > back. The benefit being that an older distro kernel could be made to
> > support newer stranger hardware via a clocksource driver. While the
> > hardware vendors have for the most part consolidated on HPET/ACPI PM
> > which has mostly avoided the need, I still think its worth preserving.
>
> If the PIT case is a real use case for unregister than we can keep it
> around. If not, then that path just becomes unused and all unused code
> is open for removal from my perspective.
>
> If the case you describe above is a good one, then someone eventually
> will add back the unregister path. Which should come with a good reason
> and with an actual user of the code..

The case I describe above is one where the user of the code doesn't
necessarily have the ability to add back the unregister path.

Old distro kernels can be difficult to make changes to when new hardware
is later released, so being able to just backport a module, compile and
load it to get a unexpectedly strange new bit of hardware to work with
an older distro kernel seems valuable enough to keep the code around to
me.

thanks
-john

2009-07-31 05:33:44

by Daniel Walker

[permalink] [raw]
Subject: Re: [RFC][patch 00/12] clocksource / timekeeping rework V2

On Thu, 2009-07-30 at 13:56 -0700, john stultz wrote:
> On Thu, 2009-07-30 at 11:08 -0700, Daniel Walker wrote:
> > On Thu, 2009-07-30 at 10:16 -0700, john stultz wrote:
> > > Clocksources as modules was one of the initial design goals I had way
> > > back. The benefit being that an older distro kernel could be made to
> > > support newer stranger hardware via a clocksource driver. While the
> > > hardware vendors have for the most part consolidated on HPET/ACPI PM
> > > which has mostly avoided the need, I still think its worth preserving.
> >
> > If the PIT case is a real use case for unregister than we can keep it
> > around. If not, then that path just becomes unused and all unused code
> > is open for removal from my perspective.
> >
> > If the case you describe above is a good one, then someone eventually
> > will add back the unregister path. Which should come with a good reason
> > and with an actual user of the code..
>
> The case I describe above is one where the user of the code doesn't
> necessarily have the ability to add back the unregister path.

I'm not sure I understand your example.. Your saying a situation where
the kernel can't modified and reloaded, and the hardware clocks aren't
fully implemented in code yet?

> Old distro kernels can be difficult to make changes to when new hardware
> is later released, so being able to just backport a module, compile and
> load it to get a unexpectedly strange new bit of hardware to work with
> an older distro kernel seems valuable enough to keep the code around to
> me.

You can just as easily back port the code as a built in, and reload the
kernel right? Why would it need to be a module?

Daniel

2009-07-31 08:34:06

by john stultz

[permalink] [raw]
Subject: Re: [RFC][patch 00/12] clocksource / timekeeping rework V2

On Thu, 2009-07-30 at 22:33 -0700, Daniel Walker wrote:
> On Thu, 2009-07-30 at 13:56 -0700, john stultz wrote:
> > On Thu, 2009-07-30 at 11:08 -0700, Daniel Walker wrote:
> > > On Thu, 2009-07-30 at 10:16 -0700, john stultz wrote:
> > > > Clocksources as modules was one of the initial design goals I had way
> > > > back. The benefit being that an older distro kernel could be made to
> > > > support newer stranger hardware via a clocksource driver. While the
> > > > hardware vendors have for the most part consolidated on HPET/ACPI PM
> > > > which has mostly avoided the need, I still think its worth preserving.
> > >
> > > If the PIT case is a real use case for unregister than we can keep it
> > > around. If not, then that path just becomes unused and all unused code
> > > is open for removal from my perspective.
> > >
> > > If the case you describe above is a good one, then someone eventually
> > > will add back the unregister path. Which should come with a good reason
> > > and with an actual user of the code..
> >
> > The case I describe above is one where the user of the code doesn't
> > necessarily have the ability to add back the unregister path.
>
> I'm not sure I understand your example.. Your saying a situation where
> the kernel can't modified and reloaded, and the hardware clocks aren't
> fully implemented in code yet?

Right. Distro kernels.

> > Old distro kernels can be difficult to make changes to when new hardware
> > is later released, so being able to just backport a module, compile and
> > load it to get a unexpectedly strange new bit of hardware to work with
> > an older distro kernel seems valuable enough to keep the code around to
> > me.
>
> You can just as easily back port the code as a built in, and reload the
> kernel right? Why would it need to be a module?

Again, distro kernels. Users can't rebuild them without possibly losing
the support they've paid for, and often recompiling them can cause 3rd
party drivers to fail to work (some distros preserve kernel ABI
stability between minor releases). Waiting 6 months or two years for the
next release where everything is fixed upstream isn't going to make
users happy.

Now, with most hardware vendors implementing decent HPET/ACPI PM
counters, maybe this case is more me reacting to a bad situation I had
to deal with in the past then what we can realistically expect in the
future. But given hardware designers like to break assumptions to
squeeze out performance or features, I'd suspect there will be future
situations where having some extra flexibility would be valuable.

Imaginary example: broken BIOS has incorrect HPET freq and the TSCs are
not in sync. Savvy IT dude finds the problem, copies the HPET driver,
names it hpet-fix and hard codes the proper HPET freq in. Sets the
rating higher then HPET, builds it as a module and loads it on the
affected hardware.

Suddenly there's a solution where otherwise none might be possible.

thanks
-john

2009-07-31 16:44:26

by Daniel Walker

[permalink] [raw]
Subject: Re: [RFC][patch 00/12] clocksource / timekeeping rework V2

On Fri, 2009-07-31 at 01:34 -0700, john stultz wrote:

>
> Again, distro kernels. Users can't rebuild them without possibly losing
> the support they've paid for, and often recompiling them can cause 3rd
> party drivers to fail to work (some distros preserve kernel ABI
> stability between minor releases). Waiting 6 months or two years for the
> next release where everything is fixed upstream isn't going to make
> users happy.

It wouldn't need to be a module. The distro would update the kernel as
needed.. Just the act of loading an unauthorized kernel module would
potentially invalidate any distro support someone might get.. Distro's
typically provide backported fixes also.

> Now, with most hardware vendors implementing decent HPET/ACPI PM
> counters, maybe this case is more me reacting to a bad situation I had
> to deal with in the past then what we can realistically expect in the
> future. But given hardware designers like to break assumptions to
> squeeze out performance or features, I'd suspect there will be future
> situations where having some extra flexibility would be valuable.
>
> Imaginary example: broken BIOS has incorrect HPET freq and the TSCs are
> not in sync. Savvy IT dude finds the problem, copies the HPET driver,
> names it hpet-fix and hard codes the proper HPET freq in. Sets the
> rating higher then HPET, builds it as a module and loads it on the
> affected hardware.

The IT guy more than likely would need to rebuild the kernel multiple
times to discover what the problem was .. In the end the distro would
push a fix for this to mainline, and provide a new kernel for the distro
users with a backported fix.

If there a potential for a clocksource to have some type of issue like
what you describe for the HPET, wouldn't it be easier to have all those
as tunable boot args or sysfs options ..

Daniel