[Devel] Re: [PATCH] c/r: enclose arch_setup_additional_pages() call between #ifdefs

[Serge E. Hallyn]

Quoting Oren Laadan (orenl at cs.columbia.edu):
> Invocation of arch_setup_additional_pages() should occur only for
> those architectures that provide it:
> 
> #ifdef ARCH_HAS_SETUP_ADDITIONAL_PAGES
> ...
> #endif
> 
> Signed-off-by: Oren Laadan <orenl at cs.columbia.edu>

Acked-by: Serge Hallyn <serue at us.ibm.com>

(one query below)

> ---
>  mm/mmap.c |   15 +++++++++++++--
>  1 files changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 6aa606a..6aadf2e 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -2394,17 +2394,28 @@ int special_mapping_restore(struct ckpt_ctx *ctx,
>  			    struct mm_struct *mm,
>  			    struct ckpt_hdr_vma *h)
>  {
> +	int ret = 0;
> +
>  	/*
>  	 * FIX:
>  	 * Currently, we only handle VDSO/vsyscall special handling.
>  	 * Even that, is very basic - call arch_setup_additional_pages
>  	 * requiring the same mapping (start address) as before.
>  	 */
> +
> +	if (h->vma_type != CKPT_VMA_VDSO)
> +		return -EINVAL;

Well this really should just be a BUG_ON, right?  Since this only
gets called for the VDSO restore_vma_ops.

> +#ifdef ARCH_HAS_SETUP_ADDITIONAL_PAGES
>  #if defined(CONFIG_X86_64) && defined(CONFIG_COMPAT)

(aside: I would have liked to make this more generic, but for reasons
i never quite tracked down that didn't end up working, and x86 is the
only one which defines compat_arch_setup_additional_pages())

>  	if (test_thread_flag(TIF_IA32))
> -		return syscall32_setup_pages(NULL, h->vm_start, 0);
> +		ret = syscall32_setup_pages(NULL, h->vm_start, 0);
> +	else
> +#endif
> +	ret = arch_setup_additional_pages(NULL, h->vm_start, 0);
>  #endif
> -	return arch_setup_additional_pages(NULL, h->vm_start, 0);
> +
> +	return ret;
>  }
>  #else /* !CONFIG_CHECKPOINT */
>  #define special_mapping_checkpoint NULL
> -- 
> 1.6.3.3
> 
> _______________________________________________
> Containers mailing list
> Containers at lists.linux-foundation.org
> https://lists.linux-foundation.org/mailman/listinfo/containers
_______________________________________________
Containers mailing list
Containers at lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers