[CRIU] GSoC: Memory mapping collection
Abhishek Dubey
dubeyabhishek777 at gmail.com
Tue Jun 11 21:22:42 MSK 2019
Summary of test/zdtm.py run -a --ignore-taint --keep-going is
################## ALL TEST(S) PASSED (TOTAL 204/SKIPPED 21)
###################
Sending complete console output separately.
On Mon, Jun 10, 2019 at 10:50 PM Andrei Vagin <avagin at gmail.com> wrote:
> On Mon, Jun 10, 2019 at 2:50 AM abhishek dubey
> <dubeyabhishek777 at gmail.com> wrote:
> >
> >
> > On 10/06/19 12:59 PM, Andrei Vagin (C) wrote:
> > > On Wed, Jun 05, 2019 at 12:49:03PM +0000, Pavel Emelianov wrote:
> > >> On 6/4/19 7:50 PM, Abhishek Dubey wrote:
> > >>> Hi,
> > >>>
> > >>> On running "zdtm.py run -a", testcases of zdtm/static directory are
> checked and that of zdtm/transition are skipped. Do all testcases of
> transition directory need manual run?
> > >> Yes.
> > > I don't understand what you mean here.
> > >
> > >
> https://ci.openvz.org/job/CRIU/job/CRIU-x86_64/job/criu-dev/5439/console
> > >
> > > Here we can see that all trasition tests were checked...
> > Here I refer to locally running zdtm tests as given on
> > https://www.criu.org/ZDTM_test_suite
>
> Could you show output for zdtm.py run -a?
>
> > >
> > >>> What is specific about transition testcases?
> > >> They take time to accomplish, so the default run is limited.
> > >>
> > >>> For the first step of fetching VMAs between freeze and unfreeze of
> pre-dump:
> > >>> - To maintain correspondence between PID and collected VMA, added
> new entry in struct pstree_item
> > >>> - Moved out unfreeze code from cr_pre_dump_finish()
> > >>> - Moved out collect_mappings() from pre_dump_one_task
> > >>>
> > >>> I have tested these changes with env00-basic example and zdtm.py
> test-suite. All test-cases under zdtm/static directory are Passing.
> Remaining transition test cases are skipped.
> > >>>
> > >>> Share your thoughts on this approach. I am attaching patch-file for
> reference.
> > >>> Hi,
> > >>>
> > >>> On running "zdtm.py run -a", testcases of zdtm/static directory are
> checked and that of zdtm/transition are skipped. Do all testcases of
> transition directory need manual run?
> > >>> What is specific about transition testcases?
> > >>>
> > >>> For the first step of fetching VMAs between freeze and unfreeze of
> pre-dump:
> > >>> - To maintain correspondence between PID and collected VMA, added
> new entry in struct pstree_item
> > >>> - Moved out unfreeze code from cr_pre_dump_finish()
> > >>> - Moved out collect_mappings() from pre_dump_one_task
> > >> Please, describe why you need this. Current code does "fetching vmas
> between freeze and unfreeze",
> > >> what's wrong with it so you need this change?
> > >>
> > >>> I have tested these changes with env00-basic example and zdtm.py
> test-suite. All test-cases under zdtm/static directory are Passing.
> Remaining transition test cases are skipped.
> > >>>
> > >>> Share your thoughts on this approach. I am attaching patch-file for
> reference.
> > >>>
> > >>> -Abhishek
> > >>>
> > >>> 0001-freezing-target-task-to-collect-memory-mappings-foll.patch
> > >>>
> > >>> From 5b131559ce20ebbdb8a04be2cced321b55d864f4 Mon Sep 17 00:00:00
> 2001
> > >> Please mind the https://criu.org/How_to_submit_patches article.
> Patch should go inline, not attached,
> > >> and properly formatted. Also comments are inline.
> > >>
> > >>> From: abhidubey4178 <abhishekdubey4178940 at gmail.com>
> > >>> Date: Tue, 4 Jun 2019 19:59:21 +0530
> > >>> Subject: [PATCH] freezing target task to collect memory mappings,
> followed by
> > >>> unfreeze
> > >>>
> > >>> ---
> > >>> criu/cr-dump.c | 103
> +++++++++++++++++++++++++++++++-----------
> > >>> criu/include/pstree.h | 1 +
> > >>> 2 files changed, 78 insertions(+), 26 deletions(-)
> > >>>
> > >>> diff --git a/criu/cr-dump.c b/criu/cr-dump.c
> > >>> index 7f2e5edf..40bc7594 100644
> > >>> --- a/criu/cr-dump.c
> > >>> +++ b/criu/cr-dump.c
> > >>> @@ -1154,20 +1154,19 @@ static int pre_dump_one_task(struct
> pstree_item *item, InventoryEntry *parent_ie
> > >>> if (item->pid->state == TASK_DEAD)
> > >>> return 0;
> > >>>
> > >>> - ret = collect_mappings(pid, &vmas, NULL);
> > >> Up above there have been left the now unused vmas variable.
> > >>
> > >>> - if (ret) {
> > >>> + if (!item->vmas) {
> > >>> pr_err("Collect mappings (pid: %d) failed with %d\n",
> pid, ret);
> > >>> goto err;
> > >>> }
> > >>>
> > >>> ret = -1;
> > >>> - parasite_ctl = parasite_infect_seized(pid, item, &vmas);
> > >>> + parasite_ctl = parasite_infect_seized(pid, item, item->vmas);
> > >>> if (!parasite_ctl) {
> > >>> pr_err("Can't infect (pid: %d) with parasite\n", pid);
> > >>> goto err_free;
> > >>> }
> > >>>
> > >>> - ret = parasite_fixup_vdso(parasite_ctl, pid, &vmas);
> > >>> + ret = parasite_fixup_vdso(parasite_ctl, pid, item->vmas);
> > >>> if (ret) {
> > >>> pr_err("Can't fixup vdso VMAs (pid: %d)\n", pid);
> > >>> goto err_cure;
> > >>> @@ -1192,14 +1191,14 @@ static int pre_dump_one_task(struct
> pstree_item *item, InventoryEntry *parent_ie
> > >>> mdc.stat = NULL;
> > >>> mdc.parent_ie = parent_ie;
> > >>>
> > >>> - ret = parasite_dump_pages_seized(item, &vmas, &mdc,
> parasite_ctl);
> > >>> + ret = parasite_dump_pages_seized(item, item->vmas, &mdc,
> parasite_ctl);
> > >>> if (ret)
> > >>> goto err_cure;
> > >>>
> > >>> if (compel_cure_remote(parasite_ctl))
> > >>> pr_err("Can't cure (pid: %d) from parasite\n", pid);
> > >>> err_free:
> > >>> - free_mappings(&vmas);
> > >>> + free_mappings(item->vmas);
> > >>> err:
> > >>> return ret;
> > >>>
> > >>> @@ -1471,26 +1470,9 @@ static int setup_alarm_handler()
> > >>>
> > >>> static int cr_pre_dump_finish(int status)
> > >>> {
> > >>> - InventoryEntry he = INVENTORY_ENTRY__INIT;
> > >>> struct pstree_item *item;
> > >>> int ret;
> > >>>
> > >>> - /*
> > >>> - * Restore registers for tasks only. The threads have not been
> > >>> - * infected. Therefore, the thread register sets have not been
> changed.
> > >>> - */
> > >>> - ret = arch_set_thread_regs(root_item, false);
> > >>> - if (ret)
> > >>> - goto err;
> > >>> -
> > >>> - ret = inventory_save_uptime(&he);
> > >>> - if (ret)
> > >>> - goto err;
> > >>> -
> > >>> - pstree_switch_state(root_item, TASK_ALIVE);
> > >>> -
> > >>> - timing_stop(TIME_FROZEN);
> > >>> -
> > >>> if (status < 0) {
> > >>> ret = status;
> > >>> goto err;
> > >>> @@ -1540,9 +1522,6 @@ err:
> > >>> if (bfd_flush_images())
> > >>> ret = -1;
> > >>>
> > >>> - if (write_img_inventory(&he))
> > >>> - ret = -1;
> > >>> -
> > >> You write inventory too early, I'm not 100% sure this is correct.
> > >>
> > >>> if (ret)
> > >>> pr_err("Pre-dumping FAILED.\n");
> > >>> else {
> > >>> @@ -1552,6 +1531,68 @@ err:
> > >>> return ret;
> > >>> }
> > >>>
> > >>> +
> > >>> +static int pre_dump_collect_vmas(struct pstree_item *item)
> > >>> +{
> > >>> + pid_t pid = item->pid->real;
> > >>> + struct vm_area_list *vmas;
> > >>> + int ret = -1;
> > >>> +
> > >>> + vmas = xmalloc(sizeof(struct vm_area_list));
> > >> This memory is never free-d.
> > >>
> > >>> + if(!vmas)
> > >>> + return -1;
> > >>> + INIT_LIST_HEAD(&vmas->h);
> > >>> + vmas->nr = 0;
> > >>> +
> > >>> + pr_info("========================================\n");
> > >>> + pr_info("Collecting VMAs of (pid: %d)\n", pid);
> > >>> + pr_info("========================================\n");
> > >>> +
> > >>> + if (item->pid->state == TASK_STOPPED) {
> > >>> + pr_warn("Stopped tasks are not supported\n");
> > >>> + return 0;
> > >>> + }
> > >> Formatting.
> > >>
> > >>> +
> > >>> + if (item->pid->state == TASK_DEAD)
> > >>> + return 0;
> > >> Vmas allocation can happen here.
> > >>
> > >>> +
> > >>> + ret = collect_mappings(pid, vmas, NULL);
> > >>> + if (ret) {
> > >>> + pr_err("Collect mappings (pid: %d) failed with
> %d\n", pid, ret);
> > >>> + return -1;
> > >>> + }
> > >>> +
> > >>> + item->vmas = vmas;
> > >>> + return 0;
> > >>> +}
> > >>> +
> > >>> +
> > >>> +static int unfreeze_pstree()
> > >>> +{
> > >>> + InventoryEntry he = INVENTORY_ENTRY__INIT;
> > >>> + int ret;
> > >>> +
> > >>> + /*
> > >>> + * Restore registers for tasks only. The threads have not been
> > >>> + * infected. Therefore, the thread register sets have not been
> changed.
> > >>> + */
> > >>> + ret = arch_set_thread_regs(root_item, false);
> > >>> + if (ret)
> > >>> + return -1;
> > >>> +
> > >>> + ret = inventory_save_uptime(&he);
> > >>> + if (ret)
> > >>> + return -1;
> > >>> +
> > >>> + if (write_img_inventory(&he))
> > >>> + return -1;
> > >>> +
> > >>> + pstree_switch_state(root_item, TASK_ALIVE);
> > >>> + timing_stop(TIME_FROZEN);
> > >>> +
> > >>> + return 0;
> > >>> +}
> > >>> +
> > >>> int cr_pre_dump_tasks(pid_t pid)
> > >>> {
> > >>> InventoryEntry *parent_ie = NULL;
> > >>> @@ -1619,6 +1660,16 @@ int cr_pre_dump_tasks(pid_t pid)
> > >>> /* Errors handled later in detect_pid_reuse */
> > >>> parent_ie = get_parent_inventory();
> > >>>
> > >>> + /* Collecting VMAs of all processes in pstree */
> > >>> + for_each_pstree_item(item)
> > >>> + if (pre_dump_collect_vmas(item))
> > >>> + goto err;
> > >>> +
> > >>> + /* Unfreeze pstree */
> > >> This is pointless comment as it 100% repeats the subsequent function
> name :)
> > >>
> > >>> + ret = unfreeze_pstree();
> > >>> + if(ret)
> > >>> + goto err;
> > >>> +
> > >>> for_each_pstree_item(item)
> > >>> if (pre_dump_one_task(item, parent_ie))
> > >>> goto err;
> > >>> diff --git a/criu/include/pstree.h b/criu/include/pstree.h
> > >>> index 7303c1fe..69562608 100644
> > >>> --- a/criu/include/pstree.h
> > >>> +++ b/criu/include/pstree.h
> > >>> @@ -30,6 +30,7 @@ struct pstree_item {
> > >>> futex_t task_st;
> > >>> unsigned long task_st_le_bits;
> > >>> };
> > >>> + struct vm_area_list *vmas;
> > >> This is dump-only field, it should be placed on dmp_info instead.
> > >>
> > >>> };
> > >>>
> > >>> static inline pid_t vpid(const struct pstree_item *i)
> > >>> --
> > >>> 2.17.1
> > >>>
> > >> -- Pavel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openvz.org/pipermail/criu/attachments/20190611/2d1220b7/attachment-0001.html>
More information about the CRIU
mailing list