[CRIU] [PATCH 08/14] restore: Reserve space in restorer for FPU frame

Cyrill Gorcunov gorcunov at openvz.org
Wed Dec 19 16:04:07 EST 2012


On Thu, Dec 20, 2012 at 12:54:25AM +0400, Pavel Emelyanov wrote:
> On 12/20/2012 12:40 AM, Cyrill Gorcunov wrote:
> > On Thu, Dec 20, 2012 at 12:26:31AM +0400, Pavel Emelyanov wrote:
> >> On 12/19/2012 08:31 PM, Cyrill Gorcunov wrote:
> >>>
> >>> For code simplicity we reserve the maximum size which
> >>> might be needed to form an FPU frame (ie for both
> >>> xsave and fxsave operations).
> >>>
> >>> Signed-off-by: Cyrill Gorcunov <gorcunov at openvz.org>
> >>> ---
> >>>  include/restorer.h | 6 ++++++
> >>>  1 file changed, 6 insertions(+)
> >>>
> >>
> >>
> >>> +
> >>> +	bool				has_fpu;
> >>> +	union {
> >>> +		struct xsave_struct	xsave;
> >>> +		unsigned char		__pad[sizeof(struct xsave_struct) + FP_XSTATE_MAGIC2_SIZE];
> >>
> >> What is this padding for?
> > 
> > We need 4 bytes to be allocated right after xsave,
> 
> Why?
> 
> > and technically it's better
> > to refer to it as singe block in structure, ie union.
> 
> Why is struct + unsigned worse?

Because compiler _might_ insert a hole between members of structure.
It;s not what I see at moment on all compilers I've had access to,
but better to be on safe side, since this entries are quite sensible.
Also note that our thread_restore_args is not _packed_ structure
but just _aligned_. If I chage it to

	struct xsave_struct     xsave;
	u8			magic2[FP_XSTATE_MAGIC2_SIZE];

this will work (I guess) but this hides fact that magic _must_
be right after xsave structure.

Of course I could have introduced another structure as a container
but this would be overkill.

	Cyrill


More information about the CRIU mailing list