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

[Tycho Andersen]

On Wed, Feb 10, 2016 at 06:02:37PM +0300, Pavel Emelyanov wrote:
> 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()

Assuming you mean rst_mem_align() here right at the end, I understand.
I'll send a patch shortly.

Tycho