[CRIU] [PATCH] Make descriptive names in vm_area_list

Pavel Emelyanov xemul at parallels.com
Fri Apr 12 12:38:22 EDT 2013


On 04/12/2013 01:21 PM, Cyrill Gorcunov wrote:
> vm_area_list::h and vm_area_list::nr names are
> too short for structure which is widely used
> in the code. Rename them to @vma_list and @nr_vmas
> respectively (moreover, in task_restore_core_args
> structure we already have a member named @nr_vmas
> so this brings some consistency over naming).

I'd agree that the vma_list::h is not nice, but the vma_list::nr is
just way too much better than the vma_list::nr_vmas.

> Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
> ---
>  cr-dump.c          | 12 ++++++------
>  cr-restore.c       | 31 ++++++++++++++++---------------
>  include/crtools.h  | 10 +++++++---
>  mem.c              |  4 ++--
>  parasite-syscall.c |  4 ++--
>  proc_parse.c       |  8 ++++----
>  6 files changed, 37 insertions(+), 32 deletions(-)
> 
> diff --git a/cr-dump.c b/cr-dump.c
> index d5eb072..d2048f1 100644
> --- a/cr-dump.c
> +++ b/cr-dump.c
> @@ -99,14 +99,14 @@ void free_mappings(struct vm_area_list *vma_area_list)
>  {
>  	struct vma_area *vma_area, *p;
>  
> -	list_for_each_entry_safe(vma_area, p, &vma_area_list->h, list) {
> +	list_for_each_entry_safe(vma_area, p, &vma_area_list->vma_list, list) {
>  		if (vma_area->vm_file_fd > 0)
>  			close(vma_area->vm_file_fd);
>  		free(vma_area);
>  	}
>  
> -	INIT_LIST_HEAD(&vma_area_list->h);
> -	vma_area_list->nr = 0;
> +	INIT_LIST_HEAD(&vma_area_list->vma_list);
> +	vma_area_list->nr_vmas = 0;
>  }
>  
>  int collect_mappings(pid_t pid, struct vm_area_list *vma_area_list)
> @@ -122,7 +122,7 @@ int collect_mappings(pid_t pid, struct vm_area_list *vma_area_list)
>  		goto err;
>  
>  	pr_info("Collected, longest ares %lu bytes\n", vma_area_list->longest);
> -	pr_info_vma_list(&vma_area_list->h);
> +	pr_info_vma_list(&vma_area_list->vma_list);
>  
>  	pr_info("----------------------------------------\n");
>  err:
> @@ -355,7 +355,7 @@ static int dump_task_mappings(pid_t pid, const struct vm_area_list *vma_area_lis
>  
>  	fd = fdset_fd(cr_fdset, CR_FD_VMAS);
>  
> -	list_for_each_entry(vma_area, &vma_area_list->h, list) {
> +	list_for_each_entry(vma_area, &vma_area_list->vma_list, list) {
>  		VmaEntry *vma = &vma_area->vma;
>  
>  		pr_info_vma(vma_area);
> @@ -690,7 +690,7 @@ static int dump_task_core_all(struct parasite_ctl *ctl,
>  	if (ret)
>  		goto err_free;
>  
> -	mark_stack_vma(CORE_THREAD_ARCH_INFO(core)->gpregs->sp, &vma_area_list->h);
> +	mark_stack_vma(CORE_THREAD_ARCH_INFO(core)->gpregs->sp, &vma_area_list->vma_list);
>  
>  	ret = get_task_futex_robust_list(pid, core->thread_core);
>  	if (ret)
> diff --git a/cr-restore.c b/cr-restore.c
> index c7a31c6..e26a06c 100644
> --- a/cr-restore.c
> +++ b/cr-restore.c
> @@ -274,7 +274,7 @@ static int restore_priv_vma_content(pid_t pid)
>  	unsigned int nr_droped = 0;
>  	unsigned long va;
>  
> -	vma = list_first_entry(&rst_vmas.h, struct vma_area, list);
> +	vma = list_first_entry(&rst_vmas.vma_list, struct vma_area, list);
>  
>  	fd = open_image(CR_FD_PAGEMAP, O_RSTR, (long)pid);
>  	if (fd < 0) {
> @@ -326,7 +326,7 @@ static int restore_priv_vma_content(pid_t pid)
>  			 * read from image file.
>  			 */
>  			while (va >= vma->vma.end) {
> -				if (vma->list.next == &rst_vmas.h)
> +				if (vma->list.next == &rst_vmas.vma_list)
>  					goto err_addr;
>  				vma = list_entry(vma->list.next, struct vma_area, list);
>  			}
> @@ -375,7 +375,7 @@ static int restore_priv_vma_content(pid_t pid)
>  		return ret;
>  
>  	/* Remove pages, which were not shared with a child */
> -	list_for_each_entry(vma, &rst_vmas.h, list) {
> +	list_for_each_entry(vma, &rst_vmas.vma_list, list) {
>  		unsigned long size, i = 0;
>  		void *addr = decode_pointer(vma_premmaped_start(&vma->vma));
>  
> @@ -424,8 +424,8 @@ static int read_vmas(int pid)
>  	void *old_premmapped_addr = NULL;
>  	unsigned long old_premmapped_len, pstart = 0;
>  
> -	rst_vmas.nr = 0;
> -	list_replace_init(&rst_vmas.h, &old);
> +	rst_vmas.nr_vmas = 0;
> +	list_replace_init(&rst_vmas.vma_list, &old);
>  
>  	/* Skip errors, because a zombie doesn't have an image of vmas */
>  	fd = open_image(CR_FD_VMAS, O_RSTR, pid);
> @@ -450,8 +450,8 @@ static int read_vmas(int pid)
>  			break;
>  		}
>  
> -		rst_vmas.nr++;
> -		list_add_tail(&vma->list, &rst_vmas.h);
> +		rst_vmas.nr_vmas++;
> +		list_add_tail(&vma->list, &rst_vmas.vma_list);
>  
>  		vma->vma = *e;
>  		vma_entry__free_unpacked(e, NULL);
> @@ -486,7 +486,7 @@ static int read_vmas(int pid)
>  
>  	pvma = list_entry(&old, struct vma_area, list);
>  
> -	list_for_each_entry(vma, &rst_vmas.h, list) {
> +	list_for_each_entry(vma, &rst_vmas.vma_list, list) {
>  		if (pstart > vma->vma.start) {
>  			ret = -1;
>  			pr_err("VMA-s are not sorted in the image file\n");
> @@ -531,7 +531,7 @@ static int open_vmas(int pid)
>  	struct vma_area *vma;
>  	int ret = 0;
>  
> -	list_for_each_entry(vma, &rst_vmas.h, list) {
> +	list_for_each_entry(vma, &rst_vmas.vma_list, list) {
>  		if (!(vma_entry_is(&vma->vma, VMA_AREA_REGULAR)))
>  			continue;
>  
> @@ -1534,7 +1534,7 @@ static VmaEntry *vma_list_remap(void *addr, unsigned long len, struct vm_area_li
>  		return NULL;
>  	}
>  
> -	list_for_each_entry(vma_area, &vmas->h, list) {
> +	list_for_each_entry(vma_area, &vmas->vma_list, list) {
>  		*vma = vma_area->vma;
>  		vma++;
>  	}
> @@ -1803,10 +1803,10 @@ static int sigreturn_restore(pid_t pid, CoreEntry *core)
>  		goto err;
>  
>  	/* required to unmap stack _with_ guard page */
> -	mark_stack_vma((long) &self_vmas, &self_vmas.h);
> +	mark_stack_vma((long) &self_vmas, &self_vmas.vma_list);
>  
> -	self_vmas_len = round_up((self_vmas.nr + 1) * sizeof(VmaEntry), PAGE_SIZE);
> -	vmas_len = round_up((rst_vmas.nr + 1) * sizeof(VmaEntry), PAGE_SIZE);
> +	self_vmas_len = round_up((self_vmas.nr_vmas + 1) * sizeof(VmaEntry), PAGE_SIZE);
> +	vmas_len = round_up((rst_vmas.nr_vmas + 1) * sizeof(VmaEntry), PAGE_SIZE);
>  
>  	/* pr_info_vma_list(&self_vma_list); */
>  
> @@ -1866,7 +1866,8 @@ static int sigreturn_restore(pid_t pid, CoreEntry *core)
>  	 * or inited from scratch).
>  	 */
>  
> -	exec_mem_hint = restorer_get_vma_hint(pid, &rst_vmas.h, &self_vmas.h,
> +	exec_mem_hint = restorer_get_vma_hint(pid, &rst_vmas.vma_list,
> +					      &self_vmas.vma_list,
>  					      restore_bootstrap_len);
>  	if (exec_mem_hint == -1) {
>  		pr_err("No suitable area for task_restore bootstrap (%ldK)\n",
> @@ -1948,7 +1949,7 @@ static int sigreturn_restore(pid_t pid, CoreEntry *core)
>  		goto err;
>  
>  	mem += self_vmas_len;
> -	task_args->nr_vmas = rst_vmas.nr;
> +	task_args->nr_vmas = rst_vmas.nr_vmas;
>  	task_args->tgt_vmas = vma_list_remap(mem, vmas_len, &rst_vmas);
>  	task_args->premmapped_addr = (unsigned long) current->rst->premmapped_addr;
>  	task_args->premmapped_len = current->rst->premmapped_len;
> diff --git a/include/crtools.h b/include/crtools.h
> index 6cd4cde..620e013 100644
> --- a/include/crtools.h
> +++ b/include/crtools.h
> @@ -232,13 +232,17 @@ struct cr_fdset *cr_glob_fdset_open(int mode);
>  void close_cr_fdset(struct cr_fdset **cr_fdset);
>  
>  struct vm_area_list {
> -	struct list_head	h;
> -	unsigned		nr;
> +	struct list_head	vma_list;
> +	unsigned		nr_vmas;
>  	unsigned long		priv_size; /* nr of pages in private VMAs */
>  	unsigned long		longest; /* nr of pages in longest VMA */
>  };
>  
> -#define VM_AREA_LIST(name)	struct vm_area_list name = { .h = LIST_HEAD_INIT(name.h), .nr = 0, }
> +#define VM_AREA_LIST(name)					\
> +	struct vm_area_list name = {				\
> +		.vma_list = LIST_HEAD_INIT(name.vma_list),	\
> +		.nr_vmas = 0,					\
> +	}
>  
>  int collect_mappings(pid_t pid, struct vm_area_list *vma_area_list);
>  void free_mappings(struct vm_area_list *vma_area_list);
> diff --git a/mem.c b/mem.c
> index f72d2db..081c1dc 100644
> --- a/mem.c
> +++ b/mem.c
> @@ -81,7 +81,7 @@ static int parasite_mprotect_seized(struct parasite_ctl *ctl, struct vm_area_lis
>  	p_vma = args->vmas;
>  	args->nr = 0;
>  
> -	list_for_each_entry(vma, &vma_area_list->h, list) {
> +	list_for_each_entry(vma, &vma_area_list->vma_list, list) {
>  		if (!privately_dump_vma(vma))
>  			continue;
>  		if (vma->vma.prot & PROT_READ)
> @@ -132,7 +132,7 @@ static int __parasite_dump_pages_seized(struct parasite_ctl *ctl,
>  	if (!pp)
>  		goto out_close;
>  
> -	list_for_each_entry(vma_area, &vma_area_list->h, list) {
> +	list_for_each_entry(vma_area, &vma_area_list->vma_list, list) {
>  		if (!privately_dump_vma(vma_area))
>  			continue;
>  
> diff --git a/parasite-syscall.c b/parasite-syscall.c
> index ba91123..f5cb96b 100644
> --- a/parasite-syscall.c
> +++ b/parasite-syscall.c
> @@ -488,7 +488,7 @@ int parasite_dump_creds(struct parasite_ctl *ctl, CredsEntry *ce)
>  static unsigned int vmas_mprotect_size(struct vm_area_list *vmas)
>  {
>  	return sizeof(struct parasite_mprotect_args) +
> -		(vmas->nr * sizeof(struct parasite_vma_entry));
> +		(vmas->nr_vmas * sizeof(struct parasite_vma_entry));
>  }
>  
>  int parasite_drain_fds_seized(struct parasite_ctl *ctl,
> @@ -653,7 +653,7 @@ struct parasite_ctl *parasite_prep_ctl(pid_t pid, struct vm_area_list *vma_area_
>  		goto err;
>  	}
>  
> -	vma_area = get_vma_by_ip(&vma_area_list->h, REG_IP(ctl->regs_orig));
> +	vma_area = get_vma_by_ip(&vma_area_list->vma_list, REG_IP(ctl->regs_orig));
>  	if (!vma_area) {
>  		pr_err("No suitable VMA found to run parasite "
>  		       "bootstrap code (pid: %d)\n", pid);
> diff --git a/proc_parse.c b/proc_parse.c
> index e735e13..865bd84 100644
> --- a/proc_parse.c
> +++ b/proc_parse.c
> @@ -178,10 +178,10 @@ int parse_smaps(pid_t pid, struct vm_area_list *vma_area_list, bool use_map_file
>  	DIR *map_files_dir = NULL;
>  	FILE *smaps = NULL;
>  
> -	vma_area_list->nr = 0;
> +	vma_area_list->nr_vmas = 0;
>  	vma_area_list->longest = 0;
>  	vma_area_list->priv_size = 0;
> -	INIT_LIST_HEAD(&vma_area_list->h);
> +	INIT_LIST_HEAD(&vma_area_list->vma_list);
>  
>  	smaps = fopen_proc(pid, "smaps");
>  	if (!smaps)
> @@ -345,8 +345,8 @@ int parse_smaps(pid_t pid, struct vm_area_list *vma_area_list, bool use_map_file
>  			vma_area->vma.flags  |= MAP_ANONYMOUS;
>  		}
>  done:
> -		list_add_tail(&vma_area->list, &vma_area_list->h);
> -		vma_area_list->nr++;
> +		list_add_tail(&vma_area->list, &vma_area_list->vma_list);
> +		vma_area_list->nr_vmas++;
>  		if (privately_dump_vma(vma_area)) {
>  			unsigned long pages;
>  
> 




More information about the CRIU mailing list