Re: [PATCH 1/3] powerpc: bare minimum checkpoint/restart implementation

[Nathan Lynch <ntl@pobox.com>]

On Tue, 24 Feb 2009 13:58:26 -0600
"Serge E. Hallyn" <serue@us.ibm.com> wrote:

> Quoting Nathan Lynch (ntl@pobox.com):
> > Nathan Lynch <ntl@pobox.com> wrote:
> > >
> > > Oren Laadan wrote:
> > > > 
> > > > Nathan Lynch wrote:
> > > > > 
> > > > > What doesn't work:
> > > > > * restarting a 32-bit task from a 64-bit task and vice versa
> > > > 
> > > > Is there a test to bail if we attempt to checkpoint such tasks ?
> > > 
> > > No, but I'll add one if it looks too hard to fix for the next round.
> > 
> > Unfortunately, adding a check for this is hard.
> > 
> > The "point of no return" in the restart path is cr_read_mm, which tears
> > down current's address space.  cr_read_mm runs way before cr_read_cpu,
> > which is the only restart method I've implemented for powerpc so far.
> > So, checking for this condition in cr_read_cpu is too late if I want
> > restart(2) to return an error and leave the caller's memory map
> > intact.  (And I do want this: restart should be as robust as execve.)
> > 
> > Well okay then, cr_read_head_arch seems to be the right place in the
> > restart sequence for the architecture code to handle this.  However,
> > cr_write_head_arch (which produces the buffer that cr_read_head_arch
> > consumes) is not provided a reference to the task to be checkpointed,
> > nor can it assume that it's operating on current.  I need a reference
> > to a task before I can determine whether it's running in 32- or 64-bit
> > mode, or using the FPU, Altivec, SPE, whatever.
> > 
> > In any case, mixing 32- and 64-bit tasks across restart is something I
> > eventually want to support, not reject.  But the problem I've outlined
> > applies to FPU state and vector extensions (VMX, SPE), as well as
> > sanity-checking debug register (DABR) contents.  We'll need to be able
> > to error out gracefully from restart when a checkpoint image specifies a
> > feature unsupported by the current kernel or hardware.  But I don't see
> > how to do it with the current architecture.  Am I missing something?
> 
> I suspect I can guess the response to this suggestion, but how about we
> accept that if sys_restart() fails due to something like this, the
> task is lost and can't exit gracefully?

In the short term it might be necessary.  But the restart code should
forcibly kill the task instead of returning an error back up to
userspace in this case.  Once the memory map of the process has been
altered, there is no point in allowing it to continue (and likely dump
a useless core).  Btw, this failure mode seems to apply when
cr_read_files() fails, too...

But in the long term, things need to be more robust (e.g. restart(2)
returns ENOEXEC without messing with current->mm).  I think it's worth
looking at how execve operates... if I understand correctly, it sets up
a new mm_struct disconnected from the current task and activates it at
the last moment.