Re: [Tarantool-patches] [PATCH v5 2/2] fiber: leak slab if unable to bring prots back

[Alexander Turenko <alexander.turenko@tarantool.org>]

Okay except one style comment.

WBR, Alexander TUrenko.

On Thu, Feb 06, 2020 at 01:06:24AM +0300, Cyrill Gorcunov wrote:
> In case if we unable to revert guard page back to
> read|write we should never use such slab again.
> 
> Initially I thought of just put panic here and
> exit but it is too destructive. I think better
> print an error and continue. If node admin ignore

I agree.

> this message then one moment at future there won't
> be slab left for use and creating new fibers get
> prohibited.
> 
> In future (hopefully near one) we plan to drop
> guard pages to prevent VMA fracturing and use
> stack marks instead.
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
> ---
>  src/lib/core/fiber.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/src/lib/core/fiber.c b/src/lib/core/fiber.c
> index 57e4cb6ef..5298541d4 100644
> --- a/src/lib/core/fiber.c
> +++ b/src/lib/core/fiber.c
> @@ -1055,15 +1055,21 @@ fiber_stack_destroy(struct fiber *fiber, struct slab_cache *slabc)
>  			 * to setup the original protection back in
>  			 * background.
>  			 *
> +			 * For now lets keep such slab referenced and
> +			 * leaked: if mprotect failed we must not allow
> +			 * to reuse such slab with PROT_NONE'ed page
> +			 * inside.
> +			 *
>  			 * Note that in case if we're called from
>  			 * fiber_stack_create() the @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);
> +			say_syserror("fiber: Can't put guard page to slab. "
> +				     "Leak %zu bytes", (size_t)fiber->stack_size);
> +		} else
> +			slab_put(slabc, fiber->stack_slab);

Usual way (AFAIK) is to use braces around both branches or neither of
them. Our code style mentions it, see [1] (from words 'Do not
unnecessarily use braces').

AFAIS there are exceptions across our code, but rules are rules :)

[1]: https://www.tarantool.io/en/doc/1.10/dev_guide/c_style_guide/

>  	}
>  }
>  
> -- 
> 2.20.1
>