[CRIU] [PATCH] rst alloc: align on reporting cpos too

[Pavel Emelyanov]

On 02/10/2016 05:44 PM, Tycho Andersen wrote:
> On Wed, Feb 10, 2016 at 04:37:59PM +0300, Pavel Emelyanov wrote:
>> On 02/09/2016 08:38 PM, Tycho Andersen wrote:
>>> Since we align in rst_mem_alloc, we should also align when reporting the
>>> current position; if we don't and things get unlucky, we report a different
>>> position than where the pointer is actually allocated, which fucks things
>>> up quite bad :)
>>
>> Applied, thanks.
>>
>> However, if I do this:
>>
>> x = rst_mem_cpos()
>> x2 = rst_mem_alloc_cont()
>>
>> then x will not correspond to x2 since the latter doesn't align what
>> it allocates.
> 
> Oh, yep :(.

But I'm not sure whether anyone uses this. Let me check...

I'm afraid, but it looks like someone does:

* prepare_posix_timers
* prepare_rlimits
* prepare_signals
* ...

:(

>> I've proposed to introduce a call rst_mem_alogn() that would just
>> shift the "free" pointer up to the nearest aligned value. With this
>> any combination would work.
> 
> Not sure I understand the call pattern you're proposing here. Can you
> elaborate? IIUC, the only way to fix this is to add a
> rst_mem_cpos_cont() call that is used with rst_mem_alloc_cont(), and
> then go fix each call site. Maybe that's what you were suggesting and
> I misunderstood :)

Yes, this could be an option, but thus we'd have 4 calls -- rst_mem_alloc,
_cpos and 2 more with _cont suffix. My proposal is to revert all of this
including Andrey's patch and just introduce a _single_ call

rst_mem_align()

which would take struct rst_mem_type *t = &rst_mems[type] and the would
adjust t->free_mem = align(t->free_mem, sizeof(void *)). And that's it.
After this call _alloc() and _cpos() would report "good" and coniciding
values.

For places that need aligned pointers (that currently just call _alloc())
we'll have to precede them with calls to rst_mem_alloc()

> Tycho
> 
>> -- Pavel
>>
>>> Closes #111
>>>
>>> Signed-off-by: Tycho Andersen <tycho.andersen at canonical.com>
>>> ---
>>>  rst-malloc.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/rst-malloc.c b/rst-malloc.c
>>> index 3556980..8e17255 100644
>>> --- a/rst-malloc.c
>>> +++ b/rst-malloc.c
>>> @@ -128,7 +128,7 @@ unsigned long rst_mem_cpos(int type)
>>>  {
>>>  	struct rst_mem_type_s *t = &rst_mems[type];
>>>  	BUG_ON(!t->remapable || !t->enabled);
>>> -	return t->free_mem - t->buf;
>>> +	return ((void*) round_up((unsigned long)t->free_mem, sizeof(void *))) - t->buf;
>>>  }
>>>  
>>>  void *rst_mem_remap_ptr(unsigned long pos, int type)
>>>
>>
> .
>