Hey All, >On 19/04/2024 18:12, David Hildenbrand wrote: >> On 19.04.24 18:30, Mike Rapoport wrote: >>> On Fri, Apr 19, 2024 at 11:45:14AM +0200, David Hildenbrand wrote: >>>> On 19.04.24 10:33, Shivansh Vij wrote: >>>>>> On 19/04/2024 08:43, Ryan Roberts wrote: >>>>>>> Hi All, >>>>>>> >>>>>>> This series adds uffd write-protect and soft-dirty tracking support for >>>>>>> arm64. I >>>>>>> consider the soft-dirty support (patches 3 and 4) as RFC - see rationale >>>>>>> below. >>>>>>> >>>>>>> That said, these are the last 2 SW bits and we may want to keep 1 bit in >>>>>>> reserve >>>>>>> for future use. soft-dirty is only used for CRIU to my knowledge, and it is >>>>>>> thought that their use case could be solved with the more generic uffd-wp. So >>>>>>> unless somebody makes a clear case for the inclusion of soft-dirty >>>>>>> support, we >>>>>>> are probably better off dropping patches 3 and 4 and keeping bit 63 for >>>>>>> future >>>>>>> use. Although note that the most recent attempt to add soft-dirty for >>>>>>> arm64 was >>>>>>> last month [1] so I'd like to give Shivansh Vij the opportunity to make the >>>>>>> case. >>>>> >>>>> Appreciate the opportunity to provide input here. >>>>> >>>>> I picked option one (dirty tracking in arm) because it seems to be the >>>>> simplest way to move forward, whereas it would be a relatively heavy >>>>> effort to add uffd-wp support to CRIU. >>>>> >>>>> From a performance perspective I am also a little worried that uffd >>>>> will be slower than just tracking the dirty bits asynchronously with >>>>> sw dirty, but maybe that's not as much of a concern with the addition >>>>> of uffd-wp async. >>>>> >>>>> With all this being said, I'll defer to the wisdom of the crowd about >>>>> which approach makes more sense - after all, with this patch we should >>>>> get uffd-wp support on arm so at least there will be _a_ way forward >>>>> for CRIU (albeit one requiring slightly more work). >>>> >>>> Ccing Mike and Peter. In 2017, Mike gave a presentation "Memory tracking for >>>> iterative container migration"[1] at LPC >>>> >>>> Some key points are still true I think: >>>> (1) More flexible and robust than soft-dirty >>>> (2) May obsolete soft-dirty >>>> >>>> We further recently added a new UFFD_FEATURE_WP_ASYNC feature as part of >>>> [2], because getting soft-dirty return reliable results in some cases turned >>>> out rather hard to fix. > >But it sounds like the current soft-dirty semantic is sufficient for CRIU on >other arches? If I understood correctly from my brief scan of the linked post, >the problem is that soft-dirty can sometimes provide false-positives? So could >result in uneccessary copy, but never lost data? This is how I've always understood it as well. > >>>>> >>>>> We might still have to optimize that approach for some very sparse large >>>>> VMAs, but that should be solvable. >>>>> >>>> "The major defect of this approach of dirty tracking is we need to >>>> populate the pgtables when tracking starts. Soft-dirty doesn't do it >>>> like that. It's unwanted in the case where the range of memory to track >>>> is huge and unpopulated (e.g., tracking updates on a 10G file with >>>> mmap() on top, without having any page cache installed yet). One way to >>>> improve this is to allow pte markers exist for larger than PTE level >>>> for PMD+. That will not change the interface if to implemented, so we >>>> can leave that for later.")[3] >>>> >>>> >>>> If we can avoid adding soft-dirty on arm64 that would be great. This will >>>> require work on the CRIU side. One downside of uffd-wp is that it is >>>> currently not as avilable on architectures as soft-dirty. >>> >>> Using uffd-wp instead of soft-dirty in CRIU will require quite some work on >>> CRIU side and probably on the kernel side too. >>> >>> And as of now we'll anyway have to maintain soft-dirty because powerpc and >>> s390 don't have uffd-wp. >>> >>> With UFFD_FEATURE_WP_ASYNC the concern that uffd-wp will be slower than >>> soft-dirty probably doesn't exist, but we won't know for sure until >>> somebody will try. >>> >>> But there were other limitations, the most prominent was checkpointing an >>> application that uses uffd. If CRIU is to use uffd-wp for tracking of the >>> dirty pages, there should be some support for multiple uffd contexts for a >>> VMA and that's surely a lot of work. >> >> Is it even already supported to checkpoint an application that is using uffd? >> Hard to believe, what if the monitor is running in a completely different >> process than the one being checkpointed? > >Shivansh, do you speak for CRIU? Are you able to comment on whether CRIU >supports checkpointing an app that uses uffd? I do not speak for CRIU - I'm just a user (and hopefully a future contributor), but not a maintainer or owner. I can however comment on whether CRIU supports checkpointing an app that uses UFFD - it doesn't. Looking through both the implementation of CRIU (specifically how they restore memory [1]), and at recently filed Github issues [2], it's pretty clear that CRIU doesn't support processes using UFFD - that they do not currently have plans to [3]. [1] https://github.com/checkpoint-restore/criu/blob/criu-2.x-stable/criu/mem.c#L683 [2] https://github.com/checkpoint-restore/criu/issues/2021 [3] https://github.com/checkpoint-restore/criu/issues/2021#issuecomment-1346971967 >> >> Further ... isn't CRIU already using uffd in some cases? ...documentation >> mentions [1] that it is used for "lazy (or post-copy) restore in CRIU". At least >> if the documentation is correct and its actually implemented. >> > >Shivansh, same question - do you know the current CRIU status/plans for using >uffd-wp instead of soft-dirty? If CRIU doesn't currently implement it and has no >current plans to, how can we guage interest in making a plan? > While I cannot gauge whether the maintainers or main contributors of CRIU plan on using uffd-wp instead of soft-dirty in the future, I can tell you that there is no currently open issue to track that work, and whenever anyone in the past has asked about ARM64 pre-dump support to CRIU (which is the feature that uses soft-dirty/would use uffd-wp), they've always just said it's not supported - but that they do want the feature [4]. So in summary, they want the feature, but no one is working on implementing it (either with soft-dirty or with uffd-wp). I doubt that CRIU would have any issues with adding the feature via soft-dirty (since, as shown in [4] they're interested in it), but as for using uffd-wp they definitely haven't shown any interest thus far. Based on the fact that it would be a very significant amount of work and it would really only be for ARM64 support (which they're already fine without), I'd be very surprised if they were interested in pursuing it. [4] https://github.com/checkpoint-restore/criu/issues/1859#issuecomment-1972674047 >> >>> >>>> But I'll throw in another idea: do we really need soft-dirty and uffd-wp to >>>> exist at the same time in the same process (or the VMA?). In theory, we > >My instinct is that MUXing a PTE bit like this will lead to some subtle problems >that won't appear on arches that support either one or both of the features >independently and unconditionally. Surely better to limit ourselves to either >"arm64 will only support uffd-wp" or "arm64 will support both uffd-wp and >soft-dirty". That way, we could move ahead with reviewing/merging the uffd-wp >support asynchronously to deciding whether we want to support soft-dirty. > My personal preference is having both approaches supported - especially in the context of CRIU since I doubt they'll be willing to rewrite all of the dumping and restore logic just for ARM64 support.
