Quoting Nathan Lynch (ntl@xxxxxxxxx):
> On Wed, 25 Feb 2009 13:12:08 -0500
> Dan Smith <danms@xxxxxxxxxx> wrote:
>
> > As suggested by Dave, this provides us a way to make the copy-in and
> > copy-out processes symmetric. I also added CR_COPY_BIT() to use with
> > the s390 bitfields, since we can't memcpy() those.
> >
> > Changelog:
> > Feb 25:
> > . Changed WARN_ON() to BUILD_BUG_ON()
> >
> > Signed-off-by: Dan Smith <danms@xxxxxxxxxx>
> > ---
> > include/linux/checkpoint.h | 20 ++++++++++++++++++++
> > 1 files changed, 20 insertions(+), 0 deletions(-)
> >
> > diff --git a/include/linux/checkpoint.h b/include/linux/checkpoint.h
> > index 217cf6e..3add90e 100644
> > --- a/include/linux/checkpoint.h
> > +++ b/include/linux/checkpoint.h
> > @@ -149,4 +149,24 @@ static inline void process_deny_checkpointing(struct task_struct *task) {}
> >
> > #endif
> >
> > +#define CR_CPT 1
> > +#define CR_RST 2
> > +
> > +#define CR_COPY(op, a, b) \
> > + do { \
> > + BUILD_BUG_ON(sizeof(a) != sizeof(b)); \
> > + if (op == CR_CPT) \
> > + memcpy(&a, &b, sizeof(a)); \
> > + else \
> > + memcpy(&b, &a, sizeof(a)); \
> > + } while (0);
> > +
> > +#define CR_COPY_BIT(op, a, b) \
> > + do { \
> > + if (op == CR_CPT) \
> > + a = b; \
> > + else \
> > + b = a; \
> > + } while (0);
> > +
> > #endif /* _CHECKPOINT_CKPT_H_ */
>
> No typechecking. Multiple expansion of arguments (probably harmless
> for the current use case, but still). Generates a memcpy where,
> depending on the arguments, simple assignment would be sufficient and
well is there any case where we must do a memcpy? Or, if the headers
in the checkpoint image also give a bounded array size, can we just
copy that array using 'a = b'?
In any case we guarantee the sizes are correct, which may be all that
we want anyway.
> preferred. Not useful for targets where fields in the checkpoint image
> are larger than the register state they reflect (32-bit variants of x86
> and powerpc).
>
> Anyway, checkpoint and restart should not be "symmetric" -- the restart
> code has to validate certain values, such as privileged registers, in
> the image before committing them.
The argument in favor is that using the CR_COPY() macros in a single fn
for the common code allows us to
1. make sure that none of the common code is accidentally
overlooked (which does in fact make it more
maintainable)
2. lets any code which must be specially done at checkpoint
or restart stand out. Like resetting access registers
in Dan's version.
> And while these macros may reduce initial development effort, the
> reading effort incurred is non-trivial. It doesn't seem worth it.
-serge
_______________________________________________
Containers mailing list
Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/containers
