[CRIU] [PATCH] pstree: Aligned pstree item allocation

[Pavel Emelyanov]

On 02/01/2017 05:27 PM, Dmitry Safonov wrote:
> 2017-02-01 17:23 GMT+03:00 Pavel Emelyanov <xemul at virtuozzo.com>:
>> On 02/01/2017 05:15 PM, Kirill Tkhai wrote:
>>> On 01.02.2017 17:12, Pavel Emelyanov wrote:
>>>> 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 at 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.
>>>
>>> It's a reminder, that the futex must be aligned. The note against
>>> unaligned *pstree_item* allocation. Maybe, for the time, when someone
>>> decides to change pstree_item allocation.
>>>
>>> Hm, maybe it'd be better to add something like this instead of the patch?
>>
>> No, let's pick the __aligned() macro from kernel and mark futex-es with it.
> 
> Hmm, I think the idea with BUG_ON() is not that bad:
> AFAIU, the problem is not in the structure's member alignment, but
> in the alignment of the whole structure after dynamic allocation.
> So, BUG_ON() clearly stands to guard it at least in __alloc_pstree_item().

This is typically solved by introducing the MEMALLOC_ALIGN flag and manual
alignment of the return pointer from allocators.

-- Pavel