port to loongarch64 #2183
port to loongarch64 #2183
Conversation
znley
force-pushed
the
criu-dev-loongarch64
branch
2 times, most recently
from
3312460
to
03dd186
Compare
|
@mihalicyn Is there more suggestions on this? |
|
Hi @znley! Sorry about delay with review. Thanks for working on porting CRIU to the new architecture. While I'm looking closely through the code, I want to ask you two things:
I'll explain why I'm asking. We already have support for mips architecture, and it's not a perfect experience, because we are not sure that it's working and we have no way to test it on our own. So, IMHO, it would be great to have some way to test this without hardware. Can you also prepare such a qemu image/configuration and describe how to do that? You can put this instruction as a separate file in the Documentation directory in CRIU tree. |
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## criu-dev #2183 +/- ##
============================================
- Coverage 70.35% 70.31% -0.05%
============================================
Files 133 133
Lines 34001 33989 -12
============================================
- Hits 23923 23900 -23
- Misses 10078 10089 +11 ☔ View full report in Codecov by Sentry. |
|
@mihalicyn First, I plan to split it into image, compel, include, criu and Makefile changes. How do you think? About how to test. There are two ways:
|
The idea is not about to split changes to different directories into a different commits. What we want is to have each commit to be an atomic change that:
For instance, you can split your changes to be like:
may be later we will split it to even smaller pieces, but I think this looks optimal for now. Speaking generally, we have the same policies as Linux kernel has. If you have a series like 60 commits to the Linux kernel then each commit should work and pass tests and should not bring any degradation (at least known). There is a reason to have such a policy - it allows to perform Hint: you can use |
|
please take a look on linter failures: |
avagin
force-pushed
the
criu-dev
branch
2 times, most recently
from
f392ea1
to
4d137b8
Compare
znley
force-pushed
the
criu-dev-loongarch64
branch
2 times, most recently
from
dd857fd
to
16c6dc0
Compare
|
Hi, @mihalicyn One of the most important thing is I have finished a docker image where criu tests can be done. You can use it on x86 machine with About this image:
Known issus with criu running on loongarch64:
Finally, thank you for your patience and guidance, I split the commit and fixed the linter failure but it might not be standard yet. |
|
Hi @znley! Great job! I'll play with this thing then. From the first look I have a question. Where I should take this files:
Can you try to integrate this into GitHub actions? My understanding is that you need:
Thanks for work on that! |
Sorry I forgot to explain this problem. I put all binaries under archives directory, but didn't commit these onto git because the file size is large. I don't have a better way yet, maybe I will package and upload the archives to github release later. It is better to use docker at present. Archives consists of:
As for how to integrate into CI, I may need to do some examples first. |
thanks for clarification!
sure, feel free to ask if you have any questions. I can advice you to create a separate branch in your CRIU fork for this CI work, create a new separate pipeline that:
next step is to integrate CRIU. Even before that you can just add a new cross-compilation target here: |
znley
force-pushed
the
criu-dev-loongarch64
branch
from
16c6dc0
to
bfc9a23
Compare
|
@mihalicyn With this work znley@927ae43, an I have adjusted the previous archlinux image without adopting the fedora you mentioned. Please see if it works. |
|
Build and test are integrated into the CI. There are two problems:
|
znley
force-pushed
the
criu-dev-loongarch64
branch
from
bfc9a23
to
8fd1607
Compare
|
The entire test run is complete. It took 2 hour and 28 minutes. |
znley
force-pushed
the
criu-dev-loongarch64
branch
2 times, most recently
from
43c98e8
to
7fe7798
Compare
|
|
Hi @znley Excellent work, thanks! Btw, it seems like Minimal reproducer: Kernel logs: So, we have a problem with loongarch64 kernel (or) Qemu? Can you try to run this reproducer on real hardware and on VM just to confirm? |
yes, that's why I think that it's worth to run only a few tests, not a full ZDTM testsuite. I think it will be sufficient for GitHub Actions environment. But I've a plan to resurrect our Jenkins and add CI Job for loongarch64 once we merge it, so we will be able to run full testsuite on our Jenkins CI. |
perfect! I've played with this thing locally. Thanks a lot for doing this! IMHO, let's add CI to this branch and run limited subset of tests like this (we can extend it later): I think that fixups like loongarch64 fix: avoid deprecated warning that $sp in a clobber list and loongarch64 fix: avoid unused compat variable warning should be squashed into an appropriate commits. |
znley
force-pushed
the
criu-dev-loongarch64
branch
from
7fe7798
to
21dd41b
Compare
|
@mihalicyn
Do you mean add CI to criud-dev branch? I'm not sure about this. If so, open another PR or just in this PR? About the |
yeah, I think you can add your CI job just in a scope of this PR.
thanks! That's very interesting if you'll be able to reproduce this behaviour on your environment in QEMU, and then if it's reproducible with QEMU it's worth trying to run this on a hardware. Anyway, this looks like a bug somewhere in Qemu (may be interrupt is not properly generated on a page fault) or Linux kernel. So testing on a hardware allows us to isolate and determine if this is a QEMU or Linux kernel. |
znley
force-pushed
the
criu-dev-loongarch64
branch
from
53c0b53
to
e4692d0
Compare
|
@mihalicyn Meanwhile CI runs a limited subset of tests now. |
yes, looks like it is.
thanks a lot. As I can see tests are all green now. I think this PR looks good to me and I've a plan to merge it on a weekend. May be I'll do some small changes in a commit descriptions or something like that, but generally it's in a perfect shape! Thanks for your work on that and hope that you'll take care of this port and help us to maintain it properly. |
Signed-off-by: znley <[email protected]>
mihalicyn
force-pushed
the
criu-dev-loongarch64
branch
from
e4692d0
to
5faf885
Compare
Signed-off-by: znley <[email protected]>
Signed-off-by: znley <[email protected]>
Signed-off-by: znley <[email protected]>
Signed-off-by: znley <[email protected]>
Signed-off-by: znley <[email protected]>
mihalicyn
force-pushed
the
criu-dev-loongarch64
branch
from
5faf885
to
53f1b58
Compare
|
@znley I've made a small changes in your PR branch (fixed spelling and commit message format), also made linter happy. I'll merge a PR right now. It looks like there is some issues with GitHub Actions runners and we experiencing issues with SSL like this: https://github.com/checkpoint-restore/criu/actions/runs/5601283935/jobs/10244922111?pr=2183 Anyway, it's not related to your changes and I've seen this tests in "green" state. I've also made appropriate testing on my local environment to confirm that everything is in a perfect shape. |
mihalicyn
merged commit f70c782
into
checkpoint-restore:criu-dev
I just did a little work. Thanks for your patient guidance and time again. it has helped me a lot. Test issues including At the same time, I will make necessary improvements for CI with operating system update. Keep communicate with any related questions at any time. Respect! |
See: checkpoint-restore/criu#2183 Signed-off-by: WANG Xuerui <[email protected]>
This patch will allows criu to support the loongarch64 platoform. Most featues are available, but there are still some known and unknow bugs.
The result of
zdtm testrun on my loongarch64 machine as follows:As you can see this patch is not perfect, so I hope to get your advice on what else I need to do next.