[CRIU] [PATCH 1/2] rst-malloc: return aligned pointers to sizeof(void *) (v3)

[Andrew Vagin]

On Wed, Jan 27, 2016 at 04:29:43PM +0300, Pavel Emelyanov wrote:
> On 01/26/2016 10:58 PM, Andrey Vagin wrote:
> > From: Andrew Vagin <avagin at virtuozzo.com>
> > 
> > Stas found that if we don't align a pointer,
> > futex and atomic operations can fail.
> > 
> > v2: don't hard-code the size of void *
> > v3: add a function to allocate memory without gaps with
> >     a privious slice. It's used to allocate arrays.
> > 
> > Cc: Stanislav Kinsburskiy <skinsbursky at virtuozzo.com>
> > Signed-off-by: Andrew Vagin <avagin at virtuozzo.com>
> > ---
> >  include/rst-malloc.h |  1 +
> >  rst-malloc.c         | 23 ++++++++++++++++++++++-
> >  2 files changed, 23 insertions(+), 1 deletion(-)
> > 
> > diff --git a/include/rst-malloc.h b/include/rst-malloc.h
> > index 8c48f27..f205622 100644
> > --- a/include/rst-malloc.h
> > +++ b/include/rst-malloc.h
> > @@ -58,6 +58,7 @@ extern void *rst_mem_remap_ptr(unsigned long pos, int type);
> >   * last object can be freed (pop-ed from buffer).
> >   */
> >  extern void *rst_mem_alloc(unsigned long size, int type);
> > +extern void *rst_mem_alloc_cont(unsigned long size, int type);
> >  extern void rst_mem_free_last(int type);
> >  /*
> >   * Routines to remap SHREMAP and PRIVATE into restorer address space
> > diff --git a/rst-malloc.c b/rst-malloc.c
> > index 14e0b41..7685430 100644
> > --- a/rst-malloc.c
> > +++ b/rst-malloc.c
> > @@ -128,6 +128,9 @@ unsigned long rst_mem_cpos(int type)
> >  {
> >  	struct rst_mem_type_s *t = &rst_mems[type];
> >  	BUG_ON(!t->remapable || !t->enabled);
> > +
> > +	t->free_mem = (void *) round_up((unsigned long)t->free_mem, sizeof(void *));
> 
> Should this hunk be here? Why?

Yes, it should. We want to aligne a start address of arrays. It's about
a case when rst_mem_alloc_cont is used.
> 
> > +
> >  	return t->free_mem - t->buf;
> >  }
> >  
> > @@ -138,7 +141,7 @@ void *rst_mem_remap_ptr(unsigned long pos, int type)
> >  	return t->buf + pos;
> >  }
> >  
> > -void *rst_mem_alloc(unsigned long size, int type)
> > +static void *__rst_mem_alloc(unsigned long size, int type)
> >  {
> >  	struct rst_mem_type_s *t = &rst_mems[type];
> >  	void *ret;
> > @@ -158,6 +161,24 @@ void *rst_mem_alloc(unsigned long size, int type)
> >  	return ret;
> >  }
> >  
> > +void *rst_mem_alloc(unsigned long size, int type)
> > +{
> > +	struct rst_mem_type_s *t = &rst_mems[type];
> > +
> > +	t->free_mem = (void *) round_up((unsigned long)t->free_mem, sizeof(void *));
> 
> Any guarantee you don't overflow the free_mem beyond the allocated memory buffer?

__rst_mem_alloc will check and call grow() if it's required.
> 
> > +
> > +	return __rst_mem_alloc(size, type);
> > +}
> > +
> > +/* Allocate memory without gaps with a previous slice */
> > +void *rst_mem_alloc_cont(unsigned long size, int type)
> 
> I'd better make a call titles rst_mem_alloc_aligned().

rst_mem_alloc_NOT_align

I don't want to rename rst_mem_alloc, because ussualy this sort of
functions (e.g. malloc) returns alligned addresses.

I was thinking about this, but rst_mem_alloc_cont looks better for met.

Maybe we need to add rst_mem_realloc() instead of rst_mem_alloc_cont.
> 
> > +{
> > +	struct rst_mem_type_s *t = &rst_mems[type];
> > +	BUG_ON(!t->remapable);
> > +
> > +	return __rst_mem_alloc(size, type);
> > +}
> > +
> >  void rst_mem_free_last(int type)
> >  {
> >  	struct rst_mem_type_s *t = &rst_mems[type];
> > 
>