go-sysinfo icon indicating copy to clipboard operation
go-sysinfo copied to clipboard

Linux kernel ticks assumption is error prone

Open uri-weisman opened this issue 2 years ago • 3 comments

Problem

The value of clock ticks (_SC_CLK_TCK) is hard coded and might varies across kernel versions and hardware platforms, it would probably be best if you could read this value dynamically. https://github.com/elastic/go-sysinfo/blob/bd131ab8b5efa45cd7e434c4b14412a253601e07/providers/linux/process_linux.go#L33

There is an assumption here for certain conditions that might not be respected between different deployments and versions. please read the wiki article about Kernel Timer Systems:

The original kernel timer system (called the "timer wheel) was based on incrementing a kernel-internal value (jiffies) every timer interrupt. The timer interrupt becomes the default scheduling quantum, and all other timers are based on jiffies. The timer interrupt rate (and jiffy increment rate) is defined by a compile-time constant called HZ. Different platforms use different values for HZ. Historically, the kernel used 100 as the value for HZ, yielding a jiffy interval of 10 ms. With 2.4, the HZ value for i386 was changed to 1000, yielding a jiffy interval of 1 ms. Recently (2.6.13) the kernel changed HZ for i386 to 250. (1000 was deemed too high).

Proposed solution

Make a sys call to retrieve the actual _SC_CLK_TCK and use it for process time calculations, the same has been done for Darwin: https://github.com/elastic/go-sysinfo/blob/f2015f14dd215ad97bd4fbe048e087945ec0223d/providers/darwin/syscall_darwin.go#L191-L194

uri-weisman avatar Jul 07 '22 09:07 uri-weisman

IIUC, looking at 2.6.23 (the earliest kernel supported by Go) the two architectures that do not use 100 HZ are alpha and ia64. And neither of those are supported by Go.

But I agree it would be more correct to use the sysconf(_SC_CLK_TCK). Currently cgo is not required for the Linux provider implementation. I think it would be good to keep it that way and offer two implementations. One that uses the constant (ticks_nocgo.go) and one that uses code similar to the proposal above (ticks_cgo.go).

There's some good discussion of this in https://github.com/containerd/cgroups/pull/12.

andrewkroh avatar Jul 07 '22 12:07 andrewkroh

I was curious what go-sysconf did for this since it claims to not use cgo.

https://github.com/tklauser/go-sysconf/blob/0dc6a3a166617b00a369c95264f8ee435c0a4910/sysconf_linux.go#L19-L26

const (
	// CLK_TCK is a constant on Linux for all architectures except alpha and ia64.
	// See e.g.
	// https://git.musl-libc.org/cgit/musl/tree/src/conf/sysconf.c#n30
	// https://github.com/containerd/cgroups/pull/12
	// https://lore.kernel.org/lkml/[email protected]/
	_SYSTEM_CLK_TCK = 100
)

andrewkroh avatar Jul 07 '22 12:07 andrewkroh

@andrewkroh - thank you for the quick response.

IIUC, looking at 2.6.23 (the earliest kernel supported by Go) the two architectures that do not use 100 HZ are alpha and ia64. And neither of those are supported by Go.

Seems they are both obsolete so it makes this issue less important. Yeah, two different implementations sound about right - allowing the user to decide if he wants to support cgo.

uri-weisman avatar Jul 07 '22 14:07 uri-weisman