From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp57.i.mail.ru (smtp57.i.mail.ru [217.69.128.37]) (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 DA0B7469710 for ; Thu, 28 May 2020 23:42:55 +0300 (MSK) From: "Timur Safin" References: <926949a835f9c4220502bab40186c4f45798c94b.1590622225.git.v.shpilevoy@tarantool.org> In-Reply-To: <926949a835f9c4220502bab40186c4f45798c94b.1590622225.git.v.shpilevoy@tarantool.org> Date: Thu, 28 May 2020 23:42:52 +0300 Message-ID: <04a701d63530$8f1b1f30$ad515d90$@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Content-Language: ru Subject: Re: [Tarantool-patches] [PATCH v2 09/10] port: make port_c_entry not PACKED List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: 'Vladislav Shpilevoy' , tarantool-patches@dev.tarantool.org, alyapunov@tarantool.org, korablev@tarantool.org LGTM! : -----Original Message----- : From: Vladislav Shpilevoy : Sent: Thursday, May 28, 2020 2:32 AM : To: tarantool-patches@dev.tarantool.org; alyapunov@tarantool.org; : korablev@tarantool.org; tsafin@tarantool.org : Subject: [PATCH v2 09/10] port: make port_c_entry not PACKED : : PACKED structures don't have padding between their members and : after the structure (needed to be able to store them in an array). : : Port_c_entry was PACKED, since it does not have padding between : its members anyway, and the padding in the end was not needed, : because these objects are never stored in an array. As a result, : sizeof(port_c_entry) was not aligned too. : : Appeared, that mempool, used to allocate port_c_entry objects, : can't work correctly, when object size is not aligned at least by : 8 bytes. Because mempool does not do any alignment internally, and : uses the free objects as a temporary storage for some metadata, : requiring 8 byte alignment. : : The patch removes PACKED attribute from port_c_entry, so now its : size is aligned by 8 bytes, and mempool works fine. : : Part of #4609 : --- : src/box/lua/schema.lua | 2 +- : src/box/port.h | 10 +--------- : src/lib/core/port.h | 2 +- : 3 files changed, 3 insertions(+), 11 deletions(-) : : diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua : index cdfbb65f7..e6844b45f 100644 : --- a/src/box/lua/schema.lua : +++ b/src/box/lua/schema.lua : @@ -79,7 +79,7 @@ ffi.cdef[[ : box_txn_savepoint_t * : box_txn_savepoint(); : : - struct __attribute__((packed)) port_c_entry { : + struct port_c_entry { : struct port_c_entry *next; : union { : struct tuple *tuple; : diff --git a/src/box/port.h b/src/box/port.h : index 76bb5ed1b..43d0f9deb 100644 : --- a/src/box/port.h : +++ b/src/box/port.h : @@ -106,12 +106,7 @@ void : port_vdbemem_create(struct port *base, struct sql_value *mem, : uint32_t mem_count); : : -/** : - * The struct is PACKED to avoid unnecessary 4 byte padding : - * after mp_size. These objects are never stored on stack, neither : - * allocated as an array, so the padding is not needed. : - */ : -struct PACKED port_c_entry { : +struct port_c_entry { : struct port_c_entry *next; : union { : /** Valid if mp_size is 0. */ : @@ -126,9 +121,6 @@ struct PACKED port_c_entry { : uint32_t mp_size; : }; : : -static_assert(sizeof(struct port_c_entry) == 20, : - "port_c_entry is expected to be perfectly aligned and : packed"); : - : /** : * C port is used by C functions from the public API. They can : * return tuples and arbitrary MessagePack. : diff --git a/src/lib/core/port.h b/src/lib/core/port.h : index bb7787679..5c51f76e1 100644 : --- a/src/lib/core/port.h : +++ b/src/lib/core/port.h : @@ -122,7 +122,7 @@ struct port { : * Implementation dependent content. Needed to declare : * an abstract port instance on stack. : */ : - char pad[52]; : + char pad[60]; : }; : : /** Is not inlined just to be exported. */ : -- : 2.21.1 (Apple Git-122.3)