[CRIU] [PATCH] deduplication: bunch neighbour data to punch together

Pavel Emelyanov xemul at parallels.com
Fri Mar 7 00:38:05 PST 2014


On 03/07/2014 12:24 PM, Pavel Tikhomirov wrote:
> 2014-03-04 9:30 GMT+04:00 Pavel Tikhomirov <snorcht at gmail.com <mailto:snorcht at gmail.com>>:
> 
>     2014-03-03 17:51 GMT+04:00 Tikhomirov Pavel <snorcht at gmail.com <mailto:snorcht at gmail.com>>:
> 
>         when decide that data is no longer needed, there are two cases:
>         1. if data neighbours previous block of "no needed" data, extend bunch
>         block(it holds begining and size of concequent "no needed" data) by
>         length of curent block and go next.
>         2. if data not neighbours bunch block(or bunch block size is bigger
>         than MAX_BUNCH_SIZE), than punch bunch block and set bunch block
>         to curent block.
> 
>         in the end make cleanup to punch last bunch block.
> 
>         Signed-off-by: Tikhomirov Pavel <snorcht at gmail.com <mailto:snorcht at gmail.com>>
>         ---
>          cr-dedup.c          | 34 ++++++++++++++++++++++++++--------
>          include/page-read.h |  5 ++++-
>          page-read.c         | 14 +++++++++++++-
>          3 files changed, 43 insertions(+), 10 deletions(-)
> 
>         diff --git a/cr-dedup.c b/cr-dedup.c
>         index 6f059d1..9d269d2 100644
>         --- a/cr-dedup.c
>         +++ b/cr-dedup.c
>         @@ -7,6 +7,8 @@
>          #include "page-read.h"
>          #include "restorer.h"
> 
>         +#define MAX_BUNCH_SIZE 256
>         +
>          static int cr_dedup_one_pagemap(int pid);
> 
>          int cr_dedup(void)
>         @@ -101,15 +103,30 @@ exit:
>                 return 0;
>          }
> 
>         -int punch_hole(int fd, unsigned long off, unsigned long len)
>         +int punch_hole(int fd, unsigned long off, unsigned long len,
>         +              struct iovec * bunch, bool cleanup, int pr_id)
>          {
>                 int ret;
>         -       pr_debug("Punch!/%lu/%lu/\n", off, len);
>         -       ret = fallocate(fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
>         -                       off, len);
>         -       if (ret != 0) {
>         -               pr_perror("Error punching hole");
>         -               return -1;
>         +
>         +       if ((unsigned long)bunch->iov_base + bunch->iov_len == off && !cleanup
>         +           && bunch->iov_len < MAX_BUNCH_SIZE * PAGE_SIZE) {
>         +               pr_debug("pr%d:Extend bunch len from %lx to %lx\n", pr_id,
>         +                        bunch->iov_len, bunch->iov_len + len);
>         +               bunch->iov_len += len;
>         +       } else {
>         +               if (bunch->iov_len > 0) {
>         +                       pr_debug("Punch!/%lx/%lx/\n", (unsigned long)bunch->iov_base, bunch->iov_len);
>         +                       ret = fallocate(fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
>         +                                       (unsigned long)bunch->iov_base, bunch->iov_len);
>         +                       if (ret != 0) {
>         +                               pr_perror("Error punching hole");
>         +                               return -1;
>         +                       }
>         +               }
>         +               bunch->iov_base = (void *)off;
>         +               bunch->iov_len = len;
>         +               pr_debug("pr%d:New bunch/%lx/%lx/\n", pr_id,
>         +                        (unsigned long)bunch->iov_base, bunch->iov_len);
>                 }
>                 return 0;
>          }
>         @@ -146,7 +163,8 @@ int dedup_one_iovec(struct page_read *pr, struct iovec *iov)
>                         piov_end = (unsigned long)piov.iov_base + piov.iov_len;
>                         off_real = lseek(pr->fd_pg, 0, SEEK_CUR);
>                         if (!pr->pe->in_parent) {
>         -                       ret = punch_hole(pr->fd_pg, off_real, min(piov_end, iov_end) - off);
>         +                       ret = punch_hole(pr->fd_pg, off_real, min(piov_end, iov_end) - off,
>         +                                        &pr->bunch, false, pr->id);
>                                 if (ret == -1)
>                                         return ret;
>                         }
>         diff --git a/include/page-read.h b/include/page-read.h
>         index 8b467c6..28435c7 100644
>         --- a/include/page-read.h
>         +++ b/include/page-read.h
>         @@ -63,6 +63,8 @@ struct page_read {
>                                                    read_pagemap_page */
>                 unsigned long cvaddr;           /* vaddr we are on */
> 
>         +       struct iovec bunch;             /* record consequent neighbour
>         +                                          iovecs to punch together */
>                 unsigned id; /* for logging */
>          };
> 
>         @@ -72,5 +74,6 @@ extern void pagemap2iovec(PagemapEntry *pe, struct iovec *iov);
>          extern int seek_pagemap_page(struct page_read *pr, unsigned long vaddr, bool warn);
> 
>          extern int dedup_one_iovec(struct page_read *pr, struct iovec *iov);
>         -extern int punch_hole(int fd, unsigned long off, unsigned long len);
>         +extern int punch_hole(int fd, unsigned long off, unsigned long len,
>         +                     struct iovec * bunch, bool cleanup, int pr_id);
>          #endif /* __CR_PAGE_READ_H__ */
>         diff --git a/page-read.c b/page-read.c
>         index e2514d6..0fd3464 100644
>         --- a/page-read.c
>         +++ b/page-read.c
>         @@ -149,7 +149,8 @@ static int read_pagemap_page(struct page_read *pr, unsigned long vaddr, void *bu
>                         }
> 
>                         if (opts.auto_dedup) {
>         -                       ret = punch_hole(pr->fd_pg, current_vaddr, (unsigned int)PAGE_SIZE);
>         +                       ret = punch_hole(pr->fd_pg, current_vaddr, (unsigned int)PAGE_SIZE,
>         +                                        &pr->bunch, false, pr->id);
>                                 if (ret == -1) {
>                                         return -1;
>                                 }
>         @@ -163,6 +164,15 @@ static int read_pagemap_page(struct page_read *pr, unsigned long vaddr, void *bu
> 
>          static void close_page_read(struct page_read *pr)
>          {
>         +       int ret;
>         +       if (pr->bunch.iov_len > 0) {
>         +               ret = punch_hole(pr->fd_pg, 0, 0, &pr->bunch, true, pr->id);
>         +               if (ret == -1)
>         +                       return;
> 
> 
>     Here I'm not sure if I can do something like this, and what will be better way to do it.
>     But this place is really suitable to make cleanup. So give me an advice, Thanks!
> 
> 
> Make close_page_read return int? Or it is unappropriate to make cleanup in close func? 

If close fails there's nothing we can do about it :) Thus void is OK.
BTW, I tend to think, that even if punch-hole fails, we should only
emit a warning (pr_warn) but not fail the whole process.

> 
>      
> 
>         +
>         +               pr->bunch.iov_len = 0;
>         +       }
>         +
>                 if (pr->parent) {
>                         close_page_read(pr->parent);
>                         xfree(pr->parent);
>         @@ -207,6 +217,8 @@ err_cl:
>          int open_page_read_at(int dfd, int pid, struct page_read *pr, int flags)
>          {
>                 pr->pe = NULL;
>         +       pr->bunch.iov_len = 0;
>         +       pr->bunch.iov_base = NULL;
> 
>                 pr->fd = open_image_at(dfd, CR_FD_PAGEMAP, O_RSTR, (long)pid);
>                 if (pr->fd < 0) {
>         --
>         1.8.3.2
> 
> 
> 




More information about the CRIU mailing list