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

[Matt Helsley]

On Mon, Mar 01, 2010 at 08:58:13AM -0600, Serge E. Hallyn wrote:
> 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.

No, since it's coming directly from the image during restart. It'd be
a BUG_ON() userspace could trigger as often as it likes. If it's
guaranteed to be caught earlier then isn't the traditional refrain:
"why recheck here"?

Cheers,
	-Matt Helsley
_______________________________________________
Containers mailing list
Containers at lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers