Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH 0/2] fiber_touch() and fiber_continue()
@ 2021-04-22 23:05 Vladislav Shpilevoy via Tarantool-patches
  2021-04-22 23:05 ` [Tarantool-patches] [PATCH 1/2] fiber: introduce fiber_touch and fiber_continue Vladislav Shpilevoy via Tarantool-patches
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Vladislav Shpilevoy via Tarantool-patches @ 2021-04-22 23:05 UTC (permalink / raw)
  To: tarantool-patches, gorcunov, sergepetrenko

The patchset introduces 2 alternatives to fiber_wakeup(): fiber_touch() and
fiber_continue(). Touch behaves like wakeup but is NOP on the current fiber -
sometimes it does not matter in the code. Continue works like wakeup but ensures
the fiber is not the current one - helps to catch errors where it is not so,
earlier than the awake fiber would try to yield.

I couldn't find better names. Tried fiber_wake() instead of fiber_touch() but it
looks too similar to fiber_wakeup(). Tried fiber_go(), but it looks too similar
to fiber_gc(), and just looks more strange than touch.

Branch: http://github.com/tarantool/tarantool/tree/gerold103/gh-5292-fiber-wakeup-nop
Issue: https://github.com/tarantool/tarantool/issues/5292

Vladislav Shpilevoy (2):
  fiber: introduce fiber_touch and fiber_continue
  fiber: use fiber_touch and fiber_continue

 src/box/applier.cc           |   4 +-
 src/box/box.cc               |   2 +-
 src/box/gc.c                 |  12 ++--
 src/box/journal.c            |   4 +-
 src/box/memtx_engine.c       |   2 +-
 src/box/raft.c               |   9 ++-
 src/box/recovery.cc          |   2 +-
 src/box/txn.c                |   8 +--
 src/box/txn_limbo.c          |   9 ++-
 src/box/vy_log.c             |   2 +-
 src/box/vy_quota.c           |   2 +-
 src/box/vy_scheduler.c       |   2 +-
 src/lib/core/cbus.c          |   2 +-
 src/lib/core/coio.cc         |   2 +-
 src/lib/core/coio_file.c     |   2 +-
 src/lib/core/coio_task.c     |   2 +-
 src/lib/core/fiber.c         | 107 +++++++++++++++++++++--------------
 src/lib/core/fiber.h         |  26 +++++++++
 src/lib/core/fiber_channel.c |   4 +-
 src/lib/core/fiber_cond.c    |   4 +-
 src/lib/core/fiber_cond.h    |   7 ++-
 src/lib/core/fiber_pool.c    |   2 +-
 src/lib/core/latch.h         |   2 +-
 src/lib/swim/swim.c          |   3 +-
 src/lua/fiber.c              |   2 +-
 test/unit/fiber.cc           |  64 +++++++++++++++++++++
 test/unit/fiber.result       |   4 ++
 27 files changed, 200 insertions(+), 91 deletions(-)

-- 
2.24.3 (Apple Git-128)


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [Tarantool-patches] [PATCH 1/2] fiber: introduce fiber_touch and fiber_continue
  2021-04-22 23:05 [Tarantool-patches] [PATCH 0/2] fiber_touch() and fiber_continue() Vladislav Shpilevoy via Tarantool-patches
@ 2021-04-22 23:05 ` Vladislav Shpilevoy via Tarantool-patches
  2021-04-22 23:05 ` [Tarantool-patches] [PATCH 2/2] fiber: use " Vladislav Shpilevoy via Tarantool-patches
  2021-04-23 23:16 ` [Tarantool-patches] [PATCH 0/2] fiber_touch() and fiber_continue() Vladislav Shpilevoy via Tarantool-patches
  2 siblings, 0 replies; 4+ messages in thread
From: Vladislav Shpilevoy via Tarantool-patches @ 2021-04-22 23:05 UTC (permalink / raw)
  To: tarantool-patches, gorcunov, sergepetrenko

These new functions are supposed to be used instead of
fiber_wakeup whose behaviour sometimes leads to unexpected and
hard to catch bugs and to crashes when called on the self fiber,
usually accidentally.

fiber_touch() allows to wakeup any fiber safely, even self.
Because on self it turns into NOP, but it is not any slower than
fiber_wakeup(), because uses the same code, but with a bit
different flag set.

fiber_continue() is like fiber_wakeup() but it has an assertion
that it is not called on self allowing to catch self-wakeup bugs
early and easy.

fiber_wakeup() still exists because it is a part of the public
API.

In order to make fiber_touch() as cheap as fiber_wakeup() there is
a new flag FIBER_IS_RUNNING. It is set for the fiber stored in the
cord - the currently running one. The only difference between the
wakeup and touch is wakeup skips READY | DEAD fibers, while the
touch skips READY | DEAD | RUNNING. Both cost one bitwise OR
operation.

Part of #5292
---
 src/lib/core/fiber.c   | 107 ++++++++++++++++++++++++-----------------
 src/lib/core/fiber.h   |  26 ++++++++++
 test/unit/fiber.cc     |  64 ++++++++++++++++++++++++
 test/unit/fiber.result |   4 ++
 4 files changed, 158 insertions(+), 43 deletions(-)

diff --git a/src/lib/core/fiber.c b/src/lib/core/fiber.c
index f8b85d99d..96de83042 100644
--- a/src/lib/core/fiber.c
+++ b/src/lib/core/fiber.c
@@ -396,10 +396,13 @@ fiber_call_impl(struct fiber *callee)
 	assert(rlist_empty(&callee->state));
 	assert(caller);
 	assert(caller != callee);
+	assert((caller->flags & FIBER_IS_RUNNING) != 0);
+	assert((callee->flags & FIBER_IS_RUNNING) == 0);
 
+	caller->flags &= ~FIBER_IS_RUNNING;
 	cord->fiber = callee;
+	callee->flags = (callee->flags & ~FIBER_IS_READY) | FIBER_IS_RUNNING;
 
-	callee->flags &= ~FIBER_IS_READY;
 	ASAN_START_SWITCH_FIBER(asan_state, 1,
 				callee->stack,
 				callee->stack_size);
@@ -452,33 +455,10 @@ fiber_checkstack(void)
  * call, it simply reschedules the fiber after other ready
  * fibers in the same event loop iteration.
  */
-void
-fiber_wakeup(struct fiber *f)
+static void
+fiber_make_ready(struct fiber *f)
 {
-	assert(! (f->flags & FIBER_IS_DEAD));
-	/**
-	 * Do nothing if the fiber is already in cord->ready
-	 * list *or* is in the call chain created by
-	 * fiber_schedule_list(). While it's harmless to re-add
-	 * a fiber to cord->ready, even if it's already there,
-	 * but the same game is deadly when the fiber is in
-	 * the callee list created by fiber_schedule_list().
-	 *
-	 * To put it another way, fiber_wakeup() is a 'request' to
-	 * schedule the fiber for execution, and once it is executing
-	 * a wakeup request is considered complete and it must be
-	 * removed.
-	 *
-	 * A dead fiber can be lingering in the cord fiber list
-	 * if it is joinable. This makes it technically possible
-	 * to schedule it. We would never make such a mistake
-	 * in our own code, hence the assert above. But as long
-	 * as fiber.wakeup() is a part of public Lua API, an
-	 * external rock can mess things up. Ignore such attempts
-	 * as well.
-	 */
-	if (f->flags & (FIBER_IS_READY | FIBER_IS_DEAD))
-		return;
+	assert((f->flags & (FIBER_IS_DEAD | FIBER_IS_READY)) == 0);
 	struct cord *cord = cord();
 	if (rlist_empty(&cord->ready)) {
 		/*
@@ -503,6 +483,45 @@ fiber_wakeup(struct fiber *f)
 	f->flags |= FIBER_IS_READY;
 }
 
+void
+fiber_wakeup(struct fiber *f)
+{
+	assert((f->flags & FIBER_IS_DEAD) == 0);
+	/*
+	 * Do nothing if the fiber is already in cord->ready list *or* is in the
+	 * call chain created by fiber_schedule_list(). While it's harmless to
+	 * re-add a fiber to cord->ready, even if it's already there, but the
+	 * same game is deadly when the fiber is in the callee list created by
+	 * fiber_schedule_list().
+	 *
+	 * To put it another way, fiber_wakeup() is a 'request' to schedule the
+	 * fiber for execution, and once it is executing a wakeup request is
+	 * considered complete and it must be removed.
+	 *
+	 * A dead fiber can be lingering in the cord fiber list if it is
+	 * joinable. This makes it technically possible to schedule it. We would
+	 * never make such a mistake in our own code, hence the assert above.
+	 * But as long as fiber.wakeup() is a part of public Lua API, an
+	 * external rock can mess things up. Ignore such attempts
+	 * as well.
+	 */
+	if ((f->flags & (FIBER_IS_READY | FIBER_IS_DEAD)) == 0)
+		fiber_make_ready(f);
+}
+
+void
+fiber_touch(struct fiber *f)
+{
+	assert((f->flags & FIBER_IS_DEAD) == 0);
+	/*
+	 * The reason why DEAD is checked and asserted at the same time see in
+	 * fiber_wakeup().
+	 */
+	if ((f->flags & (FIBER_IS_READY | FIBER_IS_DEAD |
+	    FIBER_IS_RUNNING)) == 0)
+		fiber_make_ready(f);
+}
+
 /** Cancel the subject fiber.
 *
  * Note: cancelation is asynchronous. Use fiber_join() to wait for the
@@ -521,7 +540,6 @@ void
 fiber_cancel(struct fiber *f)
 {
 	assert(f->fid != 0);
-	struct fiber *self = fiber();
 	/**
 	 * Do nothing if the fiber is dead, since cancelling
 	 * the fiber would clear the diagnostics area and
@@ -535,10 +553,8 @@ fiber_cancel(struct fiber *f)
 	/**
 	 * Don't wake self and zombies.
 	 */
-	if (f != self) {
-		if (f->flags & FIBER_IS_CANCELLABLE)
-			fiber_wakeup(f);
-	}
+	if ((f->flags & FIBER_IS_CANCELLABLE) != 0)
+		fiber_touch(f);
 }
 
 /**
@@ -686,8 +702,13 @@ fiber_yield(void)
 
 	assert(callee->flags & FIBER_IS_READY || callee == &cord->sched);
 	assert(! (callee->flags & FIBER_IS_DEAD));
+	assert((caller->flags & FIBER_IS_RUNNING) != 0);
+	assert((callee->flags & FIBER_IS_RUNNING) == 0);
+
+	caller->flags &= ~FIBER_IS_RUNNING;
 	cord->fiber = callee;
-	callee->flags &= ~FIBER_IS_READY;
+	callee->flags = (callee->flags & ~FIBER_IS_READY) | FIBER_IS_RUNNING;
+
 	ASAN_START_SWITCH_FIBER(asan_state,
 				(caller->flags & FIBER_IS_DEAD) == 0,
 				callee->stack,
@@ -712,7 +733,7 @@ fiber_schedule_timeout(ev_loop *loop,
 	struct fiber_watcher_data *state =
 			(struct fiber_watcher_data *) watcher->data;
 	state->timed_out = true;
-	fiber_wakeup(state->f);
+	fiber_continue(state->f);
 }
 
 /**
@@ -748,10 +769,6 @@ fiber_sleep(double delay)
 	if (delay == 0) {
 		ev_idle_start(loop(), &cord()->idle_event);
 	}
-	/*
-	 * We don't use fiber_wakeup() here to ensure there is
-	 * no infinite wakeup loop in case of fiber_sleep(0).
-	 */
 	fiber_yield_timeout(delay);
 
 	if (delay == 0) {
@@ -766,7 +783,7 @@ fiber_schedule_cb(ev_loop *loop, ev_watcher *watcher, int revents)
 	(void) revents;
 	struct fiber *fiber = watcher->data;
 	assert(fiber() == &cord()->sched);
-	fiber_wakeup(fiber);
+	fiber_continue(fiber);
 }
 
 static inline void
@@ -864,7 +881,11 @@ fiber_reset(struct fiber *fiber)
 {
 	rlist_create(&fiber->on_yield);
 	rlist_create(&fiber->on_stop);
-	fiber->flags = FIBER_DEFAULT_FLAGS;
+	/*
+	 * Preserve the running flag if set. Reset might be called on the
+	 * current fiber when it is recycled.
+	 */
+	fiber->flags = FIBER_DEFAULT_FLAGS | (fiber->flags & FIBER_IS_RUNNING);
 #if ENABLE_FIBER_TOP
 	clock_stat_reset(&fiber->clock_stat);
 #endif /* ENABLE_FIBER_TOP */
@@ -929,8 +950,7 @@ fiber_loop(MAYBE_UNUSED void *data)
 		       struct fiber *f;
 		       f = rlist_shift_entry(&fiber->wake, struct fiber,
 					     state);
-		       assert(f != fiber);
-		       fiber_wakeup(f);
+		       fiber_continue(f);
 	        }
 		fiber_on_stop(fiber);
 		/* reset pending wakeups */
@@ -1449,6 +1469,7 @@ cord_create(struct cord *cord, const char *name)
 	cord->sched.name = NULL;
 	fiber_set_name(&cord->sched, "sched");
 	cord->fiber = &cord->sched;
+	cord->sched.flags |= FIBER_IS_RUNNING;
 
 	cord->max_fid = FIBER_ID_MAX_RESERVED;
 	/*
@@ -1632,7 +1653,7 @@ cord_cojoin_wakeup(struct ev_loop *loop, struct ev_async *ev, int revents)
 	struct cord_cojoin_ctx *ctx = (struct cord_cojoin_ctx *)ev->data;
 
 	ctx->task_complete = true;
-	fiber_wakeup(ctx->fiber);
+	fiber_continue(ctx->fiber);
 }
 
 int
diff --git a/src/lib/core/fiber.h b/src/lib/core/fiber.h
index b9eeef697..2506a178c 100644
--- a/src/lib/core/fiber.h
+++ b/src/lib/core/fiber.h
@@ -162,6 +162,10 @@ enum {
 	 * This flag is set when fiber uses custom stack size.
 	 */
 	FIBER_CUSTOM_STACK	= 1 << 5,
+	/**
+	 * The flag is set for the fiber currently being executed by the cord.
+	 */
+	FIBER_IS_RUNNING	= 1 << 6,
 	FIBER_DEFAULT_FLAGS = FIBER_IS_CANCELLABLE
 };
 
@@ -820,6 +824,28 @@ fiber_destroy_all(struct cord *cord);
 void
 fiber_gc(void);
 
+/**
+ * A polite version of wakeup. The same as fiber_wakeup() but is nop for the
+ * currently running fiber. Allows to save one 'if' compared to wakeup by not
+ * forcing to check if the fiber is the current one.
+ */
+void
+fiber_touch(struct fiber *f);
+
+/**
+ * The same as fiber_wakeup() but bans continuation of the current fiber as it
+ * might lead to very unexpected behaviour. For instance, some code called
+ * wakeup on self accidentally, and much later in some other subsystem is called
+ * fiber_yield() to wait for something - it would return right away without
+ * waiting causing troubles of all kinds.
+ */
+static inline void
+fiber_continue(struct fiber *f)
+{
+	assert(f != fiber());
+	fiber_wakeup(f);
+}
+
 void
 fiber_call(struct fiber *callee);
 
diff --git a/test/unit/fiber.cc b/test/unit/fiber.cc
index 9c1a23bdd..65c083230 100644
--- a/test/unit/fiber.cc
+++ b/test/unit/fiber.cc
@@ -201,12 +201,76 @@ fiber_name_test()
 	footer();
 }
 
+static int
+yield_f(va_list ap)
+{
+	bool *is_started = va_arg(ap, bool *);
+	*is_started = true;
+	fiber_yield();
+	return 0;
+}
+
+static void
+fiber_touch_test()
+{
+	header();
+
+	struct fiber *f = fiber();
+
+	fiber_touch(f);
+	double duration = 0.001;
+	uint64_t t1 = fiber_clock64();
+	fiber_sleep(duration);
+	uint64_t t2 = fiber_clock64();
+	/*
+	 * It was a real sleep, not immediate wakeup. Touch is nop on a running
+	 * fiber.
+	 */
+	assert(t2 - t1 >= duration);
+
+	bool is_started = false;
+	f = fiber_new_xc("yield", yield_f);
+	fiber_set_joinable(f, true);
+	fiber_start(f, &is_started);
+	/* Let the infinitely yielding fiber to start its yield. */
+	fiber_sleep(0);
+	assert(is_started);
+	fiber_touch(f);
+	int rc = fiber_join(f);
+	assert(rc == 0);
+
+	footer();
+}
+
+static void
+fiber_continue_test()
+{
+	header();
+
+	struct fiber *f = fiber();
+
+	bool is_started = false;
+	f = fiber_new_xc("yield", yield_f);
+	fiber_set_joinable(f, true);
+	fiber_start(f, &is_started);
+	/* Let the infinitely yielding fiber to start its yield. */
+	fiber_sleep(0);
+	assert(is_started);
+	fiber_continue(f);
+	int rc = fiber_join(f);
+	assert(rc == 0);
+
+	footer();
+}
+
 static int
 main_f(va_list ap)
 {
 	fiber_name_test();
 	fiber_join_test();
 	fiber_stack_test();
+	fiber_touch_test();
+	fiber_continue_test();
 	ev_break(loop(), EVBREAK_ALL);
 	return 0;
 }
diff --git a/test/unit/fiber.result b/test/unit/fiber.result
index a61e0a2b8..23f331e9b 100644
--- a/test/unit/fiber.result
+++ b/test/unit/fiber.result
@@ -17,3 +17,7 @@ SystemError Failed to allocate 42 bytes in allocator for exception: Cannot alloc
 # normal-stack fiber not crashed
 # big-stack fiber not crashed
 	*** fiber_stack_test: done ***
+	*** fiber_touch_test ***
+	*** fiber_touch_test: done ***
+	*** fiber_continue_test ***
+	*** fiber_continue_test: done ***
-- 
2.24.3 (Apple Git-128)


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [Tarantool-patches] [PATCH 2/2] fiber: use fiber_touch and fiber_continue
  2021-04-22 23:05 [Tarantool-patches] [PATCH 0/2] fiber_touch() and fiber_continue() Vladislav Shpilevoy via Tarantool-patches
  2021-04-22 23:05 ` [Tarantool-patches] [PATCH 1/2] fiber: introduce fiber_touch and fiber_continue Vladislav Shpilevoy via Tarantool-patches
@ 2021-04-22 23:05 ` Vladislav Shpilevoy via Tarantool-patches
  2021-04-23 23:16 ` [Tarantool-patches] [PATCH 0/2] fiber_touch() and fiber_continue() Vladislav Shpilevoy via Tarantool-patches
  2 siblings, 0 replies; 4+ messages in thread
From: Vladislav Shpilevoy via Tarantool-patches @ 2021-04-22 23:05 UTC (permalink / raw)
  To: tarantool-patches, gorcunov, sergepetrenko

fiber_wakeup() is not safe when called on the self fiber - it
leads to a skip of the next yield, which leads to hard to catch
bugs and might appear much later than the wakeup was called making
it hard to trace the source of the bug.

The previous commit introduces safer versions of wakeup: touch and
continue. This patch uses them everywhere except the tests.

This allows to drop 'f != fiber()' checks in some places which
could work in the current fiber, and ensures in all the other
places the fiber is never the current one.

Closes #5292
---
 src/box/applier.cc           |  4 +---
 src/box/box.cc               |  2 +-
 src/box/gc.c                 | 12 ++++++------
 src/box/journal.c            |  4 ++--
 src/box/memtx_engine.c       |  2 +-
 src/box/raft.c               |  9 ++++-----
 src/box/recovery.cc          |  2 +-
 src/box/txn.c                |  8 +++-----
 src/box/txn_limbo.c          |  9 ++++-----
 src/box/vy_log.c             |  2 +-
 src/box/vy_quota.c           |  2 +-
 src/box/vy_scheduler.c       |  2 +-
 src/lib/core/cbus.c          |  2 +-
 src/lib/core/coio.cc         |  2 +-
 src/lib/core/coio_file.c     |  2 +-
 src/lib/core/coio_task.c     |  2 +-
 src/lib/core/fiber_channel.c |  4 ++--
 src/lib/core/fiber_cond.c    |  4 ++--
 src/lib/core/fiber_cond.h    |  7 ++++---
 src/lib/core/fiber_pool.c    |  2 +-
 src/lib/core/latch.h         |  2 +-
 src/lib/swim/swim.c          |  3 +--
 src/lua/fiber.c              |  2 +-
 23 files changed, 42 insertions(+), 48 deletions(-)

diff --git a/src/box/applier.cc b/src/box/applier.cc
index dc05c91d3..030c46ce5 100644
--- a/src/box/applier.cc
+++ b/src/box/applier.cc
@@ -792,9 +792,7 @@ apply_synchro_row_cb(struct journal_entry *entry)
 		txn_limbo_process(&txn_limbo, synchro_entry->req);
 		trigger_run(&replicaset.applier.on_wal_write, NULL);
 	}
-	/* The fiber is the same on final join. */
-	if (synchro_entry->owner != fiber())
-		fiber_wakeup(synchro_entry->owner);
+	fiber_touch(synchro_entry->owner);
 }
 
 /** Process a synchro request. */
diff --git a/src/box/box.cc b/src/box/box.cc
index 59925962d..2060aed50 100644
--- a/src/box/box.cc
+++ b/src/box/box.cc
@@ -1436,7 +1436,7 @@ box_quorum_on_ack_f(struct trigger *trigger, void *event)
 	vclock_follow(&t->vclock, ack->source, new_lsn);
 	++t->ack_count;
 	if (t->ack_count >= t->quorum) {
-		fiber_wakeup(t->waiter);
+		fiber_continue(t->waiter);
 		trigger_clear(trigger);
 	}
 	return 0;
diff --git a/src/box/gc.c b/src/box/gc.c
index 10f899923..2cc1b76de 100644
--- a/src/box/gc.c
+++ b/src/box/gc.c
@@ -314,7 +314,7 @@ gc_set_wal_cleanup_delay(double wal_cleanup_delay)
 	 * it is already in a regular cleanup stage.
 	 */
 	if (gc.is_paused)
-		fiber_wakeup(gc.cleanup_fiber);
+		fiber_continue(gc.cleanup_fiber);
 }
 
 void
@@ -334,7 +334,7 @@ gc_delay_unref(void)
 		gc.delay_ref--;
 		if (gc.delay_ref == 0) {
 			gc.is_paused = false;
-			fiber_wakeup(gc.cleanup_fiber);
+			fiber_continue(gc.cleanup_fiber);
 		}
 	}
 }
@@ -354,7 +354,7 @@ gc_schedule_cleanup(void)
 	 * the current round completes.
 	 */
 	if (gc.cleanup_scheduled++ == gc.cleanup_completed)
-		fiber_wakeup(gc.cleanup_fiber);
+		fiber_continue(gc.cleanup_fiber);
 }
 
 /**
@@ -425,7 +425,7 @@ gc_set_checkpoint_interval(double interval)
 	checkpoint_schedule_cfg(&gc.checkpoint_schedule,
 				ev_monotonic_now(loop()), interval);
 	if (!gc.checkpoint_is_in_progress)
-		fiber_wakeup(gc.checkpoint_fiber);
+		fiber_continue(gc.checkpoint_fiber);
 }
 
 void
@@ -539,7 +539,7 @@ gc_checkpoint(void)
 	checkpoint_schedule_cfg(&gc.checkpoint_schedule,
 				ev_monotonic_now(loop()),
 				gc.checkpoint_schedule.interval);
-	fiber_wakeup(gc.checkpoint_fiber);
+	fiber_continue(gc.checkpoint_fiber);
 
 	if (gc_do_checkpoint(false) != 0)
 		return -1;
@@ -566,7 +566,7 @@ gc_trigger_checkpoint(void)
 	gc.checkpoint_is_pending = true;
 	checkpoint_schedule_reset(&gc.checkpoint_schedule,
 				  ev_monotonic_now(loop()));
-	fiber_wakeup(gc.checkpoint_fiber);
+	fiber_continue(gc.checkpoint_fiber);
 }
 
 static int
diff --git a/src/box/journal.c b/src/box/journal.c
index 886a15139..b1b334237 100644
--- a/src/box/journal.c
+++ b/src/box/journal.c
@@ -68,7 +68,7 @@ journal_queue_wakeup(void)
 {
 	struct rlist *list = &journal_queue.waiters;
 	if (!rlist_empty(list) && !journal_queue_is_full())
-		fiber_wakeup(rlist_first_entry(list, struct fiber, state));
+		fiber_continue(rlist_first_entry(list, struct fiber, state));
 }
 
 void
@@ -93,6 +93,6 @@ journal_queue_flush(void)
 		return;
 	struct rlist *list = &journal_queue.waiters;
 	while (!rlist_empty(list))
-		fiber_wakeup(rlist_first_entry(list, struct fiber, state));
+		fiber_continue(rlist_first_entry(list, struct fiber, state));
 	journal_queue_wait();
 }
diff --git a/src/box/memtx_engine.c b/src/box/memtx_engine.c
index e076cd71d..66f2f28eb 100644
--- a/src/box/memtx_engine.c
+++ b/src/box/memtx_engine.c
@@ -1170,7 +1170,7 @@ memtx_engine_schedule_gc(struct memtx_engine *memtx,
 			 struct memtx_gc_task *task)
 {
 	stailq_add_tail_entry(&memtx->gc_queue, task, link);
-	fiber_wakeup(memtx->gc_fiber);
+	fiber_continue(memtx->gc_fiber);
 }
 
 void
diff --git a/src/box/raft.c b/src/box/raft.c
index 47c869712..12ddadd4a 100644
--- a/src/box/raft.c
+++ b/src/box/raft.c
@@ -155,9 +155,8 @@ box_raft_schedule_async(struct raft *raft)
 	 * adapted to this. Also don't wakeup the current fiber - it leads to
 	 * undefined behaviour.
 	 */
-	if ((box_raft_worker->flags & FIBER_IS_CANCELLABLE) != 0 &&
-	    fiber() != box_raft_worker)
-		fiber_wakeup(box_raft_worker);
+	if ((box_raft_worker->flags & FIBER_IS_CANCELLABLE) != 0)
+		fiber_touch(box_raft_worker);
 	box_raft_has_work = true;
 }
 
@@ -283,7 +282,7 @@ box_raft_broadcast(struct raft *raft, const struct raft_msg *msg)
 static void
 box_raft_write_cb(struct journal_entry *entry)
 {
-	fiber_wakeup(entry->complete_data);
+	fiber_continue(entry->complete_data);
 }
 
 static void
@@ -340,7 +339,7 @@ box_raft_wait_leader_found_f(struct trigger *trig, void *event)
 	assert(raft == box_raft());
 	struct fiber *waiter = trig->data;
 	if (raft->leader != REPLICA_ID_NIL || !raft->is_enabled)
-		fiber_wakeup(waiter);
+		fiber_continue(waiter);
 	return 0;
 }
 
diff --git a/src/box/recovery.cc b/src/box/recovery.cc
index cd33e7635..ceda1e5c9 100644
--- a/src/box/recovery.cc
+++ b/src/box/recovery.cc
@@ -398,7 +398,7 @@ public:
 	{
 		this->events |= events;
 		if (f->flags & FIBER_IS_CANCELLABLE)
-			fiber_wakeup(f);
+			fiber_continue(f);
 	}
 
 	WalSubscription(const char *wal_dir)
diff --git a/src/box/txn.c b/src/box/txn.c
index 03b39e0de..1737fd2d5 100644
--- a/src/box/txn.c
+++ b/src/box/txn.c
@@ -504,9 +504,7 @@ txn_free_or_wakeup(struct txn *txn)
 		txn_free(txn);
 	else {
 		txn_set_flags(txn, TXN_IS_DONE);
-		if (txn->fiber != fiber())
-			/* Wake a waiting fiber up. */
-			fiber_wakeup(txn->fiber);
+		fiber_touch(txn->fiber);
 	}
 }
 
@@ -578,8 +576,8 @@ txn_on_journal_write(struct journal_entry *entry)
 		txn_run_wal_write_triggers(txn);
 	if (!txn_has_flag(txn, TXN_WAIT_SYNC))
 		txn_complete_success(txn);
-	else if (txn->fiber != NULL && txn->fiber != fiber())
-		fiber_wakeup(txn->fiber);
+	else if (txn->fiber != NULL)
+		fiber_touch(txn->fiber);
 finish:
 	fiber_set_txn(fiber(), NULL);
 }
diff --git a/src/box/txn_limbo.c b/src/box/txn_limbo.c
index c22bd6665..f3115d66d 100644
--- a/src/box/txn_limbo.c
+++ b/src/box/txn_limbo.c
@@ -275,7 +275,7 @@ txn_limbo_wait_complete(struct txn_limbo *limbo, struct txn_limbo_entry *entry)
 		txn_complete_fail(e->txn);
 		if (e == entry)
 			break;
-		fiber_wakeup(e->txn->fiber);
+		fiber_continue(e->txn->fiber);
 	}
 	fiber_set_cancellable(cancellable);
 	diag_set(ClientError, ER_SYNC_QUORUM_TIMEOUT);
@@ -314,8 +314,7 @@ static void
 txn_limbo_write_cb(struct journal_entry *entry)
 {
 	assert(entry->complete_data != NULL);
-	if (fiber() != entry->complete_data)
-		fiber_wakeup(entry->complete_data);
+	fiber_touch(entry->complete_data);
 }
 
 static void
@@ -579,7 +578,7 @@ txn_commit_cb(struct trigger *trigger, void *event)
 	struct confirm_waitpoint *cwp =
 		(struct confirm_waitpoint *)trigger->data;
 	cwp->is_confirm = true;
-	fiber_wakeup(cwp->caller);
+	fiber_continue(cwp->caller);
 	return 0;
 }
 
@@ -590,7 +589,7 @@ txn_rollback_cb(struct trigger *trigger, void *event)
 	struct confirm_waitpoint *cwp =
 		(struct confirm_waitpoint *)trigger->data;
 	cwp->is_rollback = true;
-	fiber_wakeup(cwp->caller);
+	fiber_continue(cwp->caller);
 	return 0;
 }
 
diff --git a/src/box/vy_log.c b/src/box/vy_log.c
index 8214b054e..2bccc0011 100644
--- a/src/box/vy_log.c
+++ b/src/box/vy_log.c
@@ -771,7 +771,7 @@ vy_log_init(const char *dir)
 				   vy_log_flusher_f);
 	if (vy_log.flusher == NULL)
 		panic("failed to allocate vylog flusher fiber");
-	fiber_wakeup(vy_log.flusher);
+	fiber_continue(vy_log.flusher);
 }
 
 static struct vy_log_tx *
diff --git a/src/box/vy_quota.c b/src/box/vy_quota.c
index f1ac8dd9d..9f57a69a7 100644
--- a/src/box/vy_quota.c
+++ b/src/box/vy_quota.c
@@ -203,7 +203,7 @@ vy_quota_signal(struct vy_quota *q)
 			oldest = n;
 	}
 	if (oldest != NULL)
-		fiber_wakeup(oldest->fiber);
+		fiber_continue(oldest->fiber);
 }
 
 static void
diff --git a/src/box/vy_scheduler.c b/src/box/vy_scheduler.c
index 7d8324a52..f223aad7c 100644
--- a/src/box/vy_scheduler.c
+++ b/src/box/vy_scheduler.c
@@ -959,7 +959,7 @@ vy_deferred_delete_batch_free_f(struct cmsg *cmsg)
 	/* Notify the caller if this is the last batch. */
 	assert(task->deferred_delete_in_progress > 0);
 	if (--task->deferred_delete_in_progress == 0)
-		fiber_wakeup(task->fiber);
+		fiber_continue(task->fiber);
 }
 
 /**
diff --git a/src/lib/core/cbus.c b/src/lib/core/cbus.c
index 5d91fb948..9fdbf7400 100644
--- a/src/lib/core/cbus.c
+++ b/src/lib/core/cbus.c
@@ -406,7 +406,7 @@ cbus_call_done(struct cmsg *m)
 		return;
 	}
 	msg->complete = true;
-	fiber_wakeup(msg->caller);
+	fiber_continue(msg->caller);
 }
 
 /**
diff --git a/src/lib/core/coio.cc b/src/lib/core/coio.cc
index 5dfe851d4..b191f4cee 100644
--- a/src/lib/core/coio.cc
+++ b/src/lib/core/coio.cc
@@ -747,7 +747,7 @@ coio_wait_cb(struct ev_loop *loop, ev_io *watcher, int revents)
 	(void) loop;
 	struct coio_wdata *wdata = (struct coio_wdata *) watcher->data;
 	wdata->revents = revents;
-	fiber_wakeup(wdata->fiber);
+	fiber_continue(wdata->fiber);
 }
 
 API_EXPORT int
diff --git a/src/lib/core/coio_file.c b/src/lib/core/coio_file.c
index 3ec74a549..7792d7e19 100644
--- a/src/lib/core/coio_file.c
+++ b/src/lib/core/coio_file.c
@@ -121,7 +121,7 @@ coio_complete(eio_req *req)
 	eio->done = true;
 	eio->result = req->result;
 
-	fiber_wakeup(eio->fiber);
+	fiber_continue(eio->fiber);
 	return 0;
 }
 
diff --git a/src/lib/core/coio_task.c b/src/lib/core/coio_task.c
index c8be2dea1..8f58c6dc6 100644
--- a/src/lib/core/coio_task.c
+++ b/src/lib/core/coio_task.c
@@ -185,7 +185,7 @@ coio_on_finish(eio_req *req)
 	task->complete = 1;
 	/* Reset on_timeout hook - resources will be freed by coio_task user */
 	task->base.destroy = NULL;
-	fiber_wakeup(task->fiber);
+	fiber_continue(task->fiber);
 	return 0;
 }
 
diff --git a/src/lib/core/fiber_channel.c b/src/lib/core/fiber_channel.c
index 2ac585648..2676e9b01 100644
--- a/src/lib/core/fiber_channel.c
+++ b/src/lib/core/fiber_channel.c
@@ -141,7 +141,7 @@ fiber_channel_waiter_wakeup(struct fiber *f,
 	/*
 	 * fiber_channel allows an asynchronous cancel. If a fiber
 	 * is cancelled while waiting on a timeout, it is done via
-	 * fiber_wakeup(), which modifies fiber->state link.
+	 * fiber_touch(), which modifies fiber->state link.
 	 * This ensures that a fiber is never on two "state"
 	 * lists: it's either waiting on a channel, or is
 	 * cancelled, ready for execution. This is why
@@ -154,7 +154,7 @@ fiber_channel_waiter_wakeup(struct fiber *f,
 	 * delivered to it. Since 'fiber->state' is used, this
 	 * works correctly with fiber_cancel().
 	 */
-	fiber_wakeup(f);
+	fiber_continue(f);
 }
 
 
diff --git a/src/lib/core/fiber_cond.c b/src/lib/core/fiber_cond.c
index 3624dffd4..b4d34409b 100644
--- a/src/lib/core/fiber_cond.c
+++ b/src/lib/core/fiber_cond.c
@@ -85,7 +85,7 @@ fiber_cond_signal(struct fiber_cond *e)
 	if (! rlist_empty(&e->waiters)) {
 		struct fiber *f;
 		f = rlist_first_entry(&e->waiters, struct fiber, state);
-		fiber_wakeup(f);
+		fiber_continue(f);
 	}
 }
 
@@ -95,7 +95,7 @@ fiber_cond_broadcast(struct fiber_cond *e)
 	while (! rlist_empty(&e->waiters)) {
 		struct fiber *f;
 		f = rlist_first_entry(&e->waiters, struct fiber, state);
-		fiber_wakeup(f);
+		fiber_continue(f);
 	}
 }
 
diff --git a/src/lib/core/fiber_cond.h b/src/lib/core/fiber_cond.h
index 2662e0654..4b338a156 100644
--- a/src/lib/core/fiber_cond.h
+++ b/src/lib/core/fiber_cond.h
@@ -107,9 +107,10 @@ fiber_cond_broadcast(struct fiber_cond *cond);
 /**
  * Suspend the execution of the current fiber (i.e. yield) until
  * fiber_cond_signal() is called. Like pthread_cond, fiber_cond can issue
- * spurious wake ups caused by explicit fiber_wakeup() or fiber_cancel()
- * calls. It is highly recommended to wrap calls to this function into a loop
- * and check an actual predicate and fiber_testcancel() on every iteration.
+ * spurious wake ups caused by explicit fiber_wakeup()/fiber_touch()/
+ * fiber_continue() or fiber_cancel() calls. It is highly recommended to wrap
+ * calls to this function into a loop and check an actual predicate and
+ * fiber_testcancel() on every iteration.
  *
  * @param cond condition
  * @param timeout timeout in seconds
diff --git a/src/lib/core/fiber_pool.c b/src/lib/core/fiber_pool.c
index b980028e4..8d0345a2d 100644
--- a/src/lib/core/fiber_pool.c
+++ b/src/lib/core/fiber_pool.c
@@ -186,7 +186,7 @@ fiber_pool_destroy(struct fiber_pool *pool)
 	pool->idle_timeout = 0;
 	struct fiber *idle_fiber;
 	rlist_foreach_entry(idle_fiber, &pool->idle, state)
-		fiber_wakeup(idle_fiber);
+		fiber_continue(idle_fiber);
 	/**
 	 * Just wait on fiber exit condition until all fibers are done
 	 */
diff --git a/src/lib/core/latch.h b/src/lib/core/latch.h
index 49c59cf63..ccf91742c 100644
--- a/src/lib/core/latch.h
+++ b/src/lib/core/latch.h
@@ -172,7 +172,7 @@ latch_unlock(struct latch *l)
 		 * fiber can intercept this latch.
 		 */
 		l->owner = f;
-		fiber_wakeup(f);
+		fiber_continue(f);
 	}
 }
 
diff --git a/src/lib/swim/swim.c b/src/lib/swim/swim.c
index 1ecc90414..9c211c45d 100644
--- a/src/lib/swim/swim.c
+++ b/src/lib/swim/swim.c
@@ -628,7 +628,7 @@ swim_on_member_update(struct swim *swim, struct swim_member *member,
 			swim_member_ref(member);
 			stailq_add_tail_entry(&swim->event_queue, member,
 					      in_event_queue);
-			fiber_wakeup(swim->event_handler);
+			fiber_continue(swim->event_handler);
 		}
 		member->events |= events;
 	}
@@ -2219,7 +2219,6 @@ swim_kill_event_handler(struct swim *swim)
 	 * reused.
 	 */
 	swim->event_handler = NULL;
-	fiber_wakeup(f);
 	fiber_cancel(f);
 	fiber_join(f);
 }
diff --git a/src/lua/fiber.c b/src/lua/fiber.c
index 02ec3d158..59997dc36 100644
--- a/src/lua/fiber.c
+++ b/src/lua/fiber.c
@@ -520,7 +520,7 @@ lbox_fiber_new(struct lua_State *L)
 		luaL_error(L, "fiber.new(): out of fiber stack");
 
 	struct fiber *f = fiber_create(L);
-	fiber_wakeup(f);
+	fiber_continue(f);
 	return 1;
 }
 
-- 
2.24.3 (Apple Git-128)


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Tarantool-patches] [PATCH 0/2] fiber_touch() and fiber_continue()
  2021-04-22 23:05 [Tarantool-patches] [PATCH 0/2] fiber_touch() and fiber_continue() Vladislav Shpilevoy via Tarantool-patches
  2021-04-22 23:05 ` [Tarantool-patches] [PATCH 1/2] fiber: introduce fiber_touch and fiber_continue Vladislav Shpilevoy via Tarantool-patches
  2021-04-22 23:05 ` [Tarantool-patches] [PATCH 2/2] fiber: use " Vladislav Shpilevoy via Tarantool-patches
@ 2021-04-23 23:16 ` Vladislav Shpilevoy via Tarantool-patches
  2 siblings, 0 replies; 4+ messages in thread
From: Vladislav Shpilevoy via Tarantool-patches @ 2021-04-23 23:16 UTC (permalink / raw)
  To: tarantool-patches, gorcunov, sergepetrenko

Please, scratch that. It was decided fiber_wakeup() in C and
fiber.wakeup() in Lua must become NOP on the current fiber. See
a new email thread with a different patch.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-04-23 23:17 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-22 23:05 [Tarantool-patches] [PATCH 0/2] fiber_touch() and fiber_continue() Vladislav Shpilevoy via Tarantool-patches
2021-04-22 23:05 ` [Tarantool-patches] [PATCH 1/2] fiber: introduce fiber_touch and fiber_continue Vladislav Shpilevoy via Tarantool-patches
2021-04-22 23:05 ` [Tarantool-patches] [PATCH 2/2] fiber: use " Vladislav Shpilevoy via Tarantool-patches
2021-04-23 23:16 ` [Tarantool-patches] [PATCH 0/2] fiber_touch() and fiber_continue() Vladislav Shpilevoy via Tarantool-patches

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