Re: [Tarantool-patches] [PATCH v2 small 1/1] lsregion: introduce aligned alloc

["Timur Safin" <tsafin@tarantool.org>]

Now, I'm happy - LGTM

: -----Original Message-----
: From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
: Sent: Wednesday, May 20, 2020 12:30 AM
: To: Timur Safin <tsafin@tarantool.org>; tarantool-
: patches@dev.tarantool.org; alyapunov@tarantool.org; gorcunov@gmail.com
: Subject: Re: [PATCH v2 small 1/1] lsregion: introduce aligned alloc
: 
: Hi! Thanks for the review!
: 
: On 19/05/2020 18:21, Timur Safin wrote:
: > Other than small note below it's looking great, but please keep in
: mind...
: >
: > : From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
: > : Subject: [PATCH v2 small 1/1] lsregion: introduce aligned alloc
: > :
: >
: >
: > : @@ -32,59 +32,62 @@
: > :  #include "lsregion.h"
: > :
: > :  void *
: > : -lsregion_alloc_slow(struct lsregion *lsregion, size_t size, int64_t
: id)
: > : +lsregion_aligned_reserve_slow(struct lsregion *lsregion, size_t size,
: > : +			      size_t alignment, void **unaligned)
: > :  {
: > : -	struct lslab *slab = NULL;
: > : -	size_t slab_size = lsregion->arena->slab_size;
: > : +	void *pos;
: > : +	struct lslab *slab;
: > : +	struct slab_arena *arena = lsregion->arena;
: > : +	size_t slab_size = arena->slab_size;
: > :
: > :  	/* If there is an existing slab then try to use it. */
: > :  	if (! rlist_empty(&lsregion->slabs.slabs)) {
: > :  		slab = rlist_last_entry(&lsregion->slabs.slabs, struct lslab,
: > :  					next_in_list);
: > :  		assert(slab != NULL);
: > : +		*unaligned = lslab_pos(slab);
: > : +		pos = (void *)small_align((size_t)*unaligned, alignment);
: > : +		if (pos + size <= lslab_end(slab))
: > : +			return pos;
: >
: > Void* pointer arithmetic is non-standard, gcc extension, and I'd
: > avoid to use it in a longer run. There are compilers is which still
: > not support it.
: >
: > Timur
: 
: Yeah, I missed this. Thanks for noticing.
: 
: ====================
: diff --git a/small/lsregion.c b/small/lsregion.c
: index e6e347c..c01885d 100644
: --- a/small/lsregion.c
: +++ b/small/lsregion.c
: @@ -47,7 +47,7 @@ lsregion_aligned_reserve_slow(struct lsregion *lsregion,
: size_t size,
:  		assert(slab != NULL);
:  		*unaligned = lslab_pos(slab);
:  		pos = (void *)small_align((size_t)*unaligned, alignment);
: -		if (pos + size <= lslab_end(slab))
: +		if ((char *)pos + size <= (char *)lslab_end(slab))
:  			return pos;
:  	}
:  	/*
: @@ -88,6 +88,6 @@ lsregion_aligned_reserve_slow(struct lsregion *lsregion,
: size_t size,
:  	}
:  	*unaligned = lslab_pos(slab);
:  	pos = (void *)small_align((size_t)*unaligned, alignment);
: -	assert(pos + size <= lslab_end(slab));
: +	assert((char *)pos + size <= (char *)lslab_end(slab));
:  	return pos;
:  }
: diff --git a/small/lsregion.h b/small/lsregion.h
: index 2fc0888..3b9a8d9 100644
: --- a/small/lsregion.h
: +++ b/small/lsregion.h
: @@ -205,7 +205,7 @@ lsregion_aligned_reserve(struct lsregion *lsregion,
: size_t size,
:  		assert(slab != NULL);
:  		*unaligned = lslab_pos(slab);
:  		void *pos = (void *)small_align((size_t)*unaligned,
: alignment);
: -		if (pos + size <= lslab_end(slab))
: +		if ((char *)pos + size <= (char *)lslab_end(slab))
:  			return pos;
:  	}
:  	return lsregion_aligned_reserve_slow(lsregion, size, alignment,