[CRIU] [PATCH 08/13] vdso: Remap runtime vdso copy to safe place

[Cyrill Gorcunov]

On Thu, May 23, 2013 at 03:09:33PM +0400, Pavel Emelyanov wrote:
> On 05/22/2013 11:09 PM, Cyrill Gorcunov wrote:
> > 
> > Runtime vdso need to be kept in some safe place when all
> > self-vmas are unmapped. So we reserve space for it in restorer
> > blob area and then remap it into. It's quite important to do
> > a remap here rather than data copy because otherwise pfn
> > of vdso disappear and in future we won't be able to detect
> > vdso are on dumping stage.
> > 
> > Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
> > ---
> >  cr-restore.c       | 35 ++++++++++++++++++++++++++++++++---
> >  include/restorer.h |  5 +++++
> >  include/vdso.h     |  2 ++
> >  pie/restorer.c     | 18 ++++++++++++++++++
> >  pie/vdso.c         | 17 +++++++++++++++++
> >  vdso.c             |  5 +++++
> >  6 files changed, 79 insertions(+), 3 deletions(-)
> > 
> 
> 
> This looks nice, but I don't see much sense in writing _plain_ wrapper
> over memcpy and sys_mremap. Just call them directly.

Look, vdso_remap makes unified way for debug output, iow

	pr_debug("Remap %s %lx -> %lx\n", who, from, to);

and it checks for error as well, which allows me to shrink
the caller code to

+			if (vdso_remap("rt-vdso", vma_entry->start,
+				       args->vdso_rt_parked_at,
+				       symtable_vma_size(&args->vdso_sym_rt)))
+				goto core_restore_end;

but what is more important next patches uses vdso_remap too, and then
I simply write

+			if (vdso_proxify("left dumpee", &args->vdso_sym_rt,
+					 vma_entry, args->vdso_rt_parked_at))
+				goto core_restore_end;

+			if (vdso_proxify("right dumpee", &args->vdso_sym_rt,
+					 vma_entry, args->vdso_rt_parked_at))
+				goto core_restore_end;

where vdso_proxify calls for

+		if (vdso_remap(who, vdso_rt_parked_at,
+			       vma->start, size))
+			return -1;


Thus I would like to save vdso_remap.

As to memcpy -- I'll export sym_rt variable from vdso engine
and drop memcpy then.

> > +	vdso_copy_rt_symbols(&vdso_sym_rt);
> 
> No need in copying symbols on stack and then on arguments. Copy them on
> arguments on demand.

OK.

> Plz, follow existing way of filling the bootstrap area.
> 

OK