<div dir="ltr"><br>śr., 25 lip 2018 o 02:11 Andrei Vagin &lt;<a href="mailto:avagin@virtuozzo.com" target="_blank">avagin@virtuozzo.com</a>&gt; napisał(a):<br>&gt;<br>&gt; On Tue, Jul 17, 2018 at 03:38:57PM +0200, Pawel Stradomski wrote:<br>&gt; &gt; Punch holes in input files when restoring anonymous non-shared memory<br>&gt; &gt; if --auto-dedup is enabled.<br>&gt;<br>&gt; We need to update the description for --auto-dedup and describe this new<br>&gt; behaviour there.<br>&gt;<br>&gt; With these changes, criu restore can be executed only once. I think we<br>&gt; need to do something, so that a second attempt of restoring from these<br>&gt; images fails with a error.<br><br>For the record, this has already been the case. With --auto-dedup the image files were already being destroyed by criu,<div>but so far only for shared memory - see punch_hole() which calls fallocate(). </div><div>This change only makes it happen also for anonymous non-shared memory that is read by pie/restorer.c from</div><div>within the restored process and not main criu process.</div><div><div><div><br>&gt; We need to think how to test this functionality.<br>&gt;<br>&gt; &gt;<br>&gt; &gt; This reduces memory usage if image files are stored on tmpfs.<br>&gt; &gt;<br>&gt; &gt; Signed-off-by: Pawel Stradomski &lt;<a href="mailto:pstradomski@google.com" target="_blank">pstradomski@google.com</a>&gt;<br>&gt; &gt;<br>&gt; &gt; ---<br>&gt; &gt;  compel/arch/arm/plugins/std/syscalls/syscall.def |  1 +<br>&gt; &gt;  .../ppc64/plugins/std/syscalls/syscall-ppc64.tbl |  1 +<br>&gt; &gt;  .../s390/plugins/std/syscalls/syscall-s390.tbl   |  1 +<br>&gt; &gt;  .../arch/x86/plugins/std/syscalls/syscall_32.tbl |  1 +<br>&gt; &gt;  .../arch/x86/plugins/std/syscalls/syscall_64.tbl |  1 +<br>&gt; &gt;  criu/mem.c                                       |  3 ++-<br>&gt; &gt;  criu/pie/restorer.c                              | 16 ++++++++++++++++<br>&gt; &gt;  7 files changed, 23 insertions(+), 1 deletion(-)<br>&gt; &gt;<br>&gt; &gt; diff --git a/compel/arch/arm/plugins/std/syscalls/syscall.def b/compel/arch/arm/plugins/std/syscalls/syscall.def<br>&gt; &gt; index b68f9f2f..bcd61d4a 100644<br>&gt; &gt; --- a/compel/arch/arm/plugins/std/syscalls/syscall.def<br>&gt; &gt; +++ b/compel/arch/arm/plugins/std/syscalls/syscall.def<br>&gt; &gt; @@ -109,3 +109,4 @@ seccomp                           277     383     (unsigned int op, unsigned int flags, const char *uargs)<br>&gt; &gt;  gettimeofday                 169     78      (struct timeval *tv, struct timezone *tz)<br>&gt; &gt;  preadv_raw                   69      361     (int fd, struct iovec *iov, unsigned long nr, unsigned long pos_l, unsigned long pos_h)<br>&gt; &gt;  userfaultfd                  282     388     (int flags)<br>&gt; &gt; +fallocate                    47      352     (int fd, int mode, loff_t offset, loff_t len)<br>&gt; &gt; diff --git a/compel/arch/ppc64/plugins/std/syscalls/syscall-ppc64.tbl b/compel/arch/ppc64/plugins/std/syscalls/syscall-ppc64.tbl<br>&gt; &gt; index fa0b034e..62e0bc1a 100644<br>&gt; &gt; --- a/compel/arch/ppc64/plugins/std/syscalls/syscall-ppc64.tbl<br>&gt; &gt; +++ b/compel/arch/ppc64/plugins/std/syscalls/syscall-ppc64.tbl<br>&gt; &gt; @@ -89,6 +89,7 @@ __NR_set_robust_list        300             sys_set_robust_list     (struct robust_list_head *head, si<br>&gt; &gt;  __NR_get_robust_list 299             sys_get_robust_list     (int pid, struct robust_list_head **head_ptr, size_t *len_ptr)<br>&gt; &gt;  __NR_vmsplice                285             sys_vmsplice            (int fd, const struct iovec *iov, unsigned long nr_segs, unsigned int flags)<br>&gt; &gt;  __NR_openat          286             sys_openat              (int dfd, const char *filename, int flags, int mode)<br>&gt; &gt; +__NR_fallocate               309             sys_fallocate           (int fd, int mode, loff_t offset, loff_t len)<br>&gt; &gt;  __NR_timerfd_settime 311             sys_timerfd_settime     (int ufd, int flags, const struct itimerspec *utmr, struct itimerspec *otmr)<br>&gt; &gt;  __NR_signalfd4               313             sys_signalfd            (int fd, k_rtsigset_t *mask, size_t sizemask, int flags)<br>&gt; &gt;  __NR_rt_tgsigqueueinfo       322             sys_rt_tgsigqueueinfo   (pid_t tgid, pid_t pid, int sig, siginfo_t *info)<br>&gt; &gt; diff --git a/compel/arch/s390/plugins/std/syscalls/syscall-s390.tbl b/compel/arch/s390/plugins/std/syscalls/syscall-s390.tbl<br>&gt; &gt; index bc77ae97..3521e915 100644<br>&gt; &gt; --- a/compel/arch/s390/plugins/std/syscalls/syscall-s390.tbl<br>&gt; &gt; +++ b/compel/arch/s390/plugins/std/syscalls/syscall-s390.tbl<br>&gt; &gt; @@ -89,6 +89,7 @@ __NR_set_robust_list        304             sys_set_robust_list     (struct robust_list_head *head, si<br>&gt; &gt;  __NR_get_robust_list 305             sys_get_robust_list     (int pid, struct robust_list_head **head_ptr, size_t *len_ptr)<br>&gt; &gt;  __NR_vmsplice                309             sys_vmsplice            (int fd, const struct iovec *iov, unsigned long nr_segs, unsigned int flags)<br>&gt; &gt;  __NR_openat          288             sys_openat              (int dfd, const char *filename, int flags, int mode)<br>&gt; &gt; +__NR_fallocate               314             sys_fallocate           (int fd, int mode, loff_t offset, loff_t len)<br>&gt; &gt;  __NR_timerfd_settime 320             sys_timerfd_settime     (int ufd, int flags, const struct itimerspec *utmr, struct itimerspec *otmr)<br>&gt; &gt;  __NR_signalfd4               322             sys_signalfd            (int fd, k_rtsigset_t *mask, size_t sizemask, int flags)<br>&gt; &gt;  __NR_rt_tgsigqueueinfo       330             sys_rt_tgsigqueueinfo   (pid_t tgid, pid_t pid, int sig, siginfo_t *info)<br>&gt; &gt; diff --git a/compel/arch/x86/plugins/std/syscalls/syscall_32.tbl b/compel/arch/x86/plugins/std/syscalls/syscall_32.tbl<br>&gt; &gt; index 9e1de281..a6c55b83 100644<br>&gt; &gt; --- a/compel/arch/x86/plugins/std/syscalls/syscall_32.tbl<br>&gt; &gt; +++ b/compel/arch/x86/plugins/std/syscalls/syscall_32.tbl<br>&gt; &gt; @@ -83,6 +83,7 @@ __NR_set_robust_list        311             sys_set_robust_list     (struct robust_list_head *head, si<br>&gt; &gt;  __NR_get_robust_list 312             sys_get_robust_list     (int pid, struct robust_list_head **head_ptr, size_t *len_ptr)<br>&gt; &gt;  __NR_vmsplice                316             sys_vmsplice            (int fd, const struct iovec *iov, unsigned int nr_segs, unsigned int flags)<br>&gt; &gt;  __NR_signalfd                321             sys_signalfd            (int ufd, const k_rtsigset_t *sigmask, size_t sigsetsize)<br>&gt; &gt; +__NR_fallocate               324             sys_fallocate           (int fd, int mode, loff_t offset, loff_t len)<br>&gt; &gt;  __NR_timerfd_settime 325             sys_timerfd_settime     (int ufd, int flags, const struct itimerspec *utmr, struct itimerspec *otmr)<br>&gt; &gt;  __NR_preadv          333             sys_preadv_raw          (int fd, struct iovec *iov, unsigned long nr, unsigned long pos_l, unsigned long pos_h)<br>&gt; &gt;  __NR_rt_tgsigqueueinfo       335             sys_rt_tgsigqueueinfo   (pid_t tgid, pid_t pid, int sig, siginfo_t *uinfo)<br>&gt; &gt; diff --git a/compel/arch/x86/plugins/std/syscalls/syscall_64.tbl b/compel/arch/x86/plugins/std/syscalls/syscall_64.tbl<br>&gt; &gt; index 726fa797..64271514 100644<br>&gt; &gt; --- a/compel/arch/x86/plugins/std/syscalls/syscall_64.tbl<br>&gt; &gt; +++ b/compel/arch/x86/plugins/std/syscalls/syscall_64.tbl<br>&gt; &gt; @@ -94,6 +94,7 @@ __NR_set_robust_list                273             sys_set_robust_list     (struct robust_list_head *head, s<br>&gt; &gt;  __NR_get_robust_list         274             sys_get_robust_list     (int pid, struct robust_list_head **head_ptr, size_t *len_ptr)<br>&gt; &gt;  __NR_seccomp                 317             sys_seccomp             (unsigned int op, unsigned int flags, const char *uargs)<br>&gt; &gt;  __NR_vmsplice                        278             sys_vmsplice            (int fd, const struct iovec *iov, unsigned long nr_segs, unsigned int flags)<br>&gt; &gt; +__NR_fallocate                       285             sys_fallocate           (int fd, int mode, loff_t offset, loff_t len)<br>&gt; &gt;  __NR_timerfd_settime         286             sys_timerfd_settime     (int ufd, int flags, const struct itimerspec *utmr, struct itimerspec *otmr)<br>&gt; &gt;  __NR_signalfd4                       289             sys_signalfd            (int fd, k_rtsigset_t *mask, size_t sizemask, int flags)<br>&gt; &gt;  __NR_preadv                  295             sys_preadv_raw          (int fd, struct iovec *iov, unsigned long nr, unsigned long pos_l, unsigned long pos_h)<br>&gt; &gt; diff --git a/criu/mem.c b/criu/mem.c<br>&gt; &gt; index d020b7fd..c3d604a5 100644<br>&gt; &gt; --- a/criu/mem.c<br>&gt; &gt; +++ b/criu/mem.c<br>&gt; &gt; @@ -1271,7 +1271,8 @@ static int prepare_vma_ios(struct pstree_item *t, struct task_restore_args *ta)<br>&gt; &gt;  {<br>&gt; &gt;       struct cr_img *pages;<br>&gt; &gt; <br>&gt; &gt; -     pages = open_image(CR_FD_PAGES, O_RSTR, rsti(t)-&gt;pages_img_id);<br>&gt; &gt; +     pages = open_image(CR_FD_PAGES, opts.auto_dedup ? O_RDWR : O_RSTR,<br>&gt; &gt; +                             rsti(t)-&gt;pages_img_id);<br>&gt;<br>&gt; Could you add a comment here which explain why O_RDWR is required?</div><div><br></div><div>Done.</div><div><br>&gt;<br>&gt; &gt;       if (!pages)<br>&gt; &gt;               return -1;<br>&gt; &gt; <br>&gt; &gt; diff --git a/criu/pie/restorer.c b/criu/pie/restorer.c<br>&gt; &gt; index f990e9b7..3f1a8a6b 100644<br>&gt; &gt; --- a/criu/pie/restorer.c<br>&gt; &gt; +++ b/criu/pie/restorer.c<br>&gt; &gt; @@ -646,6 +646,15 @@ static unsigned long restore_mapping(VmaEntry *vma_entry)<br>&gt; &gt;                       !(vma_entry-&gt;status &amp; VMA_NO_PROT_WRITE))<br>&gt; &gt;               prot |= PROT_WRITE;<br>&gt; &gt; <br>&gt; &gt; +     /* TODO: if the mapping had MAP_LOCKED bit set, then the mmap will<br>&gt; &gt; +      * cause immediate page-in and increase in process memory usage,<br>&gt; &gt; +      * thus defeating attempts to conserve memory by running fallocate after<br>&gt; &gt; +      * each preadv.<br>&gt; &gt; +      *<br>&gt; &gt; +      * This could be fixed by zeroing MAP_LOCKED bit here and restoring it<br>&gt; &gt; +      * after all the contents is already loaded and the tmpfs files released<br>&gt; &gt; +      * by fallocate.<br>&gt;<br>&gt; Unfortunately, I don&#39;t understand  this comment. Maybe you can elaborate<br>&gt; with more details.</div><div><br></div><div>Done</div><div><br>&gt;<br>&gt; &gt; +      */<br>&gt; &gt;       pr_debug(&quot;\tmmap(%&quot;PRIx64&quot; -&gt; %&quot;PRIx64&quot;, %x %x %d)\n&quot;,<br>&gt; &gt;                       vma_entry-&gt;start, vma_entry-&gt;end,<br>&gt; &gt;                       prot, flags, (int)vma_entry-&gt;fd);<br>&gt;<br>&gt; There is one more place where a private memory is restored. It is in<br>&gt; restore_priv_vma_content(). Do we want to do the same for shared memory?</div><div><br></div><div><span style="font-size:small;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">restore_priv_vma_content either:</span><br></div><div><span style="font-size:small;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">a) uses COW code where punching holes would be tricky</span></div><div><span style="text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><span style="text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><span style="background-color:rgb(255,255,255)">b) or calls pr-&gt;read_pages which ends up (via </span>maybe_read_page) calling </span></span>read_local_page which already has</div><div>if (opts.auto_dedup) {</div><div>      ret = punch_hole(pr, pr-&gt;pi_off, len, false);</div><div>}</div><div>c) or ends up doing async reads which call fallocate() in process_async_reads()</div><div>d) or uses render_pagemap to defer actual reading to pie/restorer.c, which is the path I&#39;m fixing here.</div><div><br></div><div>&gt;<br>&gt; &gt; @@ -1367,6 +1376,13 @@ long __export_restore_task(struct task_restore_args *args)<br>&gt; &gt;                       }<br>&gt; &gt; <br>&gt; &gt;                       pr_debug(&quot;`- returned %ld\n&quot;, (long)r);<br>&gt; &gt; +                     /* TODO: Check if auto-dedup is enabled instead of trusting fallocate to fail<br>&gt; &gt; +                      * if the file is not opened for writing. */<br>&gt; &gt; +                     if (r &gt; 0) {<br>&gt; &gt; +                             pr_debug(&quot;   `fallocate %d %ld %ld\n&quot;, args-&gt;vma_ios_fd,  rio-&gt;off, r);<br>&gt; &gt; +                             sys_fallocate(args-&gt;vma_ios_fd, FALLOC_FL_KEEP_SIZE|FALLOC_FL_PUNCH_HOLE,<br>&gt; &gt; +                                     rio-&gt;off, r);<br>&gt;<br>&gt; We have to check a return code and print an error.<br><br>Done.<br><br>&gt; &gt; +                     }<br>&gt; &gt;                       rio-&gt;off += r;<br>&gt; &gt;                       /* Advance the iovecs */<br>&gt; &gt;                       do {<br>&gt; &gt; --<br>&gt; &gt; 2.18.0.203.gfac676dfb9-goog<br>&gt; &gt;<br>&gt; &gt; _______________________________________________<br>&gt; &gt; CRIU mailing list<br>&gt; &gt; <a href="mailto:CRIU@openvz.org" target="_blank">CRIU@openvz.org</a><br>&gt; &gt; <a href="https://lists.openvz.org/mailman/listinfo/criu" target="_blank">https://lists.openvz.org/mailman/listinfo/criu</a><br><br><br><br>-- <br>Paweł Stradomski</div></div></div></div>