[CRIU] [PATCH 02/11] lazy-pages: always compile uffd.c

[Adrian Reber]

On Wed, Apr 13, 2016 at 01:16:53PM +0300, Mike Rapoport wrote:
> On Wed, Apr 13, 2016 at 10:08:14AM +0200, Adrian Reber wrote:
> > Instead of adding ifdef's around the whole uffd.c content why didn't you
> > add your cr_lazy_pages()-without-UFFD implementation in the header file.
> > 
> > This would keep uffd.c free of ifdef's and should work just the way this
> > patch tries to implement it.
> 
> The #ifdef's in the .c file come instead ifeq ($(UFFD),1) in the Makefile.
> If the uffd.c is always compiled, it will produce errors on systems that do
> not have kernel headers recent enough.

Yes, but why is it necessary to always compile uffd.c? Is 'ifeq ($(UFFD),1)'
something negative which should be avoided?

		Adrian

> > On Mon, Apr 11, 2016 at 09:19:45AM +0300, Mike Rapoport wrote:
> > > If CONFIG_HAS_UFFD is not defined an attempt to run the lazy pages daemon
> > > will result in error message
> > > 
> > > Signed-off-by: Mike Rapoport <rppt at linux.vnet.ibm.com>
> > > ---
> > >  criu/Makefile.crtools  |  2 --
> > >  criu/crtools.c         |  8 --------
> > >  criu/include/crtools.h |  4 ----
> > >  criu/uffd.c            | 12 ++++++++++++
> > >  4 files changed, 12 insertions(+), 14 deletions(-)
> > > 
> > > diff --git a/criu/Makefile.crtools b/criu/Makefile.crtools
> > > index d610d2f..79fd77a 100644
> > > --- a/criu/Makefile.crtools
> > > +++ b/criu/Makefile.crtools
> > > @@ -82,9 +82,7 @@ obj-y			+= pie-util-vdso.o
> > >  obj-y			+= vdso.o
> > >  endif
> > >  
> > > -ifeq ($(UFFD),1)
> > >  obj-y	+= uffd.o
> > > -endif
> > >  
> > >  PROTOBUF_GEN := $(SRC_DIR)/scripts/protobuf-gen.sh
> > >  
> > > diff --git a/criu/crtools.c b/criu/crtools.c
> > > index d4c5731..93ca90b 100644
> > > --- a/criu/crtools.c
> > > +++ b/criu/crtools.c
> > > @@ -317,9 +317,7 @@ int main(int argc, char *argv[], char *envp[])
> > >  		{ "external",			required_argument,	0, 1073	},
> > >  		{ "empty-ns",			required_argument,	0, 1074	},
> > >  		{ "unshare",			required_argument,	0, 1075 },
> > > -#ifdef CONFIG_HAS_UFFD
> > >  		{ "lazy-pages",			no_argument,		0, 1076 },
> > > -#endif
> > >  		{ "extra",			no_argument,		0, 1077	},
> > >  		{ "experimental",		no_argument,		0, 1078	},
> > >  		{ "all",			no_argument,		0, 1079	},
> > > @@ -582,11 +580,9 @@ int main(int argc, char *argv[], char *envp[])
> > >  			if (parse_unshare_arg(optarg))
> > >  				return -1;
> > >  			break;
> > > -#ifdef CONFIG_HAS_UFFD
> > >  		case 1076:
> > >  			opts.lazy_pages = true;
> > >  			break;
> > > -#endif
> > >  		case 'M':
> > >  			{
> > >  				char *aux;
> > > @@ -801,9 +797,7 @@ usage:
> > >  "  criu page-server\n"
> > >  "  criu service [<options>]\n"
> > >  "  criu dedup\n"
> > > -#ifdef CONFIG_HAS_UFFD
> > >  "  criu lazy-pages -D DIR [<options>]\n"
> > > -#endif
> > >  "\n"
> > >  "Commands:\n"
> > >  "  dump           checkpoint a process/tree identified by pid\n"
> > > @@ -843,12 +837,10 @@ usage:
> > >  "  --unshare FLAGS       what namespaces to unshare when restoring\n"
> > >  "  --freeze-cgroup\n"
> > >  "                        use cgroup freezer to collect processes\n"
> > > -#ifdef CONFIG_HAS_UFFD
> > >  "  --lazy-pages          restore pages on demand\n"
> > >  "                        this requires running a second instance of criu\n"
> > >  "                        in lazy-pages mode: 'criu lazy-pages -D DIR'\n"
> > >  "                        --lazy-pages and lazy-pages mode require userfaultfd\n"
> > > -#endif
> > >  "\n"
> > >  "* Special resources support:\n"
> > >  "  -x|--" USK_EXT_PARAM "inode,.." "      allow external unix connections (optionally can be assign socket's inode that allows one-sided dump)\n"
> > > diff --git a/criu/include/crtools.h b/criu/include/crtools.h
> > > index e58b2be..00b7621 100644
> > > --- a/criu/include/crtools.h
> > > +++ b/criu/include/crtools.h
> > > @@ -26,11 +26,7 @@ extern int convert_to_elf(char *elf_path, int fd_core);
> > >  extern int cr_check(void);
> > >  extern int cr_exec(int pid, char **opts);
> > >  extern int cr_dedup(void);
> > > -#ifdef CONFIG_HAS_UFFD
> > >  extern int cr_lazy_pages(void);
> > > -#else
> > > -static inline int cr_lazy_pages() { return 0; };
> > > -#endif /* CONFIG_HAS_UFFD */
> > >  extern int prepare_task_entries(void);
> > >  
> > >  extern int check_add_feature(char *arg);
> > > diff --git a/criu/uffd.c b/criu/uffd.c
> > > index 3752a8c..c7aea0f 100644
> > > --- a/criu/uffd.c
> > > +++ b/criu/uffd.c
> > > @@ -28,6 +28,8 @@
> > >  #include "include/cr_options.h"
> > >  #include "xmalloc.h"
> > >  
> > > +#ifdef CONFIG_HAS_UFFD
> > > +
> > >  #undef  LOG_PREFIX
> > >  #define LOG_PREFIX "lazy-pages: "
> > >  
> > > @@ -660,3 +662,13 @@ int cr_lazy_pages()
> > >  
> > >  	return handle_requests(uffd);
> > >  }
> > > +
> > > +#else /* CONFIG_HAS_UFFD */
> > > +
> > > +int cr_lazy_pages()
> > > +{
> > > +	pr_msg("userfaultfd system call is not supported, cannot start lazy-pages daemon\n");
> > > +	return -1;
> > > +}
> > > +
> > > +#endif /* CONFIG_HAS_UFFD */
> > > -- 
> > > 1.9.1
> > > 
> > 
>