criu/mem: refactor should_dump_page helper

Make should_dump_page to return int to indicate failure, also
return useful data back through the struct page_info structure
passed as a pointer.

Also, correspondingly convert all call sites.

No functional changes intended, except fixing a bug in
should_dump_page() as it could return (-1) when pmc_fill()
fails, while caller didn't expect that before.

Signed-off-by: Alexander Mikhalitsyn <[email protected]>

Author: mihalicyn

criu/include/mem.h

@@ -49,5 +49,11 @@ int prepare_vmas(struct pstree_item *t, struct task_restore_args *ta);
 int unmap_guard_pages(struct pstree_item *t);
 int prepare_mappings(struct pstree_item *t);
 
-u64 should_dump_page(pmc_t *pmc, VmaEntry *vmae, u64 vaddr, bool *softdirty);
+struct page_info {
+	u64 next;
+	bool softdirty;
+};
+
+int should_dump_page(pmc_t *pmc, VmaEntry *vmae, u64 vaddr, struct page_info *page_info);
+
 #endif /* __CR_MEM_H__ */

criu/mem.c

@@ -115,44 +115,64 @@ static bool should_dump_entire_vma(VmaEntry *vmae)
 }
 
 /*
- * should_dump_page returns vaddr if an addressed page has to be dumped.
- * Otherwise, it returns an address that has to be inspected next.
+ * should_dump_page writes vaddr in page_info->next if an addressed page has to be dumped.
+ * Otherwise, it writes an address that has to be inspected next.
  */
-u64 should_dump_page(pmc_t *pmc, VmaEntry *vmae, u64 vaddr, bool *softdirty)
+int should_dump_page(pmc_t *pmc, VmaEntry *vmae, u64 vaddr, struct page_info *page_info)
 {
+	if (!page_info)
+		goto err;
+
 	if (vaddr >= pmc->end && pmc_fill(pmc, vaddr, vmae->end))
-		return -1;
+		goto err;
 
 	if (pmc->regs) {
 		while (1) {
-			if (pmc->regs_idx == pmc->regs_len)
-				return pmc->end;
+			if (pmc->regs_idx == pmc->regs_len) {
+				page_info->next = pmc->end;
+				return 0;
+			}
+
 			if (vaddr < pmc->regs[pmc->regs_idx].end)
 				break;
 			pmc->regs_idx++;
 		}
-		if (vaddr < pmc->regs[pmc->regs_idx].start)
-			return pmc->regs[pmc->regs_idx].start;
-		if (softdirty)
-			*softdirty = pmc->regs[pmc->regs_idx].categories & PAGE_IS_SOFT_DIRTY;
-		return vaddr;
+
+		if (vaddr < pmc->regs[pmc->regs_idx].start) {
+			page_info->next = pmc->regs[pmc->regs_idx].start;
+			return 0;
+		}
+
+		page_info->softdirty = pmc->regs[pmc->regs_idx].categories & PAGE_IS_SOFT_DIRTY;
+		page_info->next = vaddr;
+		return 0;
 	} else {
 		u64 pme = pmc->map[PAGE_PFN(vaddr - pmc->start)];
 
 		/*
 		 * Optimisation for private mapping pages, that haven't
 		 * yet being COW-ed
 		 */
-		if (vma_entry_is(vmae, VMA_FILE_PRIVATE) && (pme & PME_FILE))
-			return vaddr + PAGE_SIZE;
+		if (vma_entry_is(vmae, VMA_FILE_PRIVATE) && (pme & PME_FILE)) {
+			page_info->next = vaddr + PAGE_SIZE;
+			return 0;
+		}
+
 		if ((pme & (PME_PRESENT | PME_SWAP)) && !__page_is_zero(pme)) {
-			if (softdirty)
-				*softdirty = pme & PME_SOFT_DIRTY;
-			return vaddr;
+			page_info->softdirty = pme & PME_SOFT_DIRTY;
+			page_info->next = vaddr;
+			return 0;
 		}
 
-		return vaddr + PAGE_SIZE;
+		page_info->next = vaddr + PAGE_SIZE;
+		return 0;
 	}
+
+err:
+	pr_err("should_dump_page failed on vma "
+	       "%#016" PRIx64 "-%#016" PRIx64 " vaddr=%#016" PRIx64 "\n",
+	       vmae->start, vmae->end, vaddr);
+	return -1;
 }
 
 bool page_is_zero(u64 pme)
@@ -202,14 +222,15 @@ static int generate_iovs(struct pstree_item *item, struct vma_area *vma, struct
 	nr_scanned = 0;
 	for (vaddr = *pvaddr; vaddr < vma->e->end; vaddr += PAGE_SIZE, nr_scanned++) {
 		unsigned int ppb_flags = 0;
-		bool softdirty = false;
-		u64 next;
+		struct page_info page_info = {};
 		int st;
 
 		/* If dump_all_pages is true, should_dump_page is called to get pme. */
-		next = should_dump_page(pmc, vma->e, vaddr, &softdirty);
-		if (!dump_all_pages && next != vaddr) {
-			vaddr = next - PAGE_SIZE;
+		if (should_dump_page(pmc, vma->e, vaddr, &page_info))
+			return -1;
+
+		if (!dump_all_pages && page_info.next != vaddr) {
+			vaddr = page_info.next - PAGE_SIZE;
 			continue;
 		}
 
@@ -223,7 +244,7 @@ static int generate_iovs(struct pstree_item *item, struct vma_area *vma, struct
 		 * page. The latter would be checked in page-xfer.
 		 */
 
-		if (has_parent && page_in_parent(softdirty)) {
+		if (has_parent && page_in_parent(page_info.softdirty)) {
 			ret = page_pipe_add_hole(pp, vaddr, PP_HOLE_PARENT);
 			st = 0;
 		} else {

criu/shmem.c

@@ -206,31 +206,34 @@ static int expand_shmem(struct shmem_info *si, unsigned long new_size)
 	return 0;
 }
 
-static void update_shmem_pmaps(struct shmem_info *si, pmc_t *pmc, VmaEntry *vma)
+static int update_shmem_pmaps(struct shmem_info *si, pmc_t *pmc, VmaEntry *vma)
 {
 	unsigned long shmem_pfn, vma_pfn, vma_pgcnt;
 	u64 vaddr;
 
 	if (!is_shmem_tracking_en())
-		return;
+		return 0;
 
 	vma_pgcnt = DIV_ROUND_UP(si->size - vma->pgoff, PAGE_SIZE);
 	for (vma_pfn = 0, vaddr = vma->start; vma_pfn < vma_pgcnt; ++vma_pfn, vaddr += PAGE_SIZE) {
-		bool softdirty = false;
-		u64 next;
+		struct page_info page_info = {};
+
+		if (should_dump_page(pmc, vma, vaddr, &page_info))
+			return -1;
 
-		next = should_dump_page(pmc, vma, vaddr, &softdirty);
-		if (next != vaddr) {
-			vaddr = next - PAGE_SIZE;
+		if (page_info.next != vaddr) {
+			vaddr = page_info.next - PAGE_SIZE;
 			continue;
 		}
 
 		shmem_pfn = vma_pfn + DIV_ROUND_UP(vma->pgoff, PAGE_SIZE);
-		if (softdirty)
+		if (page_info.softdirty)
 			set_pstate(si->pstate_map, shmem_pfn, PST_DIRTY);
 		else
 			set_pstate(si->pstate_map, shmem_pfn, PST_DUMP);
 	}
+
+	return 0;
 }
 
 int collect_sysv_shmem(unsigned long shmid, unsigned long size)
@@ -667,7 +670,9 @@ int add_shmem_area(pid_t pid, VmaEntry *vma, pmc_t *pmc)
 			if (expand_shmem(si, size))
 				return -1;
 		}
-		update_shmem_pmaps(si, pmc, vma);
+
+		if (update_shmem_pmaps(si, pmc, vma))
+			return -1;
 
 		return 0;
 	}
@@ -684,7 +689,9 @@ int add_shmem_area(pid_t pid, VmaEntry *vma, pmc_t *pmc)
 
 	if (expand_shmem(si, size))
 		return -1;
-	update_shmem_pmaps(si, pmc, vma);
+
+	if (update_shmem_pmaps(si, pmc, vma))
+		return -1;
 
 	return 0;
 }