From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lj1-f193.google.com (mail-lj1-f193.google.com [209.85.208.193]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 019BC469719 for ; Sat, 15 Feb 2020 09:57:59 +0300 (MSK) Received: by mail-lj1-f193.google.com with SMTP id a13so13138278ljm.10 for ; Fri, 14 Feb 2020 22:57:59 -0800 (PST) Date: Sat, 15 Feb 2020 09:57:57 +0300 From: Cyrill Gorcunov Message-ID: <20200215065757.GA2527@uranus> References: <20200213205618.7982-1-gorcunov@gmail.com> <20200213205618.7982-2-gorcunov@gmail.com> <46c82f5e-5e4c-ec4a-b8ce-b999e6a6230c@tarantool.org> <20200214075416.GS21061@uranus> <510f66ef-182f-36dd-2e18-64562bf38a86@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <510f66ef-182f-36dd-2e18-64562bf38a86@tarantool.org> 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: Vladislav Shpilevoy Cc: tml On Fri, Feb 14, 2020 at 11:27:50PM +0100, Vladislav Shpilevoy wrote: > > > > 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. It highly depends on compiler. It might be optimized and not allocated on the stack at all or it might be allocated and loaded in the procedure prologue (which will happen with optimization disabled). > > 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. And I'll have to modify both -- comment and code if we need to change the permissions. Thanks but no. We could do better and keep R|W in one place. But if you bothers this approach I could just inline R|W, no problem. Still there is a really big question remains -- what to do with the issue. I suspect Kostya has something different in mind. So for now I'm giving up cooking this series (until I'll be sure that I really understand the next step to walk). Thanks a huge for all your comments, Vlad! > 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.