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

[Dmitry Safonov]

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().

>
>> diff --git a/criu/pstree.c b/criu/pstree.c
>> index 6daf4069a..6d980c461 100644
>> --- a/criu/pstree.c
>> +++ b/criu/pstree.c
>> @@ -221,6 +221,7 @@ struct pstree_item *__alloc_pstree_item(bool rst)
>>       item->born_sid = -1;
>>       futex_init(&item->task_st);
>>       item->pid->item = item;
>> +     BUG_ON(item & 3); /* Futex should be aligned */
>>
>>       return item;
>>  }
>> .
>>

-- 
             Dmitry