[Tarantool-patches] [PATCH v7 1/2] fiber: set diagnostics at madvise/mprotect failure
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Sat Feb 15 01:27:50 MSK 2020
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
More information about the Tarantool-patches
mailing list