From: Brian Bloniarz on
john stultz wrote:
> On Tue, 2010-05-25 at 20:16 -0400, Brian Bloniarz wrote:
>> On 05/24/2010 09:33 PM, Brian Bloniarz wrote:
>>> So what's wrong with just adding a
>>> /sys/devices/system/clocksource/clocksource0/tsc_khz?
>> As an RFC:
>>
>> Add clocksource.sys_register & sys_unregister so the
>> current clocksource can add supplemental information to
>> /sys/devices/system/clocksource/clocksource0/
>>
>> Export tsc_khz when current_clocksource==tsc so that
>> daemons like NTP can account for the variability of
>> calibration results.
>
> I think this is a bad idea, as it creates an ABI that is arch AND
> machine specific, which will cause portability problems in applications
> that expect the interface to be there.

It's an arch-independent ABI that returns ENOENT on
unsupported platforms ;)

Could you please explain what you envision as an
arch-independent solution to this problem?
I guess the tsc_long_calibration=1 alternative is
one.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Thomas Gleixner on
On Tue, 25 May 2010, Brian Bloniarz wrote:

> john stultz wrote:
> > On Tue, 2010-05-25 at 20:16 -0400, Brian Bloniarz wrote:
> >> On 05/24/2010 09:33 PM, Brian Bloniarz wrote:
> >>> So what's wrong with just adding a
> >>> /sys/devices/system/clocksource/clocksource0/tsc_khz?
> >> As an RFC:
> >>
> >> Add clocksource.sys_register & sys_unregister so the
> >> current clocksource can add supplemental information to
> >> /sys/devices/system/clocksource/clocksource0/
> >>
> >> Export tsc_khz when current_clocksource==tsc so that
> >> daemons like NTP can account for the variability of
> >> calibration results.
> >
> > I think this is a bad idea, as it creates an ABI that is arch AND
> > machine specific, which will cause portability problems in applications
> > that expect the interface to be there.
>
> It's an arch-independent ABI that returns ENOENT on
> unsupported platforms ;)
>
> Could you please explain what you envision as an
> arch-independent solution to this problem?
> I guess the tsc_long_calibration=1 alternative is
> one.

Arch independent solution is to provide information about the current
clock source in general. This is _NOT_ a TSC specific problem, you
have the same trouble with any other clocksource which gets calibrated
and does not take it's frequency as a constant value from boot loader,
configuration or some CPU/chipset register. The only missing piece is
a frequency member in struct clocksource which needs to be filled in
by the arch/machine specific code.

Thanks,

tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Thomas Gleixner on
On Tue, 25 May 2010, Brian Bloniarz wrote:

> On 05/24/2010 09:33 PM, Brian Bloniarz wrote:
> > So what's wrong with just adding a
> > /sys/devices/system/clocksource/clocksource0/tsc_khz?

It's wrong because TSC is an x86'ism.

> As an RFC:
>
> Add clocksource.sys_register & sys_unregister so the
> current clocksource can add supplemental information to
> /sys/devices/system/clocksource/clocksource0/
>
> Export tsc_khz when current_clocksource==tsc so that
> daemons like NTP can account for the variability of
> calibration results.

I'd rather see a generic solution which provides the information of
the current (and possibly those of the available) clock source(s).

This x86 centric TSC world view is horrible.

Thanks,

tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Dan Magenheimer on
> From: Thomas Gleixner [mailto:tglx(a)linutronix.de]
>
> On Tue, 25 May 2010, Brian Bloniarz wrote:
>
> > john stultz wrote:
> > > On Tue, 2010-05-25 at 20:16 -0400, Brian Bloniarz wrote:
> > >> On 05/24/2010 09:33 PM, Brian Bloniarz wrote:
> > >>> So what's wrong with just adding a
> > >>> /sys/devices/system/clocksource/clocksource0/tsc_khz?
> > >> As an RFC:
> > >>
> > >> Add clocksource.sys_register & sys_unregister so the
> > >> current clocksource can add supplemental information to
> > >> /sys/devices/system/clocksource/clocksource0/
> > >>
> > >> Export tsc_khz when current_clocksource==tsc so that
> > >> daemons like NTP can account for the variability of
> > >> calibration results.
> > >
> > > I think this is a bad idea, as it creates an ABI that is arch AND
> > > machine specific, which will cause portability problems in
> applications
> > > that expect the interface to be there.
> >
> > It's an arch-independent ABI that returns ENOENT on
> > unsupported platforms ;)
> >
> > Could you please explain what you envision as an
> > arch-independent solution to this problem?
> > I guess the tsc_long_calibration=1 alternative is
> > one.
>
> Arch independent solution is to provide information about the current
> clock source in general. This is _NOT_ a TSC specific problem, you
> have the same trouble with any other clocksource which gets calibrated
> and does not take it's frequency as a constant value from boot loader,
> configuration or some CPU/chipset register. The only missing piece is
> a frequency member in struct clocksource which needs to be filled in
> by the arch/machine specific code.

Actually there is already a frequency in struct clocksource except
it's represented by the two components: mult and shift. Maybe
it would be best to expose these instead of khz (for all clocksources)
so as to limit abuse by naive users.

So, Thomas and John, if Brian's patch is modified to provide:

/sys/devices/system/clocksource/clocksource0/current_mult
/sys/devices/system/clocksource/clocksource0/current_shift
and/or
/sys/devices/system/clocksource/clocksource0/current_khz

is that an acceptable arch-independent patch? (And which do
you prefer?)

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: john stultz on
On Tue, 2010-05-25 at 22:50 -0400, Brian Bloniarz wrote:
> john stultz wrote:
> > On Tue, 2010-05-25 at 20:16 -0400, Brian Bloniarz wrote:
> >> On 05/24/2010 09:33 PM, Brian Bloniarz wrote:
> >>> So what's wrong with just adding a
> >>> /sys/devices/system/clocksource/clocksource0/tsc_khz?
> >> As an RFC:
> >>
> >> Add clocksource.sys_register & sys_unregister so the
> >> current clocksource can add supplemental information to
> >> /sys/devices/system/clocksource/clocksource0/
> >>
> >> Export tsc_khz when current_clocksource==tsc so that
> >> daemons like NTP can account for the variability of
> >> calibration results.
> >
> > I think this is a bad idea, as it creates an ABI that is arch AND
> > machine specific, which will cause portability problems in applications
> > that expect the interface to be there.
>
> It's an arch-independent ABI that returns ENOENT on
> unsupported platforms ;)

Right but having applications add "Linux on x86 where the TSC is being
used" logic is pretty poor solution. Its an issue that should be
addressed from the kernel side.

And really, if apps really wanted this info, they can fish it out
of /proc/cpuinfo.

> Could you please explain what you envision as an
> arch-independent solution to this problem?
> I guess the tsc_long_calibration=1 alternative is
> one.

....and the tsc_khz= patch I posted earlier.


thanks
-john


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/