Use the PAGEMAP_SCAN ioctl when it is available

[avagin]

No description provided.

[codecov-commenter]

Codecov Report

Attention: 38 lines in your changes are missing coverage. Please review.

Comparison is base (8f4430d) 70.62% compared to head (0300efe) 70.57%.

❗ Current head 0300efe differs from pull request most recent head e11853f. Consider uploading reports for the commit e11853f to get more accurate results

Files	Patch %	Lines
criu/pagemap-cache.c	44.44%	20 Missing ⚠️
criu/kerndat.c	35.71%	9 Missing ⚠️
criu/mem.c	77.50%	9 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##           criu-dev    #2292      +/-   ##
============================================
- Coverage     70.62%   70.57%   -0.05%     
============================================
  Files           133      132       -1     
  Lines         33556    33619      +63     
============================================
+ Hits          23698    23726      +28     
- Misses         9858     9893      +35     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[0x7f454c46]

Hmm, at some moment all kernels we regularly run CRIU tests on will have this PAGEMAP_SCAN. I'm wondering if it is worth testing pre-ioctl code by artificially setting has_pagemap_scan = false? That will make sure that the fallback for older kernels won't get broken.
Not only related to this PR, but probably to other options in kerndat as well. Maybe a wild idea to add a command-line option like --kerndat-disable-pagemap-scan and to other kerndat features (rather than adding more fault-injects)?
Likely can be addressed in a separate CRIU issue/PR.

[adrianreber]

I had similar thoughts around how to test this. But as it is still marked as draft I was still waiting if tests will be part of this PR.

I like the idea of explicitly selecting kerndat features. It does not make too much sense to expose all the options to the users as it might be confusing. But it probably could be helpful in some cases.

[Snorch]

Maybe a wild idea to add a command-line option like --kerndat-disable-pagemap-scan and to other kerndat features (rather than adding more fault-injects)?

Would not it be cleaner to instead add an ability to read/write/edit kdat cache from crit tool? (Sounds like a simple task for GSOC students or any other newcomers.)

Likely can be addressed in a separate CRIU issue/PR.

+1

[0x7f454c46]

Would not it be cleaner to instead add an ability to read/write/edit kdat cache from crit tool?

Agree, maybe another tool. Unsure about crit - then we would have to save kerndat in protobuf format and might do the same backward-compatibility as criu does. Hopefully, we can avoid keeping/maintaining compatibility as the intended use is testing-only.

[avagin]

Would not it be cleaner to instead add an ability to read/write/edit kdat cache from crit tool?

Usually, we use injectable faults in such cases. Look at the third patch.

[Snorch]

Looks good.

I found this in kernel doc of PAGEMAP_SCAN ioctl:

he PAGE_IS_WRITTEN flag can be considered as a better-performing alternative
of soft-dirty flag. It doesn't get affected by VMA merging of the kernel and hence
the user can find the true soft-dirty pages in case of normal pages. (There may
still be extra dirty pages reported for THP or Hugetlb pages.)

Should we use PAGE_IS_WRITTEN instead of PAGE_IS_SOFT_DIRTY, as it is called "better-performing"?

[github-actions]

A friendly reminder that this PR had no activity for 30 days.

[avagin]

Should we use PAGE_IS_WRITTEN instead of PAGE_IS_SOFT_DIRTY, as it is called "better-performing"?

It is not so simple. PAGE_IS_WRITTEN requires to register all regions in uffd-s and holds them between pre-dumps. It is a separate task.