[Announce] CRIU 3.5 "Clay Jay" is out

Submitted by Andrey Vagin on Oct. 17, 2017, 7:48 p.m.

Details

Message ID 20171017194825.GC21194@outlook.office365.com
State New
Series "CRIU 3.5 "Clay Jay" is out"
Headers show

Commit Message

Andrey Vagin Oct. 17, 2017, 7:48 p.m.
On Tue, Oct 17, 2017 at 04:05:09PM +0200, Adrian Reber wrote:
> On Tue, Oct 17, 2017 at 04:56:37PM +0300, Pavel Emelyanov wrote:
> > On 10/17/2017 04:36 PM, Adrian Reber wrote:
> > > On Tue, Oct 17, 2017 at 03:44:57PM +0300, Pavel Emelyanov wrote:
> > >>>>> >From the changelog:
> > >>>>>    Use __glibc_reserved1 instead of __reserved.
> > >>>>>
> > >>>>> And it's not a macro, so we cannot check it with #ifdef :\ IIRC we've seen smth like that
> > >>>>> before, again with uc_mcontext.
> > >>>
> > >>> Has this been fixed? I see that you are preparing for the next release
> > >>> and have not seen and follow-up discussion on this.
> > >>
> > >> Andrey, I thought we had patches about it and they've been pushed to master :)
> > >> Have we?
> > > 
> > > There is only this:
> > > 
> > > https://github.com/checkpoint-restore/criu/commit/f41e386d4d40e3e26b0cfdc85a812b7edb337f1d
> > > 
> > > Which is not arch specific. I see no fix for aarch64 on criu-dev.
> > 
> > Was there a patch for it in the ML?
> 
> Not from me, as I am not sure how to properly fix it. It probably needs
> a feature detection macro to select the right structure name. But I was
> not able to get access to the combination of hardware and really new
> glibc.

Could you try out the attached patch?


> 
> 		Adrian

Patch hide | download patch | download mbox

diff --git a/compel/arch/aarch64/src/lib/include/uapi/asm/sigframe.h b/compel/arch/aarch64/src/lib/include/uapi/asm/sigframe.h
index 22246b8..009986d 100644
--- a/compel/arch/aarch64/src/lib/include/uapi/asm/sigframe.h
+++ b/compel/arch/aarch64/src/lib/include/uapi/asm/sigframe.h
@@ -41,10 +41,22 @@  struct rt_sigframe {
 			: "r"(new_sp)						\
 			: "sp", "x8", "memory")
 
+struct cr_sigcontext {
+        __u64 fault_address;
+        /* AArch64 registers */
+        __u64 regs[31];
+        __u64 sp;
+        __u64 pc;
+        __u64 pstate;
+        /* 4K reserved for FP/SIMD state and future expansion */
+        __u8 __reserved[4096] __attribute__((__aligned__(16)));
+};
+
 #define RT_SIGFRAME_UC(rt_sigframe)		(&rt_sigframe->uc)
 #define RT_SIGFRAME_REGIP(rt_sigframe)		((long unsigned int)(rt_sigframe)->uc.uc_mcontext.pc)
 #define RT_SIGFRAME_HAS_FPU(rt_sigframe)	(1)
-#define RT_SIGFRAME_AUX_CONTEXT(rt_sigframe)	((struct aux_context*)&(rt_sigframe)->uc.uc_mcontext.__reserved)
+#define RT_SIGFRAME_SIGCONTEXT(rt_sigframe)	((struct cr_sigcontext *)&(rt_sigframe)->uc.uc_mcontext_
+#define RT_SIGFRAME_AUX_CONTEXT(rt_sigframe)	((struct aux_context*)&(RT_SIGFRAME_SIGCONTEXT(rt_sigframe)->__reserved))
 #define RT_SIGFRAME_FPU(rt_sigframe)		(&RT_SIGFRAME_AUX_CONTEXT(rt_sigframe)->fpsimd)
 #define RT_SIGFRAME_OFFSET(rt_sigframe)		0
 

Comments

Andrey Vagin Oct. 17, 2017, 9:29 p.m.
On Tue, Oct 17, 2017 at 12:48:26PM -0700, Andrey Vagin wrote:
> On Tue, Oct 17, 2017 at 04:05:09PM +0200, Adrian Reber wrote:
> > On Tue, Oct 17, 2017 at 04:56:37PM +0300, Pavel Emelyanov wrote:
> > > On 10/17/2017 04:36 PM, Adrian Reber wrote:
> > > > On Tue, Oct 17, 2017 at 03:44:57PM +0300, Pavel Emelyanov wrote:
> > > >>>>> >From the changelog:
> > > >>>>>    Use __glibc_reserved1 instead of __reserved.
> > > >>>>>
> > > >>>>> And it's not a macro, so we cannot check it with #ifdef :\ IIRC we've seen smth like that
> > > >>>>> before, again with uc_mcontext.
> > > >>>
> > > >>> Has this been fixed? I see that you are preparing for the next release
> > > >>> and have not seen and follow-up discussion on this.
> > > >>
> > > >> Andrey, I thought we had patches about it and they've been pushed to master :)
> > > >> Have we?
> > > > 
> > > > There is only this:
> > > > 
> > > > https://github.com/checkpoint-restore/criu/commit/f41e386d4d40e3e26b0cfdc85a812b7edb337f1d
> > > > 
> > > > Which is not arch specific. I see no fix for aarch64 on criu-dev.
> > > 
> > > Was there a patch for it in the ML?
> > 
> > Not from me, as I am not sure how to properly fix it. It probably needs
> > a feature detection macro to select the right structure name. But I was
> > not able to get access to the combination of hardware and really new
> > glibc.
> 
> Could you try out the attached patch?

Here is a correct patch
https://github.com/avagin/criu/commit/36f3ab45867338e98e43ea5f1f26764ae3946fc2

In this patch, I declare this structure in a criu header. This should be
safe, because it is a part of the kernel API.

> 
> 
> > 
> > 		Adrian

> diff --git a/compel/arch/aarch64/src/lib/include/uapi/asm/sigframe.h b/compel/arch/aarch64/src/lib/include/uapi/asm/sigframe.h
> index 22246b8..009986d 100644
> --- a/compel/arch/aarch64/src/lib/include/uapi/asm/sigframe.h
> +++ b/compel/arch/aarch64/src/lib/include/uapi/asm/sigframe.h
> @@ -41,10 +41,22 @@ struct rt_sigframe {
>  			: "r"(new_sp)						\
>  			: "sp", "x8", "memory")
>  
> +struct cr_sigcontext {
> +        __u64 fault_address;
> +        /* AArch64 registers */
> +        __u64 regs[31];
> +        __u64 sp;
> +        __u64 pc;
> +        __u64 pstate;
> +        /* 4K reserved for FP/SIMD state and future expansion */
> +        __u8 __reserved[4096] __attribute__((__aligned__(16)));
> +};
> +
>  #define RT_SIGFRAME_UC(rt_sigframe)		(&rt_sigframe->uc)
>  #define RT_SIGFRAME_REGIP(rt_sigframe)		((long unsigned int)(rt_sigframe)->uc.uc_mcontext.pc)
>  #define RT_SIGFRAME_HAS_FPU(rt_sigframe)	(1)
> -#define RT_SIGFRAME_AUX_CONTEXT(rt_sigframe)	((struct aux_context*)&(rt_sigframe)->uc.uc_mcontext.__reserved)
> +#define RT_SIGFRAME_SIGCONTEXT(rt_sigframe)	((struct cr_sigcontext *)&(rt_sigframe)->uc.uc_mcontext_
> +#define RT_SIGFRAME_AUX_CONTEXT(rt_sigframe)	((struct aux_context*)&(RT_SIGFRAME_SIGCONTEXT(rt_sigframe)->__reserved))
>  #define RT_SIGFRAME_FPU(rt_sigframe)		(&RT_SIGFRAME_AUX_CONTEXT(rt_sigframe)->fpsimd)
>  #define RT_SIGFRAME_OFFSET(rt_sigframe)		0
>  

> _______________________________________________
> CRIU mailing list
> CRIU@openvz.org
> https://lists.openvz.org/mailman/listinfo/criu
Adrian Reber Oct. 18, 2017, 6:10 a.m.
On Tue, Oct 17, 2017 at 02:29:43PM -0700, Andrey Vagin wrote:
> On Tue, Oct 17, 2017 at 12:48:26PM -0700, Andrey Vagin wrote:
> > On Tue, Oct 17, 2017 at 04:05:09PM +0200, Adrian Reber wrote:
> > > On Tue, Oct 17, 2017 at 04:56:37PM +0300, Pavel Emelyanov wrote:
> > > > On 10/17/2017 04:36 PM, Adrian Reber wrote:
> > > > > On Tue, Oct 17, 2017 at 03:44:57PM +0300, Pavel Emelyanov wrote:
> > > > >>>>> >From the changelog:
> > > > >>>>>    Use __glibc_reserved1 instead of __reserved.
> > > > >>>>>
> > > > >>>>> And it's not a macro, so we cannot check it with #ifdef :\ IIRC we've seen smth like that
> > > > >>>>> before, again with uc_mcontext.
> > > > >>>
> > > > >>> Has this been fixed? I see that you are preparing for the next release
> > > > >>> and have not seen and follow-up discussion on this.
> > > > >>
> > > > >> Andrey, I thought we had patches about it and they've been pushed to master :)
> > > > >> Have we?
> > > > > 
> > > > > There is only this:
> > > > > 
> > > > > https://github.com/checkpoint-restore/criu/commit/f41e386d4d40e3e26b0cfdc85a812b7edb337f1d
> > > > > 
> > > > > Which is not arch specific. I see no fix for aarch64 on criu-dev.
> > > > 
> > > > Was there a patch for it in the ML?
> > > 
> > > Not from me, as I am not sure how to properly fix it. It probably needs
> > > a feature detection macro to select the right structure name. But I was
> > > not able to get access to the combination of hardware and really new
> > > glibc.
> > 
> > Could you try out the attached patch?
> 
> Here is a correct patch
> https://github.com/avagin/criu/commit/36f3ab45867338e98e43ea5f1f26764ae3946fc2
> 
> In this patch, I declare this structure in a criu header. This should be
> safe, because it is a part of the kernel API.

Thanks, with this patch I can build criu 3.5 on Fedora rawhide aarch64
again: https://koji.fedoraproject.org/koji/taskinfo?taskID=22514565

Tested-by: Adrian Reber <areber@redhat.com>

		Adrian