[v2] criu: pagemap-cache -- Drop off greedy mode

Submitted by Cyrill Gorcunov on May 6, 2016, 12:39 p.m.

Details

Message ID 20160506123946.GH15368@uranus.lan
State Rejected
Series "criu: pagemap-cache -- Drop off greedy mode"
Headers show

Commit Message

Cyrill Gorcunov May 6, 2016, 12:39 p.m.
On Fri, May 06, 2016 at 03:27:43PM +0300, Pavel Emelyanov wrote:
> On 05/05/2016 11:55 PM, Cyrill Gorcunov wrote:
> > It never worked properly before and correct
> > implementation requires interactions with
> > parasite page fetching. 
> 
> Please write why it doesn't work, otherwise once (if) we get back to
> this we'll not be able to find out what the problem was easily.
> 

Attached
From: Cyrill Gorcunov <gorcunov@openvz.org>
To: Pavel Emelyanov <xemul@virtuozzo.com>
Cc: crml <criu@openvz.org>, Cyrill Gorcunov <gorcunov@openvz.org>
Subject: [PATCH] criu: pagemap-cache -- Drop off greedy mode

It never worked properly before because the pages might simply
be not present in memory at all, so trying to fetch their contents
via splice simply return the number of bytes successfully fetched
which of course different from completed vma size.

Thus correct implementation requires interactions with parasite page
pulling: just read as much as kernel allows us and that's all.

Util it's implemented -- zap it.

Reported-by: Pavel Emelyanov <xemul@virtuozzo.com>
Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
---
 criu/pagemap-cache.c | 38 ++++++++++++++++++--------------------
 1 file changed, 18 insertions(+), 20 deletions(-)

Patch hide | download patch | download mbox

diff --git a/criu/pagemap-cache.c b/criu/pagemap-cache.c
index e38bc3fb6b16..28bd8caa1f97 100644
--- a/criu/pagemap-cache.c
+++ b/criu/pagemap-cache.c
@@ -48,9 +48,19 @@  int pmc_init(pmc_t *pmc, pid_t pid, const struct list_head *vma_head, size_t siz
 		goto err;
 
 	if (kdat.pmap == PM_DISABLED) {
-		pmc->fd = -1;
-		pr_warn("No pagemap for %d available, "
-				"switching to greedy mode\n", pid);
+		/*
+		 * FIXME We might need to implement greedy
+		 * mode via reading all pages available inside
+		 * parasite.
+		 *
+		 * Actually since linux-4.4 the pagemap file
+		 * is available for usernamespace with hiding
+		 * PFNs but providing page attributes, so other
+		 * option simply require kernel 4.4 and above
+		 * for usernamespace support.
+		 */
+		pr_err("No pagemap for %d available\n", pid);
+		goto err;
 	} else {
 		pmc->fd = open_proc(pid, "pagemap");
 		if (pmc->fd < 0)
@@ -130,24 +140,12 @@  static int pmc_fill_cache(pmc_t *pmc, const struct vma_area *vma)
 
 	size_map = PAGEMAP_LEN(pmc->end - pmc->start);
 	BUG_ON(pmc->map_len < size_map);
+	BUG_ON(pmc->fd < 0);
 
-	if (unlikely(pmc->fd < 0)) {
-		u64 pme = PME_PRESENT;
-		size_t i;
-
-		/*
-		 * We don't have access to the dumpee pagemap so fill
-		 * everything as present. It's better than refuse
-		 * to dump because it simply disables optimisation.
-		 */
-		for (i = 0; i < (size_map / sizeof(pme)); i++)
-			pmc->map[i] = pme;
-	} else {
-		if (pread(pmc->fd, pmc->map, size_map, PAGEMAP_PFN_OFF(pmc->start)) != size_map) {
-			pmc_zap(pmc);
-			pr_perror("Can't read %d's pagemap file", pmc->pid);
-			return -1;
-		}
+	if (pread(pmc->fd, pmc->map, size_map, PAGEMAP_PFN_OFF(pmc->start)) != size_map) {
+		pmc_zap(pmc);
+		pr_perror("Can't read %d's pagemap file", pmc->pid);
+		return -1;
 	}
 
 	return 0;

Comments

Pavel Emelianov May 11, 2016, 12:34 p.m.
Applied