namespace: mark mount namespaces as populated after the forking stage

Currently we mark a mount namespaces as populated when a target process
(ns_pid) switches into it. But if a process inherited the right
namespace from a parent, it doesn't call do_restore_task_mnt_ns()
and a namespace can remain unmarked.

af55c059 ("mount: fix a race between restoring namespaces and file mappings")
After this patch we could simplify logic around ns_populated. Currently
it's a futex, but nodoby waits on it. We can set ns_populated when we
are going to close namespace descriptors. To avoid additional locks, we
can do this when all task pass the forking stage and don't start the
next stage.

v2: add a comment why we wait a CR_STATE_FORKING stage
Signed-off-by: Andrew Vagin <avagin@virtuozzo.com>
Signed-off-by: Pavel Emelyanov <xemul@virtuozzo.com>

criu/cr-restore.c

@@ -1315,16 +1315,22 @@ static int restore_task_with_children(void *_arg)
 
 	restore_pgid();
 
-	if (restore_finish_stage(CR_STATE_FORKING) < 0)
-		goto err;
-
 	if (current->parent == NULL) {
+		/*
+		 * Wait when all tasks passed the CR_STATE_FORKING stage.
+		 * It means that all tasks entered into their namespaces.
+		 */
+		futex_wait_while_gt(&task_entries->nr_in_progress, 1);
+
 		if (depopulate_roots_yard())
 			goto err;
 
 		fini_restore_mntns();
 	}
 
+	if (restore_finish_stage(CR_STATE_FORKING) < 0)
+		goto err;
+
 	if (restore_one_task(current->pid.virt, ca->core))
 		goto err;
 

criu/include/namespaces.h

@@ -84,7 +84,7 @@ struct ns_id {
 	 * are mounted) and other tasks may do setns on it
 	 * and proceed.
 	 */
-	futex_t ns_populated;
+	bool ns_populated;
 
 	union {
 		struct {

criu/mount.c

@@ -2856,7 +2856,7 @@ static int rst_collect_local_mntns(enum ns_type typ)
 	if (!mntinfo)
 		return -1;
 
-	futex_set(&nsid->ns_populated, 1);
+	nsid->ns_populated = true;
 	return 0;
 }
 
@@ -3112,9 +3112,6 @@ static int do_restore_task_mnt_ns(struct ns_id *nsid, struct pstree_item *curren
 	}
 	close(fd);
 
-	if (nsid->ns_pid == current->pid.virt)
-		futex_set_and_wake(&nsid->ns_populated, 1);
-
 	return 0;
 }
 
@@ -3161,9 +3158,10 @@ void fini_restore_mntns(void)
 	for (nsid = ns_ids; nsid != NULL; nsid = nsid->next) {
 		if (nsid->nd != &mnt_ns_desc)
 			continue;
-		close(nsid->mnt.ns_fd);
+		close_safe(&nsid->mnt.ns_fd);
 		if (nsid->type != NS_ROOT)
-			close(nsid->mnt.root_fd);
+			close_safe(&nsid->mnt.root_fd);
+		nsid->ns_populated = true;
 	}
 }
 
@@ -3392,7 +3390,7 @@ int prepare_mnt_ns(void)
 			if (nsid->mnt.ns_fd < 0)
 				goto err;
 			/* we set ns_populated so we don't need to open root_fd */
-			futex_set(&nsid->ns_populated, 1);
+			nsid->ns_populated = true;
 			continue;
 		}
 
@@ -3526,7 +3524,7 @@ int mntns_get_root_fd(struct ns_id *mntns)
 	 * root from the root task.
 	 */
 
-	if (!futex_get(&mntns->ns_populated)) {
+	if (!mntns->ns_populated) {
 		int fd;
 
 		fd = open_proc(root_item->pid.virt, "fd/%d", mntns->mnt.root_fd);

criu/namespaces.c

@@ -293,7 +293,7 @@ struct ns_id *rst_new_ns_id(unsigned int id, pid_t pid,
 	if (nsid) {
 		nsid->type = type;
 		nsid_add(nsid, nd, id, pid);
-		futex_set(&nsid->ns_populated, 0);
+		nsid->ns_populated = false;
 	}
 
 	return nsid;
@@ -412,7 +412,7 @@ static unsigned int generate_ns_id(int pid, unsigned int kid, struct ns_desc *nd
 
 	nsid->type = type;
 	nsid->kid = kid;
-	futex_set(&nsid->ns_populated, 1);
+	nsid->ns_populated = true;
 	nsid_add(nsid, nd, ns_next_id++, pid);
 
 found: