[CRIU] [PATCH RFC] criu: add a debug version of rst-malloc

[Dmitry Safonov]

2017-03-30 8:30 GMT+03:00 Andrei Vagin <avagin at openvz.org>:
> From: Andrei Vagin <avagin at virtuozzo.com>
>
> rst_mem_alloc() can move all allocated objects and
> the problem is that we don't know when it will do.
>
> With this patch, if the criu is built with DEBUG=1,
> rst_mem_alloc() will move objects each time.
>
> Signed-off-by: Andrei Vagin <avagin at virtuozzo.com>
> ---
>  .travis.yml                 |  1 +
>  criu/rst-malloc.c           | 19 +++++++++++++++++++
>  scripts/travis/travis-tests |  2 +-
>  3 files changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/.travis.yml b/.travis.yml
> index a5dfb9f..d62e086 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -7,6 +7,7 @@ services:
>  env:
>    - TR_ARCH=local       GCOV=1
>    - TR_ARCH=local       CLANG=1
> +  - TR_ARCH=local       CRIU_DEBUG=1
>    - TR_ARCH=alpine
>    - TR_ARCH=x86_64
>    - TR_ARCH=armv7hf
> diff --git a/criu/rst-malloc.c b/criu/rst-malloc.c
> index 4f7357c..057fa31 100644
> --- a/criu/rst-malloc.c
> +++ b/criu/rst-malloc.c
> @@ -23,6 +23,11 @@ static inline unsigned long rst_mem_grow(unsigned long need_size)
>  {
>         int rst_mem_batch = 2 * page_size();
>
> +#ifdef CR_DEBUG
> +       if (need_size == 0)
> +               return 0;
> +#endif
> +
>         need_size = round_up(need_size, page_size());
>         if (likely(need_size < rst_mem_batch))
>                 need_size = rst_mem_batch;
> @@ -88,8 +93,18 @@ static int grow_remap(struct rst_mem_type_s *t, int flag, unsigned long size)
>                  * a completely new one and force callers use objects'
>                  * cpos-s.
>                  */
> +#ifdef CR_DEBUG

Could we name it somehow more descriptive?
So we wouldn't introduce large CR_DEBUG for everything,
and say DEBUG_RST_ALLOC?
And just add it in makefile here:

ifeq ($(DEBUG),1)
        DEFINES        += -DCR_DEBUG -DDEBUG_RST_ALLOC

Or maybe you could come with a better name.

Maybe even some day we could have it as a build option
in .config along with some other.

> +               /* Move vma each time to another palce */
> +               void *ptr = mmap(NULL, t->size + size, PROT_NONE, MAP_ANONYMOUS | MAP_PRIVATE, 0, 0);
> +               if (ptr == MAP_FAILED)
> +                       return -1;
> +               aux = mremap(t->buf, t->size,
> +                               t->size + size, MREMAP_MAYMOVE | MREMAP_FIXED, ptr);
> +               munmap(t->buf, t->size);
> +#else
>                 aux = mremap(t->buf, t->size,
>                                 t->size + size, MREMAP_MAYMOVE);
> +#endif
>         }
>         if (aux == MAP_FAILED)
>                 return -1;
> @@ -175,6 +190,10 @@ void *rst_mem_alloc(unsigned long size, int type)
>                 pr_perror("Can't grow rst mem");
>                 return NULL;
>         }
> +#ifdef CR_DEBUG
> +       else if (type == RM_PRIVATE && t->grow(t, 0)) /* Move a vma to another place */
> +               return NULL;
> +#endif
>
>         ret = t->free_mem;
>         t->free_mem += size;
> diff --git a/scripts/travis/travis-tests b/scripts/travis/travis-tests
> index 37117a3..3897542 100755
> --- a/scripts/travis/travis-tests
> +++ b/scripts/travis/travis-tests
> @@ -41,7 +41,7 @@ ulimit -c unlimited
>  echo "|`pwd`/test/abrt.sh %P %p %s %e" > /proc/sys/kernel/core_pattern
>
>  export GCOV
> -time make CC="$CC" -j4
> +time make CC="$CC" DEBUG="$CRIU_DEBUG" -j4
>  time make CC="$CC" -j4 -C test/zdtm
>
>  [ -f "$CCACHE_LOGFILE" ] && cat $CCACHE_LOGFILE
> --
> 2.7.4
>
> _______________________________________________
> CRIU mailing list
> CRIU at openvz.org
> https://lists.openvz.org/mailman/listinfo/criu



-- 
             Dmitry