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

add cpuinfo for loongarch64 #384

Merged
merged 1 commit into from Dec 17, 2022
Merged

Conversation

zhangwenlong8911
Copy link
Contributor

No description provided.

@zhangwenlong8911
Copy link
Contributor Author

@SuperQ Hello, I added cpuinfo for loongarch64, at the same time I have compiled and tested locally, and have used the codes in other projects without any problems. can you help me to review the code? Thank you

@SuperQ
Copy link
Member

SuperQ commented Jun 1, 2021

Thanks, I'll take a look at this soon.

@discordianfish
Copy link
Member

Oh exciting! I don't think it's possible to get loongarch64 systems anywhere yet, right? Or do you know a way? So I can't really verify this..

@zhangwenlong8911
Copy link
Contributor Author

zhangwenlong8911 commented Jun 9, 2021

loongarch64 is a new system, and command “make test” in loongarch64 is ok, I can provide a screenshot of the result

procfs构建结果

1111111

@xen0n
Copy link

xen0n commented Jun 9, 2021

The LoongArch kernel support hasn't been upstream yet; even the first submission is yet to appear. So in theory the /proc/cpuinfo format could change; I suggest we wait for the moment, and only revise and merge after the kernel bits get upstreamed.

@discordianfish
Copy link
Member

I'm fine either way, merging this also wouldn't break anything so..

@zhangwenlong8911 Do you know if I can buy a loongarch64 system somewhere online? Or how did you get your hands on one?

@xen0n
Copy link

xen0n commented Jul 1, 2021

FYI, the initial LoongArch kernel bits appeared; but it's still long before being mainlined, so technically things could still change. For now the /proc/cpuinfo format is very much MIPS-like, and I doubt it would persist into the final version.

@zhangwenlong8911
Copy link
Contributor Author

zhangwenlong8911 commented Jul 5, 2021

@discordianfish I am a member of the loongarch64 team, so I can get first-hand information, LoongArch kernel support has been submitted to the kernel community

Copy link

@xen0n xen0n left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aren't we using "loong/loong64" everywhere in the Go land instead of "loongarch/loongarch64"? Plus the /proc/cpuinfo format issue hasn't settled yet. We really should wait a bit longer.

@discordianfish
Copy link
Member

+1 for consistency, if it's called 'loong(64)' in golang, let's do it here as well.

@zhangwenlong8911
Copy link
Contributor Author

I think you are right@xen0n @discordianfish, I modify the architecture information now, about / proc / cpuinfo, let's wait

cpuinfo_loong64.go Outdated Show resolved Hide resolved
cpuinfo.go Outdated Show resolved Hide resolved
cpuinfo_test.go Outdated Show resolved Hide resolved
cpuinfo.go Outdated Show resolved Hide resolved
cpuinfo_loong64.go Outdated Show resolved Hide resolved
cpuinfo_others.go Outdated Show resolved Hide resolved
cpuinfo_test.go Outdated Show resolved Hide resolved
cpuinfo_test.go Outdated Show resolved Hide resolved
cpuinfo_test.go Outdated Show resolved Hide resolved
cpuinfo.go Show resolved Hide resolved
Copy link

@xen0n xen0n left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make sure to test your code on multiple arches (at least test amd64 too, in your case).

cpuinfo_loong64.go Show resolved Hide resolved
cpuinfo_test.go Outdated Show resolved Hide resolved
cpuinfo_test.go Outdated Show resolved Hide resolved
cpuinfo_test.go Outdated Show resolved Hide resolved
cpuinfo_test.go Show resolved Hide resolved
@xen0n
Copy link

xen0n commented Aug 5, 2022

Finally, please add Signed-off-by line. (see that failing DCO check?)

While at it, modify your PR title and commit message so LoongArch64 is uniformly referred to as loong64 in the Go ecosystem.

Copy link
Member

@discordianfish discordianfish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Signed-off-by: Wenlong Zhang <zhangwenlong@loongson.cn>
@SuperQ
Copy link
Member

SuperQ commented Dec 17, 2022

Go 1.19 finally supports this platform.

@SuperQ SuperQ merged commit 88f86e5 into prometheus:master Dec 17, 2022
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

Successfully merging this pull request may close these issues.

None yet

5 participants