Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build fail for riscv64 due to test case with cpu_speed() #105

Open
yuzibo opened this issue Apr 21, 2022 · 2 comments
Open

Build fail for riscv64 due to test case with cpu_speed() #105

yuzibo opened this issue Apr 21, 2022 · 2 comments

Comments

@yuzibo
Copy link

yuzibo commented Apr 21, 2022

Hi,
From Debian rust sys-info package which FBTFS fail:
https://buildd.debian.org/status/package.php?p=rust-sys-info&suite=sid
I submit a patch to pass the tests case:
https://salsa.debian.org/rust-team/debcargo-conf/-/merge_requests/295
But It seems very suboptimally.
So what is the best solution for such case?
The /proc/cpuinfo is below:

root@unmatched-local:~/git/debcargo-conf/src/sys-info/debian# cat /proc/cpuinfo processor       : 0 hart            : 1 isa             : rv64imafdc mmu             : sv39 uarch           : sifive,bullet0  processor       : 1 hart            : 2 isa             : rv64imafdc mmu             : sv39 uarch           : sifive,bullet0  processor       : 2 hart            : 3 isa             : rv64imafdc mmu             : sv39 uarch           : sifive,bullet0  processor       : 3 hart            : 4 isa             : rv64imafdc mmu             : sv39 uarch           : sifive,bullet0

If no other options, I will send RP to pass the case that is the same patch for Debian.

@pabs3
Copy link

pabs3 commented Apr 21, 2022

The /proc/cpuinfo file is very variable across Linux architectures and even varies at runtime, as an install moves between CPUs (such as with a Linux live image) or even as you switch between Linux kernel versions (since newer ones might expose more info on very new hardware such as RISC-V). So the CPU speed test should probably not be run at build time. Instead the API should indicate that the function can return nothing/error at runtime and users of the API should be expected to handle that at runtime.

The CPU speed feature also isn't very useful, since at least on my Intel system, the cpu MHz field is different for each core and different each time that I read the /proc/cpuinfo file. So perhaps the CPU speed feature should be deprecated and later removed.

@davidlt
Copy link

davidlt commented Feb 25, 2024

+1 for @pabs3 comment. That field does not provide anything useful as clocks will change rapidly, and they are different per core. This part should be modified to incl. riscv64:
https://github.com/FillZpp/sys-info-rs/blob/60ecf1470a5b7c90242f429934a3bacb6023ec4d/lib.rs#L600C1-L603C6
and it will return: Err(Error::UnsupportedSystem)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants