Drain fences before starting client recovery

A newly elected leader fences the previous quorum leader as it's removed
from the quorum, and the same rid is also a mounted client that the new
server will scan and prepare for recovery. If that fence is still pending
when the 30s recovery timeout fires, the rid is fenced a second time and
the two fence requests collide on the rid-named sysfs dir.

Close the window at its source: reclaim the pending fences and wait for
them to fully drain before scanning the mounted client btree for
recovery. Reclaim removes a fenced rid from that btree, so once the fence
list is empty the previous leader is no longer a recovery candidate and
the recovery timeout can't fence it again.

Add scoutfs_fence_drained() as a non-blocking predicate in the fence
layer: it reports an empty pending list and surfaces any errored fence.
The server owns the wait in wait_for_fence_drain(), blocking on
server->waitq so it wakes promptly when the server stops.
queue_reclaim_work() moves ahead of start_recovery() so reclaim runs
during the drain.

Draining is only an optimization, since the existing fence dedup already
makes a double fence non-fatal. The backstop timeout is therefore best
effort: on a large system a single reclaim can legitimately run longer
than the timeout, and aborting a healthy reclaim would recreate the
recovery-failure loop we're avoiding. On timeout the server warns and
starts recovery anyway, letting reclaim finish in the background and dedup
absorb any double-submit. Startup is only aborted if a fence errored (the
node couldn't be fenced, so recovery isn't safe) or the server is already
stopping.

Signed-off-by: Auke Kok <auke.kok@versity.com>

kmod/src/fence.c

@@ -409,6 +409,36 @@ int scoutfs_fence_wait_fenced(struct super_block *sb, long timeout_jiffies)
 	return ret;
 }
 
+/*
+ * True once every fence request has been fully reclaimed off the list, not
+ * merely fenced.  Reclaim is what removes a fenced rid from the mounted
+ * client btree, so an empty list is the point at which those rids are gone.
+ * If any request errored we surface that so the caller stops waiting for a
+ * reclaim that won't make progress.
+ *
+ * This is a non-blocking check.  The caller owns the wait so it can also
+ * break on its own state; the list is woken on fi->waitq as it drains, but
+ * draining is inherently a multi-second operation so a polled wait is fine.
+ */
+bool scoutfs_fence_drained(struct super_block *sb, bool *error)
+{
+	DECLARE_FENCE_INFO(sb, fi);
+	struct pending_fence *fence;
+	bool drained;
+
+	*error = false;
+
+	spin_lock(&fi->lock);
+	list_for_each_entry(fence, &fi->list, entry) {
+		if (fence->error)
+			*error = true;
+	}
+	drained = list_empty(&fi->list);
+	spin_unlock(&fi->lock);
+
+	return drained;
+}
+
 /*
  * This must be called early during startup so that it is guaranteed that
  * no other subsystems will try and call fence_start while we're waiting

kmod/src/fence.h

@@ -12,6 +12,7 @@ int scoutfs_fence_next(struct super_block *sb, u64 *rid, int *reason, bool *erro
 int scoutfs_fence_reason_pending(struct super_block *sb, int reason);
 int scoutfs_fence_free(struct super_block *sb, u64 rid);
 int scoutfs_fence_wait_fenced(struct super_block *sb, long timeout_jiffies);
+bool scoutfs_fence_drained(struct super_block *sb, bool *error);
 
 int scoutfs_fence_setup(struct super_block *sb);
 void scoutfs_fence_stop(struct super_block *sb);

kmod/src/server.c

@@ -4367,6 +4367,14 @@ void scoutfs_server_recov_finish(struct super_block *sb, u64 rid, int which)
  */
 #define SERVER_RECOV_TIMEOUT_MS (30 * MSEC_PER_SEC)
 
+/*
+ * Upper bound on how long startup waits for pre-existing fences to drain
+ * before starting recovery.  Each fence request errors out on its own
+ * timeout (raising -EIO here) well before this fires; this is only a
+ * backstop against a reclaim that's wedged without erroring.
+ */
+#define SERVER_FENCE_DRAIN_TIMEOUT_MS (60 * MSEC_PER_SEC)
+
 /*
  * Not all clients recovered in time.  We fence them and reclaim
  * whatever resources they were using.  If we see a rid here then we're
@@ -4479,6 +4487,53 @@ static void queue_reclaim_work(struct server_info *server, unsigned long delay)
 		queue_delayed_work(server->wq, &server->reclaim_dwork, delay);
 }
 
+/*
+ * Wait for pending fences to be fully reclaimed before we start recovery, so
+ * a fenced rid is out of the mounted client btree and won't be prepared for
+ * recovery and then fenced a second time when the recovery timeout fires.
+ *
+ * We own the wait here rather than in the fence layer so we can also break on
+ * the server stopping.  The fence list is drained by the reclaim worker; we
+ * block on server->waitq, which stop_server() wakes, and otherwise poll the
+ * drain state on a short interval.
+ *
+ * Draining is only an optimization: the fence dedup already makes a double
+ * fence non-fatal, so this just avoids requesting the second fence at all.
+ * The backstop timeout is therefore best effort -- on a large system a single
+ * reclaim can legitimately run longer than the timeout, and aborting a
+ * healthy reclaim would recreate the recovery-failure loop we're avoiding.
+ * On timeout we warn and proceed; reclaim keeps running in the background and
+ * dedup covers any double-submit.  We only abort startup if a fence errored
+ * (the node couldn't be fenced, so recovery isn't safe) or the server is
+ * already stopping.
+ */
+static int wait_for_fence_drain(struct super_block *sb)
+{
+	DECLARE_SERVER_INFO(sb, server);
+	unsigned long deadline = jiffies +
+		msecs_to_jiffies(SERVER_FENCE_DRAIN_TIMEOUT_MS);
+	bool error;
+
+	while (!scoutfs_fence_drained(sb, &error)) {
+		if (error)
+			return -EIO;
+		if (server_is_stopping(server))
+			return -ESHUTDOWN;
+		if (time_after_eq(jiffies, deadline)) {
+			scoutfs_warn(sb, "fences not drained after %lu ms, starting recovery anyway",
+				     SERVER_FENCE_DRAIN_TIMEOUT_MS);
+			return 0;
+		}
+
+		wait_event_timeout(server->waitq,
+				   server_is_stopping(server) ||
+				   scoutfs_fence_drained(sb, &error),
+				   msecs_to_jiffies(MSEC_PER_SEC));
+	}
+
+	return 0;
+}
+
 #define RECLAIM_WORK_DELAY_MS	MSEC_PER_SEC
 
 /*
@@ -4631,6 +4686,22 @@ static void scoutfs_server_worker(struct work_struct *work)
 		goto shutdown;
 	}
 
+	/*
+	 * Reclaim any rids that were already fenced as we were elected
+	 * (notably the previous quorum leader) and wait for those fences to
+	 * drain fully before starting recovery.  Reclaim removes a fenced rid
+	 * from the mounted client btree, so draining first ensures
+	 * start_recovery() won't scan that rid, prepare it for recovery, and
+	 * then fence it a second time when the recovery timeout expires.
+	 */
+	queue_reclaim_work(server, 0);
+
+	ret = wait_for_fence_drain(sb);
+	if (ret) {
+		scoutfs_err(sb, "server error %d draining fences before recovery", ret);
+		goto shutdown;
+	}
+
 	ret = start_recovery(sb);
 	if (ret) {
 		scoutfs_err(sb, "server error %d starting client recovery", ret);
@@ -4644,8 +4715,6 @@ static void scoutfs_server_worker(struct work_struct *work)
 	scoutfs_info(sb, "server ready at "SIN_FMT, SIN_ARG(&sin));
 	server_up(server);
 
-	queue_reclaim_work(server, 0);
-
 	/* interruptible mostly to avoid stuck messages */
 	wait_event_interruptible(server->waitq, server_is_stopping(server));