Re: [PATCH 3/3] c/r: define s390-specific checkpoint-restart code (v6)

[Dan Smith <danms-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>]

NL> In the powerpc implementation I was able to get away with
NL> returning zero, without writing dummy headers, for cases like
NL> this.

Right, as did we.  However, mktree.c expects to read a header in this
part of the stream.  The kernel expects what the kernel has written,
which is easy.  However, when writing something that needs to
interpret any platform's stream, I think it's easier if you don't have
a bunch of "optional" headers that you need to test for and maybe
handle (especially in the case of reading the stream over a socket or
the like).

>> +extern void cr_s390_regs(int op, struct cr_hdr_cpu *hh, struct task_struct *t);
>> +extern void cr_s390_mm(int op, struct cr_hdr_mm_context *hh,
>> +		       struct mm_struct *mm);

NL> These belong in a header, please...

Actually, I was hoping that Dave would stir up some conversation about
moving all of this back into a single file since we cut the line count
down so much with our evil macros :)

>> +	regs->psw.addr &= ~PSW_ADDR_INSN;
>> +	cr_s390_regs(CR_RST, hh, current);

NL> The PSW_ADDR_INSN bit in regs->psw.addr is cleared, and then
NL> regs-> psw.addr is overwritten by cr_s390_regs?

Yes, this is broken, thanks.  It is also an example of where the
macros won't work as nicely for us.  This is left over from Serge's
original code, where I believe he was attempting to avoid loading
anything into the PSW other than the instruction pointer bit.  A
trivial change to cr_s390_regs() will correct this.

Thanks!

-- 
Dan Smith
IBM Linux Technology Center
email: danms-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org