>> +struct cr_hdr_cpu { >> + __u64 args[1]; SH> Dave wanted this to not be an array, right? I think he was okay with it since it matched what is in the rest of the s390 code. I think that the use of the CR_COPY() macros makes it nicer to have matching types as closely as possible. >> + union { >> + float f; >> + double d; >> + __u64 ui; SH> Since this is a union, and you don't deal with its members but SH> just memcpy it, why not just change it to That's a fair argument, although I was thinking that there was some expectation of being able to include this in userspace at some point to inspect the contents of the CR stream. The #ifdef __KERNEL__ at the top of the file makes me think that's true. If so, does it make sense to leave this as-is for easier inspection of the contents? >> + struct { >> + __u32 fp_hi; >> + __u32 fp_lo; >> + } fp; >> + } fprs[NUM_FPRS]; >> + >> + /* per_struct */ >> + __u64 per_control_regs[3]; SH> I assume Dave still wants you to add a SH> #define PER_NUM_REGS 3 SH> into the arch/s390/include/asm/processor.h or something. Ah, right, I thought I was getting out of fixing that with the CR_COPY() macro, but I forgot about this one :) >> +void cr_s390_mm(int op, struct cr_hdr_mm_context *hh, struct mm_struct *mm) >> +{ >> +#if 0 SH> The comment about why this is ifdefed out for now should stay SH> here. Yep. >> +/* Nothing to do for mm context state */ SH> The above comment is clearly wrong :) Oops :) >> + restore_access_regs(hh->acrs); SH> Just a comment explaining why? But it's *so* obvious, right? <ahem> ... Yeah, agreed :) -- Dan Smith IBM Linux Technology Center email: danms@xxxxxxxxxx _______________________________________________ Containers mailing list Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/containers