| Message ID | 148561895682.19279.17651348327492043847.stgit@localhost.localdomain |
|---|---|
| State | Superseded |
| Series | "pstree: Aligned pstree item allocation" |
| Headers | show |
diff --git a/criu/include/pstree.h b/criu/include/pstree.h index b4dc6b635..ee46a14db 100644 --- a/criu/include/pstree.h +++ b/criu/include/pstree.h @@ -12,6 +12,11 @@ */ #define INIT_PID (1) struct pstree_item { + union { + /* Must be 4 bytes aligned */ + futex_t task_st; + unsigned long task_st_le_bits; + }; struct pstree_item *parent; struct list_head children; /* list of my children */ struct list_head sibling; /* linkage in my parent's children list */ @@ -25,10 +30,6 @@ struct pstree_item { struct pid *threads; /* array of threads */ CoreEntry **core; TaskKobjIdsEntry *ids; - union { - futex_t task_st; - unsigned long task_st_le_bits; - }; }; enum { diff --git a/criu/pstree.c b/criu/pstree.c index 8eeea947c..bee9c8917 100644 --- a/criu/pstree.c +++ b/criu/pstree.c @@ -203,6 +203,7 @@ struct pstree_item *__alloc_pstree_item(bool rst) return NULL; } else { sz = sizeof(*item) + sizeof(struct rst_info); + /* Guarantees sizeof(void *) alignment */ item = shmalloc(sz); if (!item) return NULL;
On Sat, Jan 28, 2017 at 06:56:21PM +0300, Kirill Tkhai wrote: > According to man futex(2): > > "On all platforms, futexes are four-byte integers > that must be aligned on a four-byte boundary". > > So, allocate them aligned. Why is it not aligned without this patch? > > Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com> > --- > criu/include/pstree.h | 9 +++++---- > criu/pstree.c | 1 + > 2 files changed, 6 insertions(+), 4 deletions(-) > > diff --git a/criu/include/pstree.h b/criu/include/pstree.h > index b4dc6b635..ee46a14db 100644 > --- a/criu/include/pstree.h > +++ b/criu/include/pstree.h > @@ -12,6 +12,11 @@ > */ > #define INIT_PID (1) > struct pstree_item { > + union { > + /* Must be 4 bytes aligned */ > + futex_t task_st; > + unsigned long task_st_le_bits; > + }; > struct pstree_item *parent; > struct list_head children; /* list of my children */ > struct list_head sibling; /* linkage in my parent's children list */ > @@ -25,10 +30,6 @@ struct pstree_item { > struct pid *threads; /* array of threads */ > CoreEntry **core; > TaskKobjIdsEntry *ids; > - union { > - futex_t task_st; > - unsigned long task_st_le_bits; > - }; > }; > > enum { > diff --git a/criu/pstree.c b/criu/pstree.c > index 8eeea947c..bee9c8917 100644 > --- a/criu/pstree.c > +++ b/criu/pstree.c > @@ -203,6 +203,7 @@ struct pstree_item *__alloc_pstree_item(bool rst) > return NULL; > } else { > sz = sizeof(*item) + sizeof(struct rst_info); > + /* Guarantees sizeof(void *) alignment */ > item = shmalloc(sz); > if (!item) > return NULL; > > _______________________________________________ > CRIU mailing list > CRIU@openvz.org > https://lists.openvz.org/mailman/listinfo/criu
On 30.01.2017 23:45, Andrei Vagin wrote: > On Sat, Jan 28, 2017 at 06:56:21PM +0300, Kirill Tkhai wrote: >> According to man futex(2): >> >> "On all platforms, futexes are four-byte integers >> that must be aligned on a four-byte boundary". >> >> So, allocate them aligned. > > Why is it not aligned without this patch? This patch points that in clear way! >> >> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com> >> --- >> criu/include/pstree.h | 9 +++++---- >> criu/pstree.c | 1 + >> 2 files changed, 6 insertions(+), 4 deletions(-) >> >> diff --git a/criu/include/pstree.h b/criu/include/pstree.h >> index b4dc6b635..ee46a14db 100644 >> --- a/criu/include/pstree.h >> +++ b/criu/include/pstree.h >> @@ -12,6 +12,11 @@ >> */ >> #define INIT_PID (1) >> struct pstree_item { >> + union { >> + /* Must be 4 bytes aligned */ >> + futex_t task_st; >> + unsigned long task_st_le_bits; >> + }; >> struct pstree_item *parent; >> struct list_head children; /* list of my children */ >> struct list_head sibling; /* linkage in my parent's children list */ >> @@ -25,10 +30,6 @@ struct pstree_item { >> struct pid *threads; /* array of threads */ >> CoreEntry **core; >> TaskKobjIdsEntry *ids; >> - union { >> - futex_t task_st; >> - unsigned long task_st_le_bits; >> - }; >> }; >> >> enum { >> diff --git a/criu/pstree.c b/criu/pstree.c >> index 8eeea947c..bee9c8917 100644 >> --- a/criu/pstree.c >> +++ b/criu/pstree.c >> @@ -203,6 +203,7 @@ struct pstree_item *__alloc_pstree_item(bool rst) >> return NULL; >> } else { >> sz = sizeof(*item) + sizeof(struct rst_info); >> + /* Guarantees sizeof(void *) alignment */ >> item = shmalloc(sz); >> if (!item) >> return NULL; >> >> _______________________________________________ >> CRIU mailing list >> CRIU@openvz.org >> https://lists.openvz.org/mailman/listinfo/criu
On Tue, Jan 31, 2017 at 11:58:13AM +0300, Kirill Tkhai wrote: > On 30.01.2017 23:45, Andrei Vagin wrote: > > On Sat, Jan 28, 2017 at 06:56:21PM +0300, Kirill Tkhai wrote: > >> According to man futex(2): > >> > >> "On all platforms, futexes are four-byte integers > >> that must be aligned on a four-byte boundary". > >> > >> So, allocate them aligned. > > > > Why is it not aligned without this patch? > > This patch points that in clear way! The C Standard guarantees that all fields in structures aligned, doesn't it? I don't have objections about this patch, just to know that I don't skip something. > > >> > >> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com> > >> --- > >> criu/include/pstree.h | 9 +++++---- > >> criu/pstree.c | 1 + > >> 2 files changed, 6 insertions(+), 4 deletions(-) > >> > >> diff --git a/criu/include/pstree.h b/criu/include/pstree.h > >> index b4dc6b635..ee46a14db 100644 > >> --- a/criu/include/pstree.h > >> +++ b/criu/include/pstree.h > >> @@ -12,6 +12,11 @@ > >> */ > >> #define INIT_PID (1) > >> struct pstree_item { > >> + union { > >> + /* Must be 4 bytes aligned */ > >> + futex_t task_st; > >> + unsigned long task_st_le_bits; > >> + }; > >> struct pstree_item *parent; > >> struct list_head children; /* list of my children */ > >> struct list_head sibling; /* linkage in my parent's children list */ > >> @@ -25,10 +30,6 @@ struct pstree_item { > >> struct pid *threads; /* array of threads */ > >> CoreEntry **core; > >> TaskKobjIdsEntry *ids; > >> - union { > >> - futex_t task_st; > >> - unsigned long task_st_le_bits; > >> - }; > >> }; > >> > >> enum { > >> diff --git a/criu/pstree.c b/criu/pstree.c > >> index 8eeea947c..bee9c8917 100644 > >> --- a/criu/pstree.c > >> +++ b/criu/pstree.c > >> @@ -203,6 +203,7 @@ struct pstree_item *__alloc_pstree_item(bool rst) > >> return NULL; > >> } else { > >> sz = sizeof(*item) + sizeof(struct rst_info); > >> + /* Guarantees sizeof(void *) alignment */ > >> item = shmalloc(sz); > >> if (!item) > >> return NULL; > >> > >> _______________________________________________ > >> CRIU mailing list > >> CRIU@openvz.org > >> https://lists.openvz.org/mailman/listinfo/criu
On 01/28/2017 06:56 PM, Kirill Tkhai wrote: > According to man futex(2): > > "On all platforms, futexes are four-byte integers > that must be aligned on a four-byte boundary". > > So, allocate them aligned. > > Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com> > --- > criu/include/pstree.h | 9 +++++---- > criu/pstree.c | 1 + > 2 files changed, 6 insertions(+), 4 deletions(-) > > diff --git a/criu/include/pstree.h b/criu/include/pstree.h > index b4dc6b635..ee46a14db 100644 > --- a/criu/include/pstree.h > +++ b/criu/include/pstree.h > @@ -12,6 +12,11 @@ > */ > #define INIT_PID (1) > struct pstree_item { > + union { > + /* Must be 4 bytes aligned */ > + futex_t task_st; > + unsigned long task_st_le_bits; > + }; Ugh... Can't we make this more kernel-style? With the gcc attribute. > struct pstree_item *parent; > struct list_head children; /* list of my children */ > struct list_head sibling; /* linkage in my parent's children list */ > @@ -25,10 +30,6 @@ struct pstree_item { > struct pid *threads; /* array of threads */ > CoreEntry **core; > TaskKobjIdsEntry *ids; > - union { > - futex_t task_st; > - unsigned long task_st_le_bits; > - }; > }; > > enum { > diff --git a/criu/pstree.c b/criu/pstree.c > index 8eeea947c..bee9c8917 100644 > --- a/criu/pstree.c > +++ b/criu/pstree.c > @@ -203,6 +203,7 @@ struct pstree_item *__alloc_pstree_item(bool rst) > return NULL; > } else { > sz = sizeof(*item) + sizeof(struct rst_info); > + /* Guarantees sizeof(void *) alignment */ > item = shmalloc(sz); > if (!item) > return NULL; > > . >
On 01.02.2017 04:14, Andrei Vagin wrote: > On Tue, Jan 31, 2017 at 11:58:13AM +0300, Kirill Tkhai wrote: >> On 30.01.2017 23:45, Andrei Vagin wrote: >>> On Sat, Jan 28, 2017 at 06:56:21PM +0300, Kirill Tkhai wrote: >>>> According to man futex(2): >>>> >>>> "On all platforms, futexes are four-byte integers >>>> that must be aligned on a four-byte boundary". >>>> >>>> So, allocate them aligned. >>> >>> Why is it not aligned without this patch? >> >> This patch points that in clear way! > > The C Standard guarantees that all fields in structures aligned, doesn't > it? I don't have objections about this patch, just to know that I don't > skip something. You sure, and patch does not change something in this way. But it may be a reminder for a person, who changes allocator. I'm not insist on this. >> >>>> >>>> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com> >>>> --- >>>> criu/include/pstree.h | 9 +++++---- >>>> criu/pstree.c | 1 + >>>> 2 files changed, 6 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/criu/include/pstree.h b/criu/include/pstree.h >>>> index b4dc6b635..ee46a14db 100644 >>>> --- a/criu/include/pstree.h >>>> +++ b/criu/include/pstree.h >>>> @@ -12,6 +12,11 @@ >>>> */ >>>> #define INIT_PID (1) >>>> struct pstree_item { >>>> + union { >>>> + /* Must be 4 bytes aligned */ >>>> + futex_t task_st; >>>> + unsigned long task_st_le_bits; >>>> + }; >>>> struct pstree_item *parent; >>>> struct list_head children; /* list of my children */ >>>> struct list_head sibling; /* linkage in my parent's children list */ >>>> @@ -25,10 +30,6 @@ struct pstree_item { >>>> struct pid *threads; /* array of threads */ >>>> CoreEntry **core; >>>> TaskKobjIdsEntry *ids; >>>> - union { >>>> - futex_t task_st; >>>> - unsigned long task_st_le_bits; >>>> - }; >>>> }; >>>> >>>> enum { >>>> diff --git a/criu/pstree.c b/criu/pstree.c >>>> index 8eeea947c..bee9c8917 100644 >>>> --- a/criu/pstree.c >>>> +++ b/criu/pstree.c >>>> @@ -203,6 +203,7 @@ struct pstree_item *__alloc_pstree_item(bool rst) >>>> return NULL; >>>> } else { >>>> sz = sizeof(*item) + sizeof(struct rst_info); >>>> + /* Guarantees sizeof(void *) alignment */ >>>> item = shmalloc(sz); >>>> if (!item) >>>> return NULL; >>>> >>>> _______________________________________________ >>>> CRIU mailing list >>>> CRIU@openvz.org >>>> https://lists.openvz.org/mailman/listinfo/criu
According to man futex(2): "On all platforms, futexes are four-byte integers that must be aligned on a four-byte boundary". So, allocate them aligned. Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com> --- criu/include/pstree.h | 9 +++++---- criu/pstree.c | 1 + 2 files changed, 6 insertions(+), 4 deletions(-)