[CRIU] [PATCH v5 3/5] Try to include userfaultfd with criu (part 1)

[Dmitry Safonov]

On 03/10/2016 10:39 AM, Adrian Reber wrote:
> From: Adrian Reber <areber at redhat.com>
>
> This is a first try to include userfaultfd with criu. Right now it
> still requires a "normal" checkpoint. After checkpointing the
> application it can be restored with the help of userfaultfd.
>
> All restored pages with MAP_ANONYMOUS and MAP_PRIVATE set are marked as
> being handled by userfaultfd.
>
> As soon as the process is restored it blocks on the first memory access
> and waits for pages being transferred by userfaultfd.
>
> To handle the required pages a new criu command has been added. For a
> userfaultfd supported restore the first step is to start the
> 'lazy-pages' server:
>
>    criu lazy-pages -v4 -D /tmp/3/ --address /tmp/userfault.socket
>
> This is part 1 of the userfaultfd integration which provides the
> 'lazy-pages' server implementation.
>
> v2:
>      * provide option '--lazy-pages' to enable uffd style restore
>      * use send_fd()/recv_fd() provided by criu (instead of own
>        implementation)
>      * do not install the uffd as service_fd
>      * use named constants for MAP_ANONYMOUS
>      * do not restore memory pages and then later mark them as uffd
>        handled
>      * remove function find_pages() to search in pages-<id>.img;
>        now using criu functions to find the necessary pages;
>        for each new page search the pages-<id>.img file is opened
>      * only check the UFFDIO_API once
>      * trying to protect uffd code by CONFIG_UFFD;
>        use make UFFD=1 to compile criu with this patch
>
> v3:
>     * renamed the server mode from 'uffd' -> 'lazy-pages'
>     * switched client and server roles transferring the UFFD FD
>       * the criu part running in lazy-pages server mode is now
>         waiting for connections
>       * the criu restore process connects to the lazy-pages server
>         to pass the UFFD FD
>     * before UFFD copying anything else the VDSO pages are copied
>       as it fails to copy unused VDSO pages once the process is running.
>       this was necessary to be able to copy all pages.
>     * if there are no more UFFD messages for 5 seconds the lazy-pages
>       server switches in copy mode to copy all remaining pages, which
>       have not been requested yet, into the restored process
>     * check the UFFDIO_API at the correct place
>     * close UFFD FD in the restorer to remove open UFFD FD in the
>       restored process
>
> v4:
>      * removed unnecessary madvise() calls ; it seemed necessary when
>        first running tests with uffd; it actually is not necessary
>      * auto-detect if build-system provides linux/userfaultfd.h
>        header
>      * simplify unix domain socket setup and communication.
>      * use --address to specify the location of the used
>        unix domain socket
>
> v5:
>      * split the userfaultfd patch in multiple smaller patches
>      * introduced vma_can_be_lazy() function to check if a page
>        can be handled by uffd
>      * moved uffd related code from cr-restore.c to uffd.c
>      * handle failure to register a memory page of the restored process
>        with userfaultfd
Hello, Adrian,
just a small nip from me below:
>
> Signed-off-by: Adrian Reber <areber at redhat.com>
> ---
>   criu/Makefile.config      |   6 +-
>   criu/Makefile.crtools     |   4 +
>   criu/crtools.c            |   6 +
>   criu/include/crtools.h    |   5 +
>   criu/include/uffd.h       |  16 ++
>   criu/uffd.c               | 437 ++++++++++++++++++++++++++++++++++++++++++++++
>   scripts/feature-tests.mak |  15 ++
>   7 files changed, 488 insertions(+), 1 deletion(-)
>   create mode 100644 criu/include/uffd.h
>   create mode 100644 criu/uffd.c
>
> diff --git a/criu/Makefile.config b/criu/Makefile.config
> index aaaca1f..c3841b9 100644
> --- a/criu/Makefile.config
> +++ b/criu/Makefile.config
> @@ -14,8 +14,12 @@ ifeq ($(call pkg-config-check,libselinux),y)
>           DEFINES	+= -DCONFIG_HAS_SELINUX
>   endif
>   
> +ifeq ($(call try-cc,$(FEATURE_TEST_UFFD)),y)
> +	export UFFD := 1
> +endif
> +
Here we try-cc feature test,
>   FEATURES_LIST	:= TCP_REPAIR PRLIMIT STRLCPY STRLCAT PTRACE_PEEKSIGINFO \
> -	SETPROCTITLE_INIT MEMFD
> +	SETPROCTITLE_INIT MEMFD UFFD
and adding it to FEATURES_LIST will compile feature test for CONFIG_HAS_*
define the second time, which will slowdown makefile processing.
As we know the result of first try-cc, we may use it (completely untested):

diff --git a/criu/Makefile.config b/criu/Makefile.config
index c3841b9..56076b6 100644
--- a/criu/Makefile.config
+++ b/criu/Makefile.config
@@ -19,7 +19,7 @@ ifeq ($(call try-cc,$(FEATURE_TEST_UFFD)),y)
  endif

  FEATURES_LIST  := TCP_REPAIR PRLIMIT STRLCPY STRLCAT PTRACE_PEEKSIGINFO \
-       SETPROCTITLE_INIT MEMFD UFFD
+       SETPROCTITLE_INIT MEMFD

  # $1 - config name
  define gen-feature-test
@@ -38,6 +38,10 @@ $(CONFIG_HEADER): include/config-base.h
         $(Q) @echo '#include "config-base.h"'                           
 >> $$@
         $(Q) @echo ''                                                   
 >> $$@
  $(call map,gen-feature-test,$(FEATURES_LIST))
+ifeq ($$(UFFD),1)
+       $(Q) @echo '#define CONFIG_HAS_UFFD'                            
 >> $$@
+       $(Q) @echo ''                                                   
 >> $$@
+endif
  ifeq ($$(VDSO),y)
         $(Q) @echo '#define CONFIG_VDSO'                                
 >> $$@
         $(Q) @echo ''                                                   
 >> $$@

Through, slowdown will not be great (exec+cc a small program) - you
may drop this and I'm fine.
Maybe if we'll have more feature test that need Makefile define exports,
we'll make some standard for makefile variables like $(UFFD) and
clean this, but yet, I suggest adding this diff.

Thanks,
Dmitry