[CRIU] [PATCH v3 4/8] memory: don't use parent memdump if detected possible pid reuse
Andrew Vagin
avagin at virtuozzo.com
Wed Feb 14 22:12:52 MSK 2018
On Wed, Feb 14, 2018 at 02:28:59PM +0300, Pavel Tikhomirov wrote:
> From: ptikhomirov <ptikhomirov at virtuozzo.com>
>
> We have a problem when a pid is reused between consequent dumps we can't
> understand if pagemap and pages from images of parent dump are invalid
> to restore these pid already. That can lead even to wrong memory
> restored for these pid, see the test in last patch.
>
> So these is a try do separate processes with (likely) invalid previous
> memory dump from processes with 100% valid previous dump.
>
> For that we use the value of /proc/<pid>/stat's start_time and also the
> timestamp of each (pre)dump. If the start time is strictly less than the
> timestamp, that means that the pagemap for these pid from previous dump
> is valid - was done for exactly the same process.
>
> Creation time is in centiseconds by default so if predump is really fast
> (<1csec) we can have false negative decisions for some processes, but in
> case of long running processes we are fine.
>
> https://jira.sw.ru/browse/PSBM-67502
>
> v2: remove __maybe_unused for get_parent_stats; fix get_parent_stats to
> have static typing; print warning only if unsure; check has_dump_uptime
> v3: read parent stats from image only once; reuse stat from previous
> parse_pid_stat call on dump
>
> Signed-off-by: Pavel Tikhomirov <ptikhomirov at virtuozzo.com>
> ---
> criu/cr-dump.c | 4 ++--
> criu/include/mem.h | 5 ++++-
> criu/mem.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++----
> criu/stats.c | 2 +-
> 4 files changed, 59 insertions(+), 8 deletions(-)
>
> diff --git a/criu/cr-dump.c b/criu/cr-dump.c
> index 9e8425504..d0cf922b7 100644
> --- a/criu/cr-dump.c
> +++ b/criu/cr-dump.c
> @@ -1290,7 +1290,7 @@ static int pre_dump_one_task(struct pstree_item *item)
> mdc.pre_dump = true;
> mdc.lazy = false;
>
> - ret = parasite_dump_pages_seized(item, &vmas, &mdc, parasite_ctl);
> + ret = parasite_dump_pages_seized(item, &vmas, &mdc, parasite_ctl, NULL);
> if (ret)
> goto err_cure;
>
> @@ -1449,7 +1449,7 @@ static int dump_one_task(struct pstree_item *item)
> mdc.pre_dump = false;
> mdc.lazy = opts.lazy_pages;
>
> - ret = parasite_dump_pages_seized(item, &vmas, &mdc, parasite_ctl);
> + ret = parasite_dump_pages_seized(item, &vmas, &mdc, parasite_ctl, &pps_buf);
> if (ret)
> goto err_cure;
>
> diff --git a/criu/include/mem.h b/criu/include/mem.h
> index bb897c599..aab8e183f 100644
> --- a/criu/include/mem.h
> +++ b/criu/include/mem.h
> @@ -4,6 +4,8 @@
> #include <stdbool.h>
> #include "int.h"
> #include "vma.pb-c.h"
> +#include "pid.h"
> +#include "proc_parse.h"
>
> struct parasite_ctl;
> struct vm_area_list;
> @@ -26,7 +28,8 @@ extern unsigned long dump_pages_args_size(struct vm_area_list *vmas);
> extern int parasite_dump_pages_seized(struct pstree_item *item,
> struct vm_area_list *vma_area_list,
> struct mem_dump_ctl *mdc,
> - struct parasite_ctl *ctl);
> + struct parasite_ctl *ctl,
> + struct proc_pid_stat* stat);
>
> #define PME_PRESENT (1ULL << 63)
> #define PME_SWAP (1ULL << 62)
> diff --git a/criu/mem.c b/criu/mem.c
> index 4c6942a11..a1646fd7d 100644
> --- a/criu/mem.c
> +++ b/criu/mem.c
> @@ -30,9 +30,11 @@
> #include "fault-injection.h"
> #include "prctl.h"
> #include <compel/compel.h>
> +#include "proc_parse.h"
>
> #include "protobuf.h"
> #include "images/pagemap.pb-c.h"
> +#include "images/stats.pb-c.h"
>
> static int task_reset_dirty_track(int pid)
> {
> @@ -294,7 +296,8 @@ static int __parasite_dump_pages_seized(struct pstree_item *item,
> struct parasite_dump_pages_args *args,
> struct vm_area_list *vma_area_list,
> struct mem_dump_ctl *mdc,
> - struct parasite_ctl *ctl)
> + struct parasite_ctl *ctl,
> + struct proc_pid_stat *stat)
> {
> pmc_t pmc = PMC_INIT;
> struct page_pipe *pp;
> @@ -303,6 +306,7 @@ static int __parasite_dump_pages_seized(struct pstree_item *item,
> int ret = -1;
> unsigned cpp_flags = 0;
> unsigned long pmc_size;
> + bool possible_pid_reuse = false;
>
> if (opts.check_only)
> return 0;
> @@ -360,6 +364,49 @@ static int __parasite_dump_pages_seized(struct pstree_item *item,
> xfer.parent = NULL + 1;
> }
>
> + if (xfer.parent) {
pls, move this code in a separate function.
__parasite_dump_pages_seized() is big already.
> + struct proc_pid_stat pps_buf;
> + static StatsEntry *stats;
> + unsigned long dump_ticks;
unsigned long long
> + unsigned long clock_ticks;
> +
> + clock_ticks = sysconf(_SC_CLK_TCK);
> + if (clock_ticks == -1) {
> + pr_perror("Failed to get clock ticks via sysconf");
> + goto out_xfer;
> + }
> +
> + if (stat) {
> + pps_buf = *stat;
> + } else {
> + ret = parse_pid_stat(item->pid->real, &pps_buf);
> + if (ret < 0)
> + goto out_xfer;
> + }
> +
> + if (!stats) {
> + stats = get_parent_stats();
> + if (!stats)
> + goto out_xfer;
> + }
> +
> + if (stats->dump->has_dump_uptime) {
> + dump_ticks = stats->dump->dump_uptime/(USEC_PER_SEC / clock_ticks);
> +
> + if (pps_buf.start_time >= dump_ticks) {
> + /* Print warning when we are not sure */
> + if (pps_buf.start_time == dump_ticks)
> + pr_warn("Will do full redump for pid=%d due " \
> + "to possible pid reuse\n",
> + item->pid->real);
> + possible_pid_reuse = true;
> + }
> + } else
> + pr_warn_once("Parent image has no dump timestamp, " \
> + "pid reuse detection OFF!\n");
> + }
> +
> +
> /*
> * Step 1 -- generate the pagemap
> */
> @@ -386,7 +433,7 @@ static int __parasite_dump_pages_seized(struct pstree_item *item,
> else {
> again:
> ret = generate_iovs(vma_area, pp, map, &off,
> - has_parent);
> + has_parent && !possible_pid_reuse);
> if (ret == -EAGAIN) {
> BUG_ON(!(pp->flags & PP_CHUNK_MODE));
>
> @@ -436,7 +483,8 @@ static int __parasite_dump_pages_seized(struct pstree_item *item,
> int parasite_dump_pages_seized(struct pstree_item *item,
> struct vm_area_list *vma_area_list,
> struct mem_dump_ctl *mdc,
> - struct parasite_ctl *ctl)
> + struct parasite_ctl *ctl,
> + struct proc_pid_stat* stat)
> {
> int ret;
> struct parasite_dump_pages_args *pargs;
> @@ -463,7 +511,7 @@ int parasite_dump_pages_seized(struct pstree_item *item,
> return -1;
> }
>
> - ret = __parasite_dump_pages_seized(item, pargs, vma_area_list, mdc, ctl);
> + ret = __parasite_dump_pages_seized(item, pargs, vma_area_list, mdc, ctl, stat);
> if (ret) {
> pr_err("Can't dump page with parasite\n");
> /* Parasite will unprotect VMAs after fail in fini() */
> diff --git a/criu/stats.c b/criu/stats.c
> index 4823e48de..4d646c104 100644
> --- a/criu/stats.c
> +++ b/criu/stats.c
> @@ -213,7 +213,7 @@ void write_stats(int what)
> display_stats(what, &stats);
> }
>
> -__maybe_unused StatsEntry *get_parent_stats(void)
> +StatsEntry *get_parent_stats(void)
> {
> struct cr_img *img;
> StatsEntry *se;
> --
> 2.14.3
>
More information about the CRIU
mailing list