Re: Ensuring c/r maintainability (WAS Re: [RFC][PATCH 00/11] track files for checkpointability)

Serge E. Hallyn
2009-03-13T17:53:19Z

Quoting Matt Helsley (matthltc@us.ibm.com):
> On Thu, Mar 12, 2009 at 10:30:48AM -0500, Serge E. Hallyn wrote:
> > Quoting Cedric Le Goater (legoater@free.fr):
> > > >> And if Ingo's requirement is fulfilled, would any C/R patchset be acceptable ?
> > > >
> > > > Yup, no matter how hideous :) Ok not really.
> > > >
> > > > But the point was that it wasn't Dave not understanding Alexey's
> > > > suggestion, but Greg not understanding Ingo's. If you think Ingo's
> > > > goal isn't worthwhile or achievable, then argue that (as I am), don't
> > > > keep elaborating on something we all agree will be needed (Alexey's
> > > > suggestion or some other way of doing a true may-be-checkpointed test).
> > >
> > > I rather spend my time on enabling things rather than forbid them.
> >
> > That sure sounds productive. How could I argue with that.
> >
> > But wait, haven't several teams been doing that for years? So why is
> > c/r not in the upstream kernel? Could it be that ignoring the
> > upstream maintainers' concerns about (a) treating the feature as a
> > toy, (b) long-term maintainability, and (c) c/r becoming an impediment
> > to future features, and instead hacking away at our toy feature, is
> > *not* always the best course?
>
> I've been thinking about how we could make checkpoint/restart (c/r) more
> maintainable in the long-term. I've only come up with two ideas:
>
> I. Implement sparse-like __cr struct annotations for some compile-time checking.
>
> First we annotate structures which c/r needs to save. For example we might have:
>
> struct mm_struct {
> __cr struct vm_area_struct * mmap;
> struct rb_root mm_rb;
> struct vm_area_struct *mmap_cache;
> ...
> __cr unsigned long mmap_base;
> __cr unsigned long task_size;
> ..
> };
>
> The __cr annotations indicate fields of the mm_struct which must be
> saved during checkpoint restart. In fact, for non-pointer fields these
> annotations would be sufficient to generate c/r code.
>
> Next we would need a __cr_root annotation. These mark structures which
> the c/r code visits that determine the scope of c/r. If there is no path from a
> __cr annotation to a __cr_root annotation then we would conclude that c/r of
> this struct is broken. These path constraint checks could be done at compile
> time.

Hi Matt,

is what you're detecting here really something we're worried about?

Maybe that's something we should be doing - coming up with a list of
the things we are trying to detect or prevent. I can only think of
a few offhand:

1. checkpoint (and restart) a task which has used a resource which we
do cannot (yet, or ever) safely checkpoint/restart.

2. kernel has a new feature for which we have not considered
checkpoint/restart. Not only is it not safe to c/r a task using it,
but we haven't even implemented a check for tasks using it.

3. Some new kernel feature has an attribute which simply must be
stored away. An example would be the vdso_base in s390 as of
recent 2.6.29 rc's, which was not present in 2.6.28. So there are
two things to worry about in this one:

a. detect that this happened and handle it, so c/r continues
to work.
b. figure out a way to restart an older c/r image on a newer
kernel - or simply detect older images and call them
incompatible.

-serge