[CRIU] [PATCHv2] pages: Share page_ids between ns dump-helpers

Pavel Emelyanov xemul at virtuozzo.com
Tue Aug 15 16:09:23 MSK 2017


On 08/15/2017 02:52 PM, Dmitry Safonov wrote:
> 2017-06-30 22:14 GMT+03:00 Dmitry Safonov <dsafonov at virtuozzo.com>:
>> After the commit 9d2e1dfebedf ("ipc: Keep shmem segments contents into
>> pagemap/page images"), SysV shmem pages are stored in the pages-<n>.img
>> files.  Where <n> is page_ids number.
>>
>> The problem here is that we dump namespaces with a helper after fork().
>> This results in non-shared between child(s) and parent-criu counter
>> with the result of overwriting pages-*.img files later by helpers/criu.
>> This resulted in restore with corrupted shmem segments or
>> in the following failures on restore:
>>> (04.501019)      1: Error (criu/pagemap.c:265): Can't read mapping page 0: No such file or directory
>>> (04.637516)      1: Error (criu/pagemap.c:265): Can't read mapping page 0: No such file or directory
>>
>> Allocate page_ids with shmalloc() and make it atomic.
>> As pre-dump, dump and page-server may run as a commands during
>> one service session, don't set it up early in main() - as
>> on the next command the value of page_ids will be preserved.
>> Rather do it lately during initialization of (pre-)dump/pageserver.
>> As (pre-)dump/pageserver rpc commands are executed under fork(),
>> we will not share the counter to criu.
>>
>> Cc: Pavel Emelyanov <xemul at virtuozzo.com>
>> Signed-off-by: Dmitry Safonov <dsafonov at virtuozzo.com>
>> ---
>> v2: Keep page_ids a little bit more privately by initing it later.
>>
>>  criu/cr-dump.c       |  6 ++++++
>>  criu/image.c         | 22 +++++++++++++++++-----
>>  criu/include/image.h |  3 ++-
>>  criu/page-xfer.c     |  7 ++++---
>>  criu/rst-malloc.c    |  5 +++++
>>  5 files changed, 34 insertions(+), 9 deletions(-)
> 
> ping?

Sorry. I have a not yet published branch that gets rid of separate pages-id thing.
So it's up to Andrey, whether he agrees to wait for it, or merges this fix.

-- Pavel

>>
>> diff --git a/criu/cr-dump.c b/criu/cr-dump.c
>> index 3f9f8d06a9ce..0ce03c601bfa 100644
>> --- a/criu/cr-dump.c
>> +++ b/criu/cr-dump.c
>> @@ -1612,6 +1612,9 @@ int cr_pre_dump_tasks(pid_t pid)
>>         struct pstree_item *item;
>>         int ret = -1;
>>
>> +       if (images_init(false))
>> +               goto err;
>> +
>>         if (opts.remote && push_snapshot_id() < 0) {
>>                 pr_err("Failed to push image namespace.\n");
>>                 goto err;
>> @@ -1813,6 +1816,9 @@ int cr_dump_tasks(pid_t pid)
>>         pr_info("Dumping processes (pid: %d)\n", pid);
>>         pr_info("========================================\n");
>>
>> +       if (images_init(false))
>> +               goto err;
>> +
>>         if (opts.remote && push_snapshot_id() < 0) {
>>                 pr_err("Failed to push image namespace.\n");
>>                 goto err;
>> diff --git a/criu/image.c b/criu/image.c
>> index 62e8e7109e65..4d8e9d28e8fd 100644
>> --- a/criu/image.c
>> +++ b/criu/image.c
>> @@ -17,6 +17,7 @@
>>  #include "images/inventory.pb-c.h"
>>  #include "images/pagemap.pb-c.h"
>>  #include "img-remote.h"
>> +#include "rst-malloc.h"
>>
>>  bool ns_per_id = false;
>>  bool img_common_magic = true;
>> @@ -505,10 +506,21 @@ void close_image_dir(void)
>>         close_service_fd(IMG_FD_OFF);
>>  }
>>
>> -static unsigned long page_ids = 1;
>> +/*
>> + * As we dump SysV shmem IPC into pages-<page_ids>.img,
>> + * the keys should be shared in each dump ns-helpers and in criu.
>> + */
>> +static atomic_t *page_ids;
>>
>> -void up_page_ids_base(void)
>> +int images_init(bool page_server_mode)
>>  {
>> +       BUG_ON(page_ids);
>> +       page_ids = shmalloc(sizeof(*page_ids));
>> +       if (!page_ids) {
>> +               pr_err("Failed to shmalloc page_ids\n");
>> +               return -1;
>> +       }
>> +
>>         /*
>>          * When page server and criu dump work on
>>          * the same dir, the shmem pagemaps and regular
>> @@ -516,9 +528,9 @@ void up_page_ids_base(void)
>>          * making page server produce page images with
>>          * higher IDs.
>>          */
>> +       atomic_set(page_ids, page_server_mode ? 0x10001 : 1);
>>
>> -       BUG_ON(page_ids != 1);
>> -       page_ids += 0x10000;
>> +       return 0;
>>  }
>>
>>  struct cr_img *open_pages_image_at(int dfd, unsigned long flags, struct cr_img *pmi, u32 *id)
>> @@ -531,7 +543,7 @@ struct cr_img *open_pages_image_at(int dfd, unsigned long flags, struct cr_img *
>>                 pagemap_head__free_unpacked(h, NULL);
>>         } else {
>>                 PagemapHead h = PAGEMAP_HEAD__INIT;
>> -               *id = h.pages_id = page_ids++;
>> +               *id = h.pages_id = atomic_inc_return(page_ids);
>>                 if (pb_write_one(pmi, &h, PB_PAGEMAP_HEAD) < 0)
>>                         return NULL;
>>         }
>> diff --git a/criu/include/image.h b/criu/include/image.h
>> index d9c4bdbc8da4..705c09a424a3 100644
>> --- a/criu/include/image.h
>> +++ b/criu/include/image.h
>> @@ -156,7 +156,6 @@ extern struct cr_img *open_image_at(int dfd, int type, unsigned long flags, ...)
>>  extern int open_image_lazy(struct cr_img *img);
>>  extern struct cr_img *open_pages_image(unsigned long flags, struct cr_img *pmi, u32 *pages_id);
>>  extern struct cr_img *open_pages_image_at(int dfd, unsigned long flags, struct cr_img *pmi, u32 *pages_id);
>> -extern void up_page_ids_base(void);
>>
>>  extern struct cr_img *img_from_fd(int fd); /* for cr-show mostly */
>>
>> @@ -170,4 +169,6 @@ extern int read_img_str(struct cr_img *, char **pstr, int size);
>>
>>  extern void close_image(struct cr_img *);
>>
>> +extern int images_init(bool page_server_mode);
>> +
>>  #endif /* __CR_IMAGE_H__ */
>> diff --git a/criu/page-xfer.c b/criu/page-xfer.c
>> index 52f5df9899c8..45c9bdf29f30 100644
>> --- a/criu/page-xfer.c
>> +++ b/criu/page-xfer.c
>> @@ -923,9 +923,10 @@ int cr_page_server(bool daemon_mode, bool lazy_dump, int cfd)
>>         int sk = -1;
>>         int ret;
>>
>> -       if (!opts.lazy_pages)
>> -               up_page_ids_base();
>> -       else if (!lazy_dump)
>> +       if (images_init(!opts.lazy_pages))
>> +               return -1;
>> +
>> +       if (opts.lazy_pages && !lazy_dump)
>>                 if (page_server_init_send())
>>                         return -1;
>>
>> diff --git a/criu/rst-malloc.c b/criu/rst-malloc.c
>> index 4f7357ca28c4..9de2fa77869b 100644
>> --- a/criu/rst-malloc.c
>> +++ b/criu/rst-malloc.c
>> @@ -112,6 +112,11 @@ static int grow_private(struct rst_mem_type_s *t, unsigned long size)
>>         return grow_remap(t, MAP_PRIVATE, size);
>>  }
>>
>> +/*
>> + * Service code expects that we do not share rst_mems allocator's
>> + * methodata and keep it in private. Otherwise restore may leave
>> + * some of shared resorces allocated to the next service command.
>> + */
>>  static struct rst_mem_type_s rst_mems[RST_MEM_TYPES] = {
>>         [RM_SHARED] = {
>>                 .grow = grow_shared,
>> --
>> 2.13.1
>>
> 
> 
> 



More information about the CRIU mailing list