<div dir="auto"><div><br><div class="gmail_extra"><br><div class="gmail_quote">15 авг 2017 г. 16:09 пользователь "Pavel Emelyanov" <<a href="mailto:xemul@virtuozzo.com">xemul@virtuozzo.com</a>> написал:<br type="attribution"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="elided-text">On 08/15/2017 02:52 PM, Dmitry Safonov wrote:<br>
> 2017-06-30 22:14 GMT+03:00 Dmitry Safonov <<a href="mailto:dsafonov@virtuozzo.com">dsafonov@virtuozzo.com</a>>:<br>
>> After the commit 9d2e1dfebedf ("ipc: Keep shmem segments contents into<br>
>> pagemap/page images"), SysV shmem pages are stored in the pages-<n>.img<br>
>> files. Where <n> is page_ids number.<br>
>><br>
>> The problem here is that we dump namespaces with a helper after fork().<br>
>> This results in non-shared between child(s) and parent-criu counter<br>
>> with the result of overwriting pages-*.img files later by helpers/criu.<br>
>> This resulted in restore with corrupted shmem segments or<br>
>> in the following failures on restore:<br>
>>> (04.501019) 1: Error (criu/pagemap.c:265): Can't read mapping page 0: No such file or directory<br>
>>> (04.637516) 1: Error (criu/pagemap.c:265): Can't read mapping page 0: No such file or directory<br>
>><br>
>> Allocate page_ids with shmalloc() and make it atomic.<br>
>> As pre-dump, dump and page-server may run as a commands during<br>
>> one service session, don't set it up early in main() - as<br>
>> on the next command the value of page_ids will be preserved.<br>
>> Rather do it lately during initialization of (pre-)dump/pageserver.<br>
>> As (pre-)dump/pageserver rpc commands are executed under fork(),<br>
>> we will not share the counter to criu.<br>
>><br>
>> Cc: Pavel Emelyanov <<a href="mailto:xemul@virtuozzo.com">xemul@virtuozzo.com</a>><br>
>> Signed-off-by: Dmitry Safonov <<a href="mailto:dsafonov@virtuozzo.com">dsafonov@virtuozzo.com</a>><br>
>> ---<br>
>> v2: Keep page_ids a little bit more privately by initing it later.<br>
>><br>
>> criu/cr-dump.c | 6 ++++++<br>
>> criu/image.c | 22 +++++++++++++++++-----<br>
>> criu/include/image.h | 3 ++-<br>
>> criu/page-xfer.c | 7 ++++---<br>
>> criu/rst-malloc.c | 5 +++++<br>
>> 5 files changed, 34 insertions(+), 9 deletions(-)<br>
><br>
> ping?<br>
<br>
</div>Sorry. I have a not yet published branch that gets rid of separate pages-id thing.<br>
So it's up to Andrey, whether he agrees to wait for it, or merges this fix.<br></blockquote></div></div></div><div dir="auto"><br></div><div dir="auto">No worry. I've just reminded so it will not get lost ;)</div><div dir="auto"><br></div><div dir="auto"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<font color="#888888"><br>
-- Pavel<br>
</font><div class="elided-text"><br>
>><br>
>> diff --git a/criu/cr-dump.c b/criu/cr-dump.c<br>
>> index 3f9f8d06a9ce..0ce03c601bfa 100644<br>
>> --- a/criu/cr-dump.c<br>
>> +++ b/criu/cr-dump.c<br>
>> @@ -1612,6 +1612,9 @@ int cr_pre_dump_tasks(pid_t pid)<br>
>> struct pstree_item *item;<br>
>> int ret = -1;<br>
>><br>
>> + if (images_init(false))<br>
>> + goto err;<br>
>> +<br>
>> if (opts.remote && push_snapshot_id() < 0) {<br>
>> pr_err("Failed to push image namespace.\n");<br>
>> goto err;<br>
>> @@ -1813,6 +1816,9 @@ int cr_dump_tasks(pid_t pid)<br>
>> pr_info("Dumping processes (pid: %d)\n", pid);<br>
>> pr_info("=====================<wbr>===================\n");<br>
>><br>
>> + if (images_init(false))<br>
>> + goto err;<br>
>> +<br>
>> if (opts.remote && push_snapshot_id() < 0) {<br>
>> pr_err("Failed to push image namespace.\n");<br>
>> goto err;<br>
>> diff --git a/criu/image.c b/criu/image.c<br>
>> index 62e8e7109e65..4d8e9d28e8fd 100644<br>
>> --- a/criu/image.c<br>
>> +++ b/criu/image.c<br>
>> @@ -17,6 +17,7 @@<br>
>> #include "images/inventory.pb-c.h"<br>
>> #include "images/pagemap.pb-c.h"<br>
>> #include "img-remote.h"<br>
>> +#include "rst-malloc.h"<br>
>><br>
>> bool ns_per_id = false;<br>
>> bool img_common_magic = true;<br>
>> @@ -505,10 +506,21 @@ void close_image_dir(void)<br>
>> close_service_fd(IMG_FD_OFF);<br>
>> }<br>
>><br>
>> -static unsigned long page_ids = 1;<br>
>> +/*<br>
>> + * As we dump SysV shmem IPC into pages-<page_ids>.img,<br>
>> + * the keys should be shared in each dump ns-helpers and in criu.<br>
>> + */<br>
>> +static atomic_t *page_ids;<br>
>><br>
>> -void up_page_ids_base(void)<br>
>> +int images_init(bool page_server_mode)<br>
>> {<br>
>> + BUG_ON(page_ids);<br>
>> + page_ids = shmalloc(sizeof(*page_ids));<br>
>> + if (!page_ids) {<br>
>> + pr_err("Failed to shmalloc page_ids\n");<br>
>> + return -1;<br>
>> + }<br>
>> +<br>
>> /*<br>
>> * When page server and criu dump work on<br>
>> * the same dir, the shmem pagemaps and regular<br>
>> @@ -516,9 +528,9 @@ void up_page_ids_base(void)<br>
>> * making page server produce page images with<br>
>> * higher IDs.<br>
>> */<br>
>> + atomic_set(page_ids, page_server_mode ? 0x10001 : 1);<br>
>><br>
>> - BUG_ON(page_ids != 1);<br>
>> - page_ids += 0x10000;<br>
>> + return 0;<br>
>> }<br>
>><br>
>> struct cr_img *open_pages_image_at(int dfd, unsigned long flags, struct cr_img *pmi, u32 *id)<br>
>> @@ -531,7 +543,7 @@ struct cr_img *open_pages_image_at(int dfd, unsigned long flags, struct cr_img *<br>
>> pagemap_head__free_unpacked(h, NULL);<br>
>> } else {<br>
>> PagemapHead h = PAGEMAP_HEAD__INIT;<br>
>> - *id = h.pages_id = page_ids++;<br>
>> + *id = h.pages_id = atomic_inc_return(page_ids);<br>
>> if (pb_write_one(pmi, &h, PB_PAGEMAP_HEAD) < 0)<br>
>> return NULL;<br>
>> }<br>
>> diff --git a/criu/include/image.h b/criu/include/image.h<br>
>> index d9c4bdbc8da4..705c09a424a3 100644<br>
>> --- a/criu/include/image.h<br>
>> +++ b/criu/include/image.h<br>
>> @@ -156,7 +156,6 @@ extern struct cr_img *open_image_at(int dfd, int type, unsigned long flags, ...)<br>
>> extern int open_image_lazy(struct cr_img *img);<br>
>> extern struct cr_img *open_pages_image(unsigned long flags, struct cr_img *pmi, u32 *pages_id);<br>
>> extern struct cr_img *open_pages_image_at(int dfd, unsigned long flags, struct cr_img *pmi, u32 *pages_id);<br>
>> -extern void up_page_ids_base(void);<br>
>><br>
>> extern struct cr_img *img_from_fd(int fd); /* for cr-show mostly */<br>
>><br>
>> @@ -170,4 +169,6 @@ extern int read_img_str(struct cr_img *, char **pstr, int size);<br>
>><br>
>> extern void close_image(struct cr_img *);<br>
>><br>
>> +extern int images_init(bool page_server_mode);<br>
>> +<br>
>> #endif /* __CR_IMAGE_H__ */<br>
>> diff --git a/criu/page-xfer.c b/criu/page-xfer.c<br>
>> index 52f5df9899c8..45c9bdf29f30 100644<br>
>> --- a/criu/page-xfer.c<br>
>> +++ b/criu/page-xfer.c<br>
>> @@ -923,9 +923,10 @@ int cr_page_server(bool daemon_mode, bool lazy_dump, int cfd)<br>
>> int sk = -1;<br>
>> int ret;<br>
>><br>
>> - if (!opts.lazy_pages)<br>
>> - up_page_ids_base();<br>
>> - else if (!lazy_dump)<br>
>> + if (images_init(!opts.lazy_pages)<wbr>)<br>
>> + return -1;<br>
>> +<br>
>> + if (opts.lazy_pages && !lazy_dump)<br>
>> if (page_server_init_send())<br>
>> return -1;<br>
>><br>
>> diff --git a/criu/rst-malloc.c b/criu/rst-malloc.c<br>
>> index 4f7357ca28c4..9de2fa77869b 100644<br>
>> --- a/criu/rst-malloc.c<br>
>> +++ b/criu/rst-malloc.c<br>
>> @@ -112,6 +112,11 @@ static int grow_private(struct rst_mem_type_s *t, unsigned long size)<br>
>> return grow_remap(t, MAP_PRIVATE, size);<br>
>> }<br>
>><br>
>> +/*<br>
>> + * Service code expects that we do not share rst_mems allocator's<br>
>> + * methodata and keep it in private. Otherwise restore may leave<br>
>> + * some of shared resorces allocated to the next service command.<br>
>> + */<br>
>> static struct rst_mem_type_s rst_mems[RST_MEM_TYPES] = {<br>
>> [RM_SHARED] = {<br>
>> .grow = grow_shared,<br>
>> --<br>
>> 2.13.1<br>
>><br>
><br>
><br>
><br>
<br>
</div></blockquote></div><br></div></div></div>