Re: [Tarantool-patches] [PATCH v7 1/2] fiber: set diagnostics at madvise/mprotect failure

[Cyrill Gorcunov <gorcunov@gmail.com>]

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.