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

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Thanks for the fixes!

> @@ -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.
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.

>  	if (fiber->stack != NULL) {
>  		VALGRIND_STACK_DEREGISTER(fiber->stack_id);
>  #if ENABLE_ASAN
> @@ -1017,7 +1047,22 @@ fiber_stack_destroy(struct fiber *fiber, struct slab_cache *slabc)
>  			guard = page_align_down(fiber->stack - page_size);
>  		else
>  			guard = page_align_up(fiber->stack + fiber->stack_size);
> -		fiber_mprotect(guard, page_size, PROT_READ | PROT_WRITE);
> +
> +		if (fiber_mprotect(guard, page_size, mprotect_flags) != 0) {
> +			/*
> +			 * FIXME: We need some intelligent handling:
> +			 * say put this slab into a queue and retry
> +			 * to setup the original protection back in
> +			 * background.
> +			 *
> +			 * Note that in case if we're called from
> +			 * fiber_stack_create() the @a mprotect_flags is
> +			 * the same as the slab been created with, so
> +			 * calling mprotect for VMA with same flags
> +			 * won't fail.
> +			 */
> +			diag_log();
> +		}
>  		slab_put(slabc, fiber->stack_slab);
>  	}
>  }
> @@ -1064,6 +1109,11 @@ fiber_stack_create(struct fiber *fiber, struct slab_cache *slabc,
>  						  fiber->stack_size);
>  
>  	if (fiber_mprotect(guard, page_size, PROT_NONE)) {
> +		/*
> +		 * Write an error into the log since a guard
> +		 * page is critical for functionality.
> +		 */
> +		diag_log();
>  		fiber_stack_destroy(fiber, slabc);
>  		return -1;
>  	}