Tarantool development patches archive
 help / color / mirror / Atom feed
From: Cyrill Gorcunov <gorcunov@gmail.com>
To: tarantool-patches@freelists.org
Cc: vdavydov.dev@gmail.com, kostja@tarantool.org,
	Cyrill Gorcunov <gorcunov@gmail.com>
Subject: [PATCH 3/3] core/fiber: Put watermarks into stack to track its usage
Date: Wed,  6 Mar 2019 01:38:54 +0300	[thread overview]
Message-ID: <20190305223854.14660-4-gorcunov@gmail.com> (raw)
In-Reply-To: <20190305223854.14660-1-gorcunov@gmail.com>

We want to detect a situation where task in fiber is too eager for
stack and close to its exhausting. For this sake upon stack creation
we put 8 marks on last stack page with step of 128 bytes. Such params
allows us to fill ~1/4 of a page, which does seem reasonable but
we might change this params with time.

Since the watermark position is permanent and some task is close
to stack limit we print about the situation once to not spam
a user much and stop putting the mark on recycling.

Still this techique doesn't guarantee to handle all possible
situations so to increas probability of catching eager fibers
we put marks *not* at page start address but withing some offset,
randomly generated during startup procedure.

To minimize a pressue on memory system we try to relax stack
usage with that named dynamic watermark. Basically it is the
same marks as for overflow detection but placed "earlier" and
on every recycle we try to shrink stack usage moving dynamic
mark closer to stack start dropping clean pages with madvise
help if OS supports it.

Thus the stack will look like (for "growsdown" case)

	+---			<- fiber->stack + fiber->stack_size
	|
	| first stack page
	|
	+---
	|
	~~~~
	|
	+---
	| ###			<- fiber->stack_dynamic_wmark
	|
	|
	+---
	| ###			<- fiber->stack_overflow_wmark
	| last usabe page
	|
	+---
	| guard page
	+---

Closes #3418
---
v3:
 - add random offset from beginning of the page to
   put mark not at predefined address but by chance
 - drop freeze/frozen macros just use NULL
 - guard with TARGET_OS_DARWIN where needed

 src/lib/core/fiber.c | 185 +++++++++++++++++++++++++++++++++++++++++++
 src/lib/core/fiber.h |   4 +
 2 files changed, 189 insertions(+)

diff --git a/src/lib/core/fiber.c b/src/lib/core/fiber.c
index 64bcda26a..d18227e7f 100644
--- a/src/lib/core/fiber.c
+++ b/src/lib/core/fiber.c
@@ -41,6 +41,7 @@
 #include "assoc.h"
 #include "memory.h"
 #include "trigger.h"
+#include "random.h"
 
 #include "third_party/valgrind/memcheck.h"
 
@@ -104,6 +105,32 @@ static const struct fiber_attr fiber_attr_default = {
        .flags = FIBER_DEFAULT_FLAGS
 };
 
+/*
+ * Random values generated with uuid.
+ */
+static const uint64_t poison_pool[] = {
+	0x74f31d37285c4c37, 0xb10269a05bf10c29,
+	0x0994d845bd284e0f, 0x9ffd4f7129c184df,
+	0x357151e6711c4415, 0x8c5e5f41aafe6f28,
+	0x6917dd79e78049d5, 0xba61957c65ca2465,
+};
+
+/*
+ * An offset inside page to the first mark.
+ */
+static unsigned int wmark_inpage_offset;
+
+/*
+ * We poison by 8 bytes as it natural for stack
+ * step on x86-64. Also 128 byte gap between
+ * poison values should cover a common cases.
+ */
+#define POISON_SIZE	(sizeof(poison_pool) / sizeof(poison_pool[0]))
+#define POISON_GAP	(128 + sizeof(poison_pool[0]))
+#define POISON_OFF	(POISON_GAP / sizeof(poison_pool[0]))
+
+static void fiber_wmark_recycle(struct fiber *fiber);
+
 void
 fiber_attr_create(struct fiber_attr *fiber_attr)
 {
@@ -624,6 +651,7 @@ fiber_recycle(struct fiber *fiber)
 	/* no pending wakeup */
 	assert(rlist_empty(&fiber->state));
 	bool has_custom_stack = fiber->flags & FIBER_CUSTOM_STACK;
+	fiber_wmark_recycle(fiber);
 	fiber_reset(fiber);
 	fiber->name[0] = '\0';
 	fiber->f = NULL;
@@ -710,6 +738,157 @@ page_align_up(void *ptr)
 	return page_align_down(ptr + page_size - 1);
 }
 
+static bool
+stack_has_wmark(void *addr)
+{
+	const uint64_t *src = poison_pool;
+	const uint64_t *dst = addr;
+	size_t i;
+
+	for (i = 0; i < POISON_SIZE; i++) {
+		if (*dst != src[i])
+			return false;
+		dst += POISON_OFF;
+	}
+
+	return true;
+}
+
+static void
+stack_put_wmark(void *addr)
+{
+	const uint64_t *src = poison_pool;
+	uint64_t *dst = addr;
+	size_t i;
+
+	for (i = 0; i < POISON_SIZE; i++) {
+		*dst = src[i];
+		dst += POISON_OFF;
+	}
+}
+
+#ifndef TARGET_OS_DARWIN
+static void
+stack_shrink(struct fiber *fiber)
+{
+	void *hi, *lo;
+
+	if (stack_direction < 0) {
+		hi = fiber->stack + fiber->stack_size;
+		lo = fiber->stack_overflow_wmark+ page_size;
+	} else {
+		hi = fiber->stack_overflow_wmark - page_size;
+		lo = fiber->stack - fiber->stack_size;
+	}
+
+	if (fiber->stack_dynamic_wmark <= lo ||
+	    fiber->stack_dynamic_wmark >= hi)
+		return;
+
+	if (stack_direction < 0) {
+		madvise(page_align_up(fiber->stack_dynamic_wmark),
+			page_size, MADV_DONTNEED);
+		fiber->stack_dynamic_wmark += page_size;
+	} else {
+		madvise(page_align_down(fiber->stack_dynamic_wmark),
+			page_size, MADV_DONTNEED);
+		fiber->stack_dynamic_wmark -= page_size;
+	}
+	stack_put_wmark(fiber);
+}
+#endif
+
+static void
+fiber_wmark_recycle(struct fiber *fiber)
+{
+	static bool overflow_warned = false;
+
+	if (fiber->stack == NULL || fiber->flags & FIBER_CUSTOM_STACK)
+		return;
+
+#ifndef TARGET_OS_DARWIN
+	/*
+	 * On recycle we're trying to shrink stack
+	 * as much as we can until first mark overwrite
+	 * detected, then we simply zap watermark and
+	 * assume the stack is balanced and won't change
+	 * much in future.
+	 */
+	if (fiber->stack_dynamic_wmark != NULL) {
+		if (!stack_has_wmark(fiber->stack_dynamic_wmark))
+			fiber->stack_dynamic_wmark = NULL;
+		else
+			stack_shrink(fiber);
+	}
+#endif
+
+	/*
+	 * We are watching for stack overflow in one shot way:
+	 * simply to not spam a user with messages, if someone
+	 * triggered the problem it is highly likely that another
+	 * fiber hit the same.
+	 */
+	if (overflow_warned)
+		return;
+
+	if (!stack_has_wmark(fiber->stack_overflow_wmark)) {
+		say_warn("fiber %d seems to overflow the stack soon",
+			 fiber->name, fiber->fid);
+		overflow_warned = true;
+	}
+}
+
+static void
+fiber_wmark_init(struct fiber *fiber)
+{
+	/*
+	 * No tracking on custom stacks
+	 * in a sake of simplicity.
+	 */
+	if (fiber->flags & FIBER_CUSTOM_STACK) {
+		fiber->stack_overflow_wmark = NULL;
+		fiber->stack_dynamic_wmark = NULL;
+		return;
+	}
+
+	/*
+	 * Initially we arm the last page of the stack
+	 * to catch if we're getting close to its exhausting.
+	 * In turn the dynamic mark is put into next page so
+	 * we could drop pages later if they are unused.
+	 */
+	if (stack_direction < 0) {
+		fiber->stack_overflow_wmark  = fiber->stack;
+		fiber->stack_overflow_wmark += wmark_inpage_offset;
+
+		fiber->stack_dynamic_wmark = fiber->stack_overflow_wmark + page_size;
+	} else {
+		fiber->stack_overflow_wmark  = fiber->stack + fiber->stack_size;
+		fiber->stack_overflow_wmark -= page_size;
+		fiber->stack_overflow_wmark -= wmark_inpage_offset;
+
+		fiber->stack_dynamic_wmark = fiber->stack_overflow_wmark - page_size;
+	}
+	stack_put_wmark(fiber->stack_overflow_wmark);
+}
+
+static void
+stack_wmark_init(void)
+{
+	uint16_t v;
+
+	/*
+	 * To increase probability of the stack overflow
+	 * detection we put _first_ mark at random position
+	 * in first 128 bytes range. The rest of the marks
+	 * are put with constant step (because we should not
+	 * pressue random generator much in case of hight
+	 * number of fibers).
+	 */
+	random_bytes((void *)&v, sizeof(v));
+	wmark_inpage_offset = ((v % 128) + 8) & ~7;
+}
+
 static int
 fiber_stack_create(struct fiber *fiber, size_t stack_size)
 {
@@ -757,6 +936,7 @@ fiber_stack_create(struct fiber *fiber, size_t stack_size)
 	madvise(fiber->stack, fiber->stack_size, MADV_DONTNEED);
 #endif
 
+	fiber_wmark_init(fiber);
 	mprotect(guard, page_size, PROT_NONE);
 	return 0;
 }
@@ -926,8 +1106,12 @@ cord_create(struct cord *cord, const char *name)
 	/* Record stack extents */
 	tt_pthread_attr_getstack(cord->id, &cord->sched.stack,
 				 &cord->sched.stack_size);
+	cord->sched.stack_overflow_wmark = cord->sched.stack;
+	cord->sched.stack_dynamic_wmark = cord->sched.stack;
 #else
 	cord->sched.stack = NULL;
+	cord->sched.stack_overflow_wmark = NULL;
+	cord->sched.stack_dynamic_wmark = NULL;
 	cord->sched.stack_size = 0;
 #endif
 }
@@ -1238,6 +1422,7 @@ fiber_init(int (*invoke)(fiber_func f, va_list ap))
 {
 	page_size = sysconf(_SC_PAGESIZE);
 	stack_direction = check_stack_direction(__builtin_frame_address(0));
+	stack_wmark_init();
 	fiber_invoke = invoke;
 	main_thread_id = pthread_self();
 	main_cord.loop = ev_default_loop(EVFLAG_AUTO | EVFLAG_ALLOCFD);
diff --git a/src/lib/core/fiber.h b/src/lib/core/fiber.h
index f1f5a0555..271d2b4d0 100644
--- a/src/lib/core/fiber.h
+++ b/src/lib/core/fiber.h
@@ -348,6 +348,10 @@ struct fiber {
 	struct slab *stack_slab;
 	/** Coro stack addr. */
 	void *stack;
+	/** Stack dynamic watermark addr for usage optimization. */
+	void *stack_dynamic_wmark;
+	/** Stack watermark addr for overflow detection. */
+	void *stack_overflow_wmark;
 	/** Coro stack size. */
 	size_t stack_size;
 	/** Valgrind stack id. */
-- 
2.20.1

  parent reply	other threads:[~2019-03-05 22:38 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-05 22:38 [PATCH v3 0/3] core/fiber: Rework stack management Cyrill Gorcunov
2019-03-05 22:38 ` [PATCH 1/3] core/fiber: Increase default stack size Cyrill Gorcunov
2019-03-05 23:43   ` [tarantool-patches] " Alexander Turenko
2019-03-06  7:09     ` Cyrill Gorcunov
2019-03-06  7:30     ` Konstantin Osipov
2019-03-05 22:38 ` [PATCH 2/3] core/fiber: Mark stack as unneeded on creation Cyrill Gorcunov
2019-03-05 22:38 ` Cyrill Gorcunov [this message]
2019-03-06 18:05   ` [PATCH 3/3] core/fiber: Put watermarks into stack to track its usage Vladimir Davydov
2019-03-06 19:03     ` Cyrill Gorcunov
2019-03-07  8:27       ` Vladimir Davydov
2019-03-07  9:04         ` Cyrill Gorcunov
2019-03-07  9:20           ` Vladimir Davydov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190305223854.14660-4-gorcunov@gmail.com \
    --to=gorcunov@gmail.com \
    --cc=kostja@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --cc=vdavydov.dev@gmail.com \
    --subject='Re: [PATCH 3/3] core/fiber: Put watermarks into stack to track its usage' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox