From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp59.i.mail.ru (smtp59.i.mail.ru [217.69.128.39]) (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 6E54D469719 for ; Sat, 15 Feb 2020 01:27:52 +0300 (MSK) References: <20200213205618.7982-1-gorcunov@gmail.com> <20200213205618.7982-2-gorcunov@gmail.com> <46c82f5e-5e4c-ec4a-b8ce-b999e6a6230c@tarantool.org> <20200214075416.GS21061@uranus> From: Vladislav Shpilevoy Message-ID: <510f66ef-182f-36dd-2e18-64562bf38a86@tarantool.org> Date: Fri, 14 Feb 2020 23:27:50 +0100 MIME-Version: 1.0 In-Reply-To: <20200214075416.GS21061@uranus> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH v7 1/2] fiber: set diagnostics at madvise/mprotect failure List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cyrill Gorcunov Cc: tml Thanks for the answers! >>> @@ -1007,6 +1035,8 @@ fiber_stack_watermark_create(struct fiber *fiber) >>> static void >>> fiber_stack_destroy(struct fiber *fiber, struct slab_cache *slabc) >>> { >>> + static const int mprotect_flags = PROT_READ | PROT_WRITE; >>> + >> >> Why is it static? From what I know, when it is static we don't >> have a guarantee, that it won't occupy memory in the .data section. > > it is not about memory occupation but rather init it once instead > of doing so during function prologue. It actually depends on compiler > and modern ones would simply optimize this assignment. Sorry, I don't understand. What do you mean init it only once? PROT_READ | PROT_WRITE is a constant. There is nothing to init or to calculate. This is just a one constant integer, which does not need to be saved into a variable before being passed to a function, especially into a static variable. > Actually there is one hidden idea -- R|W is initial flags the 'small' > uses when allocates memory that's why I mention this variable in > comment. You don't really need a static variable to mention its value in a comment. Just say that you use READ|WRITE because this is what usually can be done with normal memory. Otherwise I am missing something. >> Even though here it is clearly not necessary. Wouldn't just const >> be enough? Why was not it possible to leave these flags inlined in >> fiber_mprotect() call? They are not used anywhere else. PROT_NONE, >> for example, is left inlined. > > PROT_NONE is special, it is unrelated to 'small'. Strictly speaking, core/fiber.c can't assume anything about small internals, including READ|WRITE flags. Because fiber is not a part of small. But I propose not to overcomplicate this. > Cyrill