checkpoint-restore / criu

Checkpoint/Restore tool
criu.org
Other
2.79k stars 565 forks source link

vma: Add !VVAR condition to vma_entry_can_be_lazy #2272

Closed yota9 closed 10 months ago

yota9 commented 10 months ago

Currently most of the times we don't have problems with VVAR segment and lazy restore because when VDSO is parked there is an munmap call that calls UFFDIO_UNREGISTER on the destination address. But we don't want to enable userfaultfd for VDSO and VVAR at the first place.

codecov-commenter commented 10 months ago

Codecov Report

All modified lines are covered by tests :white_check_mark:

Comparison is base (1e4f5fb) 70.54% compared to head (1c5357b) 70.54%.

:exclamation: Current head 1c5357b differs from pull request most recent head df497f2. Consider uploading reports for the commit df497f2 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## criu-dev #2272 +/- ## ========================================= Coverage 70.54% 70.54% ========================================= Files 133 132 -1 Lines 33522 33510 -12 ========================================= - Hits 23647 23640 -7 + Misses 9875 9870 -5 ``` | [Files](https://app.codecov.io/gh/checkpoint-restore/criu/pull/2272?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=checkpoint-restore) | Coverage Δ | | |---|---|---| | [criu/include/vma.h](https://app.codecov.io/gh/checkpoint-restore/criu/pull/2272?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=checkpoint-restore#diff-Y3JpdS9pbmNsdWRlL3ZtYS5o) | `100.00% <100.00%> (ø)` | | ... and [4 files with indirect coverage changes](https://app.codecov.io/gh/checkpoint-restore/criu/pull/2272/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=checkpoint-restore)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

avagin commented 10 months ago

lgtm

rppt commented 10 months ago

AFAIU Currently most of the times we don't have problems with VVAR segment due to during parking vdso on it's old place mremap is used and during it munmap is called to destination address which results in calling UFFDIO_UNREGISTER. But as for VDSO we don't want to enable uffd for these addresses at firse place.

I find the changelog a bit hard to parse, how about

Currently most of the times we don't have problems with VVAR segment and lazy restore because when VDSO is parked there is an munmap call that calls UFFDIO_UNREGISTER on the destination address. But we don't want to enable userfaultfd for VDSO and VVAR at the first place.

Other than that, LGTM

yota9 commented 10 months ago

@rppt Thanks, done. Please merge it since I don't have rights.