From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp52.i.mail.ru (smtp52.i.mail.ru [94.100.177.112]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 662E0469710 for ; Wed, 20 May 2020 00:29:49 +0300 (MSK) References: <194d73cbb844795fbe2a9a6599bb684f4e6a0cf2.1589584903.git.v.shpilevoy@tarantool.org> <36fa01d62df9$94ec0250$bec406f0$@tarantool.org> From: Vladislav Shpilevoy Message-ID: Date: Tue, 19 May 2020 23:29:47 +0200 MIME-Version: 1.0 In-Reply-To: <36fa01d62df9$94ec0250$bec406f0$@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH v2 small 1/1] lsregion: introduce aligned alloc List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Timur Safin , tarantool-patches@dev.tarantool.org, alyapunov@tarantool.org, gorcunov@gmail.com 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 > : 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,