[Devel] Re: [RFC v14][PATCH 31/54] powerpc: checkpoint/restart implementation

[Oren Laadan]

Nathan Lynch wrote:
> Hello Oren,
> 
> Oren Laadan <orenl at cs.columbia.edu> writes:
> 
>> From: Nathan Lynch <ntl at pobox.com>
>>
>> Support for checkpointing and restarting GPRs, FPU state, DABR, and
>> Altivec state.
> 
> ...
> 
>> Signed-off-by: Nathan Lynch <ntl at pobox.com>
> 
> ...
> 
>> +/* dump the cpu state and registers of a given task */
>> +int checkpoint_write_cpu(struct ckpt_ctx *ctx, struct task_struct *t)
>> +{
>> +	struct ckpt_hdr_cpu *cpu_hdr;
>> +	int rc;
>> +
>> +	rc = -ENOMEM;
>> +	cpu_hdr = ckpt_hdr_get(ctx, sizeof(*cpu_hdr), CKPT_HDR_CPU);
> 
> This won't build (should be ckpt_hdr_get_type?).
> 
> I didn't write this code (I used kzalloc).
> 
> In the code I did write, I deliberately preferred the slab allocator to
> the checkpoint-specific APIs.  I do not see the advantage of using an
> arbitrarily fixed size special allocation stack that is prone to
> overflow or, worse, data corruption if someone improperly interleaves
> their gets and puts.
> 
> I don't believe you were acting in bad faith here, and I'm not sure
> there's an established etiquette.  But my signed-off-by line is on this
> patch, and I don't think it belongs there unless I've actually written
> the code or agreed to the modifications.
> 
> If you insist on replacing kzalloc with ckpt_hdr_get, then please do so
> in a separate commit with an explanation in the changelog.  I'd have no
> objection to that -- it's your tree, after all.  Or if you want to munge
> my patch in place, just replace my signoff with yours and note "based on
> work by Nathan Lynch" or something.

You are correct. Originally I was unsure how to modify that, and
later in the flood of  other changes I forgot to get back to that
commit message. I apologize for that.

Oren.

_______________________________________________
Containers mailing list
Containers at lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers