restore: Don't open same files to mmap mutiple times

Submitted by Pavel Emelianov on April 13, 2017, 3:28 p.m.

Details

Message ID 58EF991C.3010206@virtuozzo.com
State New
Series "restore: Don't open same files to mmap mutiple times"
Headers show

Commit Message

Pavel Emelianov April 13, 2017, 3:28 p.m.
This is symmetrical to dump -- it's typical when one file
is mapped several times in a row. On dump we don't open
the mapping's map_files links for each of such files, thus
it makes sence to do the same on restore.

This speeds restore a little bit due to lower amount of
openat() and close() calls.

Signed-off-by: Pavel Emelyanov <xemul@virtuozzo.com>
---
 criu/files-reg.c         | 38 +++++++++++++++++++++++++++++++++++---
 criu/include/files-reg.h |  2 +-
 criu/mem.c               |  6 +++---
 criu/pie/restorer.c      | 13 +++++++++++--
 4 files changed, 50 insertions(+), 9 deletions(-)

Patch hide | download patch | download mbox

diff --git a/criu/files-reg.c b/criu/files-reg.c
index 4514275..6dbd6c9 100644
--- a/criu/files-reg.c
+++ b/criu/files-reg.c
@@ -1616,6 +1616,25 @@  int open_reg_by_id(u32 id)
 	return open_reg_fd(fd);
 }
 
+/*
+ * In many cases subsequent areas have the same file mapped,
+ * so we may save some open-s when mapping them and map the
+ * same descriptor many times (provided fdflags match).
+ */
+static struct {
+	struct file_desc *d;
+	u32 flags;
+	int fd;
+} prev_filemap;
+
+void filemap_fin_opens(void)
+{
+	if (prev_filemap.d) {
+		close(prev_filemap.fd);
+		prev_filemap.d = NULL;
+	}
+}
+
 static int open_filemap(int pid, struct vma_area *vma)
 {
 	u32 flags;
@@ -1636,9 +1655,20 @@  static int open_filemap(int pid, struct vma_area *vma)
 	else
 		flags = O_RDONLY;
 
-	ret = open_path(vma->vmfd, do_open_reg_noseek_flags, &flags);
-	if (ret < 0)
-		return ret;
+	if (prev_filemap.d == vma->vmfd && prev_filemap.flags == flags) {
+		BUG_ON(prev_filemap.fd < 0);
+		ret = prev_filemap.fd;
+	} else {
+		ret = open_path(vma->vmfd, do_open_reg_noseek_flags, &flags);
+		if (ret < 0)
+			return ret;
+
+		if (prev_filemap.d)
+			close(prev_filemap.fd);
+		prev_filemap.d = vma->vmfd;
+		prev_filemap.flags = flags;
+		prev_filemap.fd = ret;
+	}
 
 	vma->e->fd = ret;
 	return 0;
diff --git a/criu/include/files-reg.h b/criu/include/files-reg.h
index 5a6c691..5ef5e88 100644
--- a/criu/include/files-reg.h
+++ b/criu/include/files-reg.h
@@ -52,7 +52,7 @@  extern int prepare_remaps(void);
 extern int try_clean_remaps(bool only_ghosts);
 
 extern int strip_deleted(struct fd_link *link);
-
+extern void filemap_fin_opens(void);
 extern int prepare_procfs_remaps(void);
 extern int dead_pid_conflict(void);
 
diff --git a/criu/mem.c b/criu/mem.c
index 3bcf467..5efd377 100644
--- a/criu/mem.c
+++ b/criu/mem.c
@@ -568,8 +568,6 @@  static int map_private_vma(struct pstree_item *t,
 			pr_err("Can't fixup VMA's fd\n");
 			return -1;
 		}
-
-		vma->vm_open = NULL; /* prevent from 2nd open in prepare_vmas */
 	}
 
 	nr_pages = vma_entry_len(vma->e) / PAGE_SIZE;
@@ -668,7 +666,7 @@  static int map_private_vma(struct pstree_item *t,
 	}
 
 	if (vma_area_is(vma, VMA_FILE_PRIVATE))
-		close(vma->e->fd);
+		vma->vm_open = NULL; /* prevent from 2nd open in prepare_vmas */
 
 	*tgt_addr += size;
 	return 0;
@@ -709,6 +707,8 @@  static int premap_priv_vmas(struct pstree_item *t, struct vm_area_list *vmas, vo
 			break;
 	}
 
+	filemap_fin_opens();
+
 	return ret;
 }
 
diff --git a/criu/pie/restorer.c b/criu/pie/restorer.c
index 53ecaa0..85caf96 100644
--- a/criu/pie/restorer.c
+++ b/criu/pie/restorer.c
@@ -545,6 +545,7 @@  static long restore_self_exe_late(struct task_restore_args *args)
 
 static unsigned long restore_mapping(VmaEntry *vma_entry)
 {
+	static int prev_fd = -1;
 	int prot	= vma_entry->prot;
 	int flags	= vma_entry->flags | MAP_FIXED;
 	unsigned long addr;
@@ -595,8 +596,16 @@  static unsigned long restore_mapping(VmaEntry *vma_entry)
 			vma_entry->fd,
 			vma_entry->pgoff);
 
-	if (vma_entry->fd != -1)
-		sys_close(vma_entry->fd);
+	if (vma_entry->fd != -1) {
+		/*
+		 * Descriptors on VMA-s may be shared on subsequent
+		 * areas. See open_filemap() and filemap_fin_opens()
+		 */
+		if (prev_fd != vma_entry->fd) {
+			sys_close(vma_entry->fd);
+			prev_fd = vma_entry->fd;
+		}
+	}
 
 	return addr;
 }

Comments

Pavel Emelianov April 14, 2017, 10 a.m.
On 04/13/2017 09:27 PM, Patchwork wrote:
> == Series Details ==
> 
> Series: restore: Don't open same files to mmap mutiple times
> URL   : https://patchwork.criu.org/series/1453/
> State : failure
> 
> == Logs ==
> 
> For more details see: https://travis-ci.org/criupatchwork/criu/builds/221781082
> .
> 

Valid failure, ignore the patch for now.