[CRIU] [PATCH v2 11/13] forking: Use last_pid_mutex for synchronization during clone()
Kirill Tkhai
ktkhai at virtuozzo.com
Tue May 16 09:27:07 PDT 2017
Before this patch we used flock to order task creation,
but this way is not good. It took 5 syscalls to synchronize
a creation of a single child:
1)open()
2)flock(LOCK_EX)
3)flock(LOCK_UN)
4)close() in parent
5)close() in child
The patch introduces more effective way for synchronization,
which executes 2 syscalls only. We use last_pid_mutex,
and the syscalls number sounds definitely better.
v2: Don't use flock() at all
Signed-off-by: Kirill Tkhai <ktkhai at virtuozzo.com>
---
criu/cr-restore.c | 20 +++-----------------
criu/img-remote-proto.c | 16 ----------------
criu/namespaces.c | 33 +++++----------------------------
criu/pie/restorer.c | 34 ++++++++++++++--------------------
4 files changed, 22 insertions(+), 81 deletions(-)
diff --git a/criu/cr-restore.c b/criu/cr-restore.c
index e042137bd..3ee7dfc07 100644
--- a/criu/cr-restore.c
+++ b/criu/cr-restore.c
@@ -1172,7 +1172,6 @@ static int restore_one_task(int pid, CoreEntry *core)
struct cr_clone_arg {
struct pstree_item *item;
unsigned long clone_flags;
- int fd;
CoreEntry *core;
};
@@ -1373,19 +1372,12 @@ static inline int fork_with_pid(struct pstree_item *item)
pr_info("Forking task with %d pid (flags 0x%lx)\n", pid, ca.clone_flags);
- ca.fd = open_proc_rw(PROC_GEN, LAST_PID_PATH);
- if (ca.fd < 0)
- goto err;
-
wait_pid_ns_helper_prepared(pid_ns, item->pid);
if (set_pid_ns_for_children(pid_ns, item->pid) < 0)
- goto err_close;
+ goto err;
- if (flock(ca.fd, LOCK_EX)) {
- pr_perror("%d: Can't lock %s", pid, LAST_PID_PATH);
- goto err_close;
- }
+ lock_last_pid();
ret = do_fork_with_pid(item, pid_ns, &ca);
if (ret < 0) {
@@ -1394,10 +1386,7 @@ static inline int fork_with_pid(struct pstree_item *item)
}
err_unlock:
- if (flock(ca.fd, LOCK_UN))
- pr_perror("%d: Can't unlock %s", pid, LAST_PID_PATH);
-err_close:
- close(ca.fd);
+ unlock_last_pid();
err:
if (ca.core)
core_entry__free_unpacked(ca.core, NULL);
@@ -1666,9 +1655,6 @@ static int restore_task_with_children(void *_arg)
if (current->pid->real < 0)
goto err;
- if ( !(ca->clone_flags & CLONE_FILES))
- close_safe(&ca->fd);
-
ret = clone_service_fd(rsti(current)->service_fd_id);
if (ret)
goto err;
diff --git a/criu/img-remote-proto.c b/criu/img-remote-proto.c
index 4f440976a..a5450e67c 100644
--- a/criu/img-remote-proto.c
+++ b/criu/img-remote-proto.c
@@ -616,28 +616,12 @@ void *accept_local_image_connections(void *port)
prepare_fwd_rimg();
}
- /* We need to flock the last pid file to avoid stealing pids
- * from restore.
- */
- int fd = open_proc_rw(PROC_GEN, LAST_PID_PATH);
- if (fd < 0)
- goto err;
-
- if (flock(fd, LOCK_EX)) {
- pr_perror("Can't lock %s", LAST_PID_PATH);
- goto err;
- }
-
if (pthread_create(
&tid, NULL, process_local_image_connection, (void *) wt)) {
pr_perror("Unable to create worker thread");
goto err;
}
- if (flock(fd, LOCK_UN))
- pr_perror("Can't unlock %s", LAST_PID_PATH);
- close(fd);
-
wt->tid = tid;
add_worker(wt);
}
diff --git a/criu/namespaces.c b/criu/namespaces.c
index 74c0ad7f4..75465e157 100644
--- a/criu/namespaces.c
+++ b/criu/namespaces.c
@@ -2621,7 +2621,7 @@ static int pid_ns_helper(struct ns_id *ns, int sk)
static int do_create_pid_ns_helper(void *arg, int sk, pid_t unused_pid)
{
- int pid_ns_fd, mnt_ns_fd, i, lock_fd, transport_fd, saved_errno;
+ int pid_ns_fd, i, transport_fd, saved_errno;
struct pstree_item *ns_reaper;
struct ns_id *ns, *tmp;
struct pid *pid;
@@ -2639,26 +2639,7 @@ static int do_create_pid_ns_helper(void *arg, int sk, pid_t unused_pid)
goto err;
}
- if (switch_ns(root_item->pid->real, &mnt_ns_desc, &mnt_ns_fd) < 0) {
- pr_err("Can't set mnt_ns\n");
- goto err;
- }
-
- lock_fd = open("/proc/" LAST_PID_PATH, O_RDONLY);
- if (lock_fd < 0) {
- pr_perror("Unable to open /proc/" LAST_PID_PATH);
- goto err;
- }
-
- if (restore_ns(mnt_ns_fd, &mnt_ns_desc) < 0) {
- pr_err("Can't restore ns\n");
- goto err;
- }
-
- if (flock(lock_fd, LOCK_EX)) {
- close(lock_fd);
- pr_perror("Can't lock %s", LAST_PID_PATH);
- }
+ lock_last_pid();
transport_fd = get_service_fd(TRANSPORT_FD_OFF);
BUG_ON(transport_fd < 0);
@@ -2670,18 +2651,14 @@ static int do_create_pid_ns_helper(void *arg, int sk, pid_t unused_pid)
for (i = pid->level - 2, tmp = ns->parent; i >= 0; i--, tmp = tmp->parent)
if (request_set_next_pid(tmp->id, pid->ns[i].virt, transport_fd)) {
pr_err("Can't set next pid using helper\n");
- flock(lock_fd, LOCK_UN);
- close(lock_fd);
+ unlock_last_pid();
goto err;
}
child = fork();
- if (!child) {
- close(lock_fd);
+ if (!child)
exit(pid_ns_helper(ns, sk));
- }
saved_errno = errno;
- flock(lock_fd, LOCK_UN);
- close(lock_fd);
+ unlock_last_pid();
if (child < 0) {
errno = saved_errno;
pr_perror("Can't fork");
diff --git a/criu/pie/restorer.c b/criu/pie/restorer.c
index 98c81f378..d369f6575 100644
--- a/criu/pie/restorer.c
+++ b/criu/pie/restorer.c
@@ -1488,20 +1488,18 @@ long __export_restore_task(struct task_restore_args *args)
CLONE_THREAD | CLONE_SYSVSEM | CLONE_FS;
long last_pid_len;
long parent_tid;
- int i, fd;
+ int i, fd = -1;
- fd = sys_openat(args->proc_fd, LAST_PID_PATH, O_RDWR, 0);
- if (fd < 0) {
- pr_err("can't open last pid fd %d\n", fd);
- goto core_restore_end;
+ if (thread_args[0].pid[1] == 0) {
+ /* One level pid ns hierarhy */
+ fd = sys_openat(args->proc_fd, LAST_PID_PATH, O_RDWR, 0);
+ if (fd < 0) {
+ pr_err("can't open last pid fd %d\n", fd);
+ goto core_restore_end;
+ }
}
- ret = sys_flock(fd, LOCK_EX);
- if (ret) {
- pr_err("Can't lock last_pid %d\n", fd);
- sys_close(fd);
- goto core_restore_end;
- }
+ mutex_lock(&task_entries_local->last_pid_mutex);
for (i = 0; i < args->nr_threads; i++) {
char last_pid_buf[16], *s;
@@ -1509,13 +1507,14 @@ long __export_restore_task(struct task_restore_args *args)
if (thread_args[i].pid[0] == args->t->pid[0])
continue;
- if (thread_args[i].pid[1] == 0) {
+ if (fd >= 0) {
/* One level pid ns hierarhy */
last_pid_len = std_vprint_num(last_pid_buf, sizeof(last_pid_buf), thread_args[i].pid[0] - 1, &s);
sys_lseek(fd, 0, SEEK_SET);
ret = sys_write(fd, s, last_pid_len);
if (ret < 0) {
pr_err("Can't set last_pid %ld/%s\n", ret, last_pid_buf);
+ mutex_unlock(&task_entries_local->last_pid_mutex);
sys_close(fd);
goto core_restore_end;
}
@@ -1525,7 +1524,7 @@ long __export_restore_task(struct task_restore_args *args)
break;
if (request_set_next_pid(args->pid_ns_id[k], thread_args[i].pid[k], args->transport_fd) < 0) {
pr_err("Can't request to set pid\n");
- sys_close(fd);
+ mutex_unlock(&task_entries_local->last_pid_mutex);
goto core_restore_end;
}
}
@@ -1542,14 +1541,9 @@ long __export_restore_task(struct task_restore_args *args)
RUN_CLONE_RESTORE_FN(ret, clone_flags, new_sp, parent_tid, thread_args, args->clone_restore_fn);
}
- ret = sys_flock(fd, LOCK_UN);
- if (ret) {
- pr_err("Can't unlock last_pid %ld\n", ret);
+ mutex_unlock(&task_entries_local->last_pid_mutex);
+ if (fd >= 0)
sys_close(fd);
- goto core_restore_end;
- }
-
- sys_close(fd);
}
restore_rlims(args);
More information about the CRIU
mailing list