* [PATCH 1/2] errseq: don't let errseq_check_and_advance() hide later errors
2026-09-27 5:17 [PATCH 0/2] errseq: fix lost writeback errors in errseq_check_and_advance() Shashank Mohan Jain
@ 2026-09-27 5:17 ` Shashank Mohan Jain
2026-09-27 5:17 ` [PATCH 2/2] lib/tests: errseq: add a concurrent check_and_advance test Shashank Mohan Jain
2026-09-27 15:48 ` [PATCH 0/2] errseq: fix lost writeback errors in errseq_check_and_advance() Jeff Layton
2 siblings, 0 replies; 4+ messages in thread
From: Shashank Mohan Jain @ 2026-09-27 5:17 UTC (permalink / raw)
To: Andrew Morton
Cc: Jeff Layton, Jan Kara, NeilBrown, Thomas Maarseveen,
linux-fsdevel, linux-kernel
errseq_check_and_advance() reads the errseq_t, sets ERRSEQ_SEEN with a
cmpxchg() and then advances *since to the value it computed, ignoring
whether the cmpxchg() succeeded. When it fails because errseq_set()
recorded a different error in the meantime, *since ends up holding a
value that was never stored in the errseq_t.
errseq_set() only bumps the counter when ERRSEQ_SEEN is set. So as long
as nobody has seen the new error, recording the original errno again
recreates exactly the old value, and once another subscriber marks it
seen, it is equal to the stale *since. The next check through that
cursor then reports nothing, although -ENOSPC was recorded after the
value that check reported, and -EIO was recorded again after it
returned:
cursor f writeback cursor g
-------- --------- --------
set(-EIO)
check_and_advance(&f)
old = [c, EIO]
set(-ENOSPC)
-> [c, ENOSPC]
cmpxchg() fails
f = [c, EIO, SEEN]
returns -EIO
set(-EIO)
-> [c, EIO]
(no bump: unseen)
check_and_advance(&g)
-> [c, EIO, SEEN]
returns -EIO
check_and_advance(&f)
[c, EIO, SEEN] == f
returns 0 <- -ENOSPC and the second -EIO are lost
For file->f_wb_err this means that fsync() on one descriptor can return
0 although writeback failed after the previous fsync() on that
descriptor returned, when writeback errors race with fsync() on another
descriptor of the same file. The same applies to syncfs() through
sb->s_wb_err, which every file on the filesystem shares, and to the
ext4 and jbd2 cursors on the block device mapping. The kernel-doc
promises "Negative errno if one has been stored, or 0 if no new error
has occurred".
Retry with the value found when the cmpxchg() fails, so that *since is
only ever advanced to a value that was actually stored with ERRSEQ_SEEN
set. Every later errseq_set() then has to bump the counter, and the
error is reported. The retry loop terminates because each iteration
needs a concurrent update, like the loop in errseq_set().
A TLA+ model of errseq_set() and errseq_check_and_advance(), checked
with the TLC model checker, finds the lost error with the current code;
with this change TLC checks that an error recorded after a check
returned is always reported by the next check (exhaustively for two
subscribers with three checks each, and one writer recording four
errors or two writers recording two each). The KUnit race test added
in the next patch misses the error in 1-6% of 2 million rounds on
4-CPU UML before this change (depending on host load), and in none
after it.
Fixes: 84cbadadc6ea ("lib: add errseq_t type and infrastructure for handling it")
Cc: stable@vger.kernel.org
Assisted-by: LLM TLC
Signed-off-by: Shashank Mohan Jain <jain.sm@gmail.com>
---
Found with a TLA+ model of lib/errseq.c checked with TLC; the fix, the
test and the changelogs were drafted with an LLM assistant (Claude
Code, Claude Opus 5.5).
Tested: the KUnit case in patch 2 on UML x86_64 with 4 CPUs (8 runs)
and 2 CPUs, and on UML i386 with 4 CPUs (--kernel_args seccomp=on
--kernel_args ncpus=4): 22k-116k of 2M rounds lose the error before,
0 after; a userspace replay of the unmodified lib/errseq.c with a hook
before the cmpxchg(); TLC (exhaustive for 2 files x 3 checks, 1 writer
x 4 errors or 2 writers x 2 errors); W=1 builds for UML x86_64 and
i386.
Not tested: fsync()/syncfs() on a real failing device, and weakly
ordered hardware (the model is sequentially consistent; the fix adds
no ordering requirements beyond cmpxchg()).
Applies to mainline on its own; see the cover letter for patch 2.
lib/errseq.c | 29 ++++++++++++++++++-----------
1 file changed, 18 insertions(+), 11 deletions(-)
diff --git a/lib/errseq.c b/lib/errseq.c
index 13a2581c5a87..ea4ff9cc2166 100644
--- a/lib/errseq.c
+++ b/lib/errseq.c
@@ -162,7 +162,8 @@ EXPORT_SYMBOL(errseq_check);
* points to. If it does, then just return 0.
*
* If it doesn't, then the value has changed. Set the "seen" flag, and try to
- * swap it into place as the new eseq value. Then, set that value as the new
+ * swap it into place as the new eseq value. If the swap fails because the
+ * value changed, retry with the new value. Then, set that value as the new
* "since" value, and return whatever the error portion is set to.
*
* Note that no locking is provided here for concurrent updates to the "since"
@@ -184,24 +185,30 @@ int errseq_check_and_advance(errseq_t *eseq, errseq_t *since)
* to take the lock that protects the "since" value.
*/
old = READ_ONCE(*eseq);
- if (old != *since) {
+ while (old != *since) {
/*
* Set the flag and try to swap it into place if it has
* changed.
*
- * We don't care about the outcome of the swap here. If the
- * swap doesn't occur, then it has either been updated by a
- * writer who is altering the value in some way (updating
- * counter or resetting the error), or another reader who is
- * just setting the "seen" flag. Either outcome is OK, and we
- * can advance "since" and return an error based on what we
- * have.
+ * If the swap fails, a writer or another reader changed the
+ * value under us, so retry with the new value. "since" must
+ * only ever be set to a value that was stored with the SEEN
+ * flag: errseq_set() does not bump the counter while the
+ * flag is clear, so an unstored value could come back later
+ * and hide the errors that were recorded in the meantime.
*/
new = old | ERRSEQ_SEEN;
- if (new != old)
- cmpxchg(eseq, old, new);
+ if (new != old) {
+ errseq_t cur = cmpxchg(eseq, old, new);
+
+ if (cur != old) {
+ old = cur;
+ continue;
+ }
+ }
*since = new;
err = -(new & ERRNO_MASK);
+ break;
}
return err;
}
--
2.43.0
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH 2/2] lib/tests: errseq: add a concurrent check_and_advance test
2026-09-27 5:17 [PATCH 0/2] errseq: fix lost writeback errors in errseq_check_and_advance() Shashank Mohan Jain
2026-09-27 5:17 ` [PATCH 1/2] errseq: don't let errseq_check_and_advance() hide later errors Shashank Mohan Jain
@ 2026-09-27 5:17 ` Shashank Mohan Jain
2026-09-27 15:48 ` [PATCH 0/2] errseq: fix lost writeback errors in errseq_check_and_advance() Jeff Layton
2 siblings, 0 replies; 4+ messages in thread
From: Shashank Mohan Jain @ 2026-09-27 5:17 UTC (permalink / raw)
To: Andrew Morton
Cc: Jeff Layton, Jan Kara, NeilBrown, Thomas Maarseveen,
linux-fsdevel, linux-kernel
Add a test case that races errseq_check_and_advance() on one CPU
against errseq_set() of a different error on another CPU, then records
the first error again and lets a second cursor consume it. The second
-EIO is recorded after the first cursor's check returned, so that
cursor's next check must report an error, whichever way the race with
-ENOSPC went.
Without the previous commit 1-6% of 2 million rounds lose the error
on UML with 4 CPUs (seccomp=on ncpus=4); with it none do. The case is
skipped on a single CPU and stops after 5 seconds.
Assisted-by: LLM
Signed-off-by: Shashank Mohan Jain <jain.sm@gmail.com>
---
Needs the errseq KUnit suite from commit b52f5c1605f2 ("lib/tests: add
KUnit tests for errseq"), which is in mm-nonmm-unstable. The case is
skipped unless the kernel has at least two CPUs (on UML:
--kernel_args seccomp=on --kernel_args ncpus=4).
lib/tests/errseq_kunit.c | 110 +++++++++++++++++++++++++++++++++++++--
1 file changed, 105 insertions(+), 5 deletions(-)
diff --git a/lib/tests/errseq_kunit.c b/lib/tests/errseq_kunit.c
index 8f39ebc4a248..5ebc411ea749 100644
--- a/lib/tests/errseq_kunit.c
+++ b/lib/tests/errseq_kunit.c
@@ -2,21 +2,26 @@
/*
* KUnit tests for the errseq_t error-tracking infrastructure.
*
- * These exercise the documented single-threaded semantics of the errseq
- * API (see Documentation/core-api/errseq.rst and lib/errseq.c): error
+ * Most cases exercise the documented single-threaded semantics of the
+ * errseq API (see Documentation/core-api/errseq.rst and lib/errseq.c): error
* recording and overwriting, the "seen" handoff between errseq_sample()
* and errseq_check_and_advance(), and the re-reporting of an error that
* is recorded again after it has been seen.
*
- * The lockless properties of errseq_t under concurrent updates are
- * outside the scope of these deterministic tests, as is the WARN path
- * for invalid error values.
+ * The WARN path for invalid error values is not tested. The last case
+ * races errseq_set() against errseq_check_and_advance() on another CPU.
*/
#include <kunit/test.h>
+#include <linux/atomic.h>
+#include <linux/cpumask.h>
#include <linux/err.h>
#include <linux/errno.h>
#include <linux/errseq.h>
+#include <linux/jiffies.h>
+#include <linux/kthread.h>
+#include <linux/random.h>
+#include <linux/sched.h>
/*
* A zeroed errseq_t is the "no error has ever occurred" epoch: it
@@ -209,6 +214,100 @@ static void errseq_test_two_subscribers_independent(struct kunit *test)
KUNIT_EXPECT_EQ(test, errseq_check_and_advance(&eseq, &cursor_b), 0);
}
+/*
+ * errseq_check_and_advance() racing with errseq_set() on another CPU.
+ *
+ * Each round, cursor @f checks an unseen -EIO while another CPU records
+ * -ENOSPC. Afterwards -EIO is recorded again and cursor @g checks. The
+ * second -EIO is recorded after @f's check returned, so @f's next check
+ * must report an error, whichever way the race with -ENOSPC went.
+ */
+struct errseq_race {
+ errseq_t eseq;
+ atomic_t go; /* round the writer should run */
+ atomic_t done; /* last round the writer completed */
+ unsigned int wdelay;
+};
+
+static void errseq_race_spin(unsigned int n)
+{
+ while (n--)
+ cpu_relax();
+}
+
+static int errseq_race_writer(void *data)
+{
+ struct errseq_race *r = data;
+ int seen = 0;
+
+ while (!kthread_should_stop()) {
+ int round = atomic_read_acquire(&r->go);
+
+ if (round == seen) {
+ cond_resched();
+ continue;
+ }
+ seen = round;
+ errseq_race_spin(READ_ONCE(r->wdelay));
+ errseq_set(&r->eseq, -ENOSPC);
+ atomic_set_release(&r->done, round);
+ }
+ return 0;
+}
+
+#define ERRSEQ_RACE_ROUNDS 2000000
+#define ERRSEQ_RACE_SECONDS 5
+
+static void errseq_test_check_and_advance_race(struct kunit *test)
+{
+ struct errseq_race *r;
+ struct task_struct *writer;
+ unsigned long deadline;
+ int round, unreported = 0, missed = 0;
+
+ if (num_online_cpus() < 2)
+ kunit_skip(test, "needs at least two CPUs");
+
+ r = kunit_kzalloc(test, sizeof(*r), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, r);
+
+ writer = kthread_run(errseq_race_writer, r, "errseq_race");
+ KUNIT_ASSERT_FALSE(test, IS_ERR(writer));
+
+ deadline = jiffies + ERRSEQ_RACE_SECONDS * HZ;
+ for (round = 1; round <= ERRSEQ_RACE_ROUNDS; round++) {
+ errseq_t f = 0, g = 0;
+
+ WRITE_ONCE(r->eseq, 0);
+ errseq_set(&r->eseq, -EIO);
+ WRITE_ONCE(r->wdelay, get_random_u32_below(64));
+ atomic_set_release(&r->go, round);
+
+ errseq_race_spin(get_random_u32_below(64));
+ if (!errseq_check_and_advance(&r->eseq, &f))
+ unreported++;
+
+ while (atomic_read_acquire(&r->done) != round)
+ cond_resched();
+
+ /* Recorded after f's check returned: f must hear about it. */
+ errseq_set(&r->eseq, -EIO);
+ errseq_check_and_advance(&r->eseq, &g);
+ if (!errseq_check_and_advance(&r->eseq, &f))
+ missed++;
+
+ if (time_after(jiffies, deadline))
+ break;
+ if (!(round & 1023))
+ cond_resched();
+ }
+ kthread_stop(writer);
+
+ kunit_info(test, "%d rounds, %d missed errors\n", round - 1, missed);
+ KUNIT_EXPECT_EQ(test, unreported, 0);
+ KUNIT_EXPECT_EQ(test, missed, 0);
+}
+
static struct kunit_case errseq_test_cases[] = {
KUNIT_CASE(errseq_test_zero_epoch_reports_no_error),
KUNIT_CASE(errseq_test_set_records_error),
@@ -223,6 +322,7 @@ static struct kunit_case errseq_test_cases[] = {
KUNIT_CASE(errseq_test_repeat_error_visible_to_all_cursors),
KUNIT_CASE(errseq_test_advance_stable_when_unchanged),
KUNIT_CASE(errseq_test_two_subscribers_independent),
+ KUNIT_CASE_SLOW(errseq_test_check_and_advance_race),
{}
};
--
2.43.0
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH 0/2] errseq: fix lost writeback errors in errseq_check_and_advance()
2026-09-27 5:17 [PATCH 0/2] errseq: fix lost writeback errors in errseq_check_and_advance() Shashank Mohan Jain
2026-09-27 5:17 ` [PATCH 1/2] errseq: don't let errseq_check_and_advance() hide later errors Shashank Mohan Jain
2026-09-27 5:17 ` [PATCH 2/2] lib/tests: errseq: add a concurrent check_and_advance test Shashank Mohan Jain
@ 2026-09-27 15:48 ` Jeff Layton
2 siblings, 0 replies; 4+ messages in thread
From: Jeff Layton @ 2026-09-27 15:48 UTC (permalink / raw)
To: Shashank Mohan Jain, Andrew Morton
Cc: Jan Kara, NeilBrown, Thomas Maarseveen, linux-fsdevel, linux-kernel
On Sun, 2026-09-27 at 10:47 +0530, Shashank Mohan Jain wrote:
> errseq_check_and_advance() advances the caller's cursor to the value it
> tried to store even when its cmpxchg() failed. If errseq_set() stored a
> different errno in the meantime, the cursor holds a value that was
> never in the errseq_t. errseq_set() does not bump the counter while the
> current error is unseen, so recording the first errno again recreates
> exactly that value, and once another subscriber has marked it seen, the
> cursor's next check returns 0. For fsync() (file->f_wb_err) and
> syncfs() (sb->s_wb_err) this means that a writeback error recorded
> after the previous call returned is not reported. The race is rare: it
> needs two different errnos, an error recorded while the check runs, and
> a second subscriber consuming the value. There is no user report; it
> was found with a model checker.
>
> Patch 1 retries with the value found when the cmpxchg() fails, so the
> cursor is only ever advanced to a value that was stored with
> ERRSEQ_SEEN set. Patch 2 adds a KUnit case that races errseq_set()
> against errseq_check_and_advance() on two CPUs.
>
> Two behaviour changes are visible to callers, both intended. When a
> writer wins the race, the call now returns the newer errno (still "the
> latest error", as documented). And when two threads race on the same
> unserialised cursor (syncfs() does not lock f_sb_err), the loser may
> now return 0 where both used to return the error, so each open file
> description gets one report.
>
> Dependencies: patch 1 applies to mainline (fd179f8a05be) on its own and
> carries Cc: stable. Patch 2 extends the errseq KUnit suite from commit
> b52f5c1605f2 ("lib/tests: add KUnit tests for errseq"), which is only
> in mm-nonmm-unstable, so the series is based on mm-nonmm-unstable
> (e8d6475e77a7). Patch 1 could go through mm-hotfixes and patch 2
> through mm-nonmm-unstable.
>
> The bug was found with a TLA+ model of lib/errseq.c checked with TLC.
> The patches were prepared with Claude Code (Anthropic), model Claude
> Opus 5.5 (claude-opus-5-5).
>
> Tested:
> - KUnit on UML x86_64 (kunitconfig with CONFIG_ERRSEQ_KUNIT_TEST=y,
> CONFIG_SMP=y and CONFIG_NR_CPUS=8, run with --kernel_args seccomp=on
> --kernel_args ncpus=4). Without patch 1 the new case loses the error
> in 21,957 to 116,003 of 2,000,000 rounds (1-6% over 8 runs,
> depending on host load), in 44,886 with ncpus=2, and in 30,353 on
> UML i386 with ncpus=4. With patch 1 it loses none (8 runs with 4
> CPUs, 1 with 2 CPUs, 1 on i386). All other errseq cases pass. On a
> single CPU the new case is skipped.
> - A userspace replay of the unmodified lib/errseq.c, with a hook before
> the cmpxchg() standing in for the other CPU, loses the error every
> time.
> - TLC: the current code violates "an error recorded after a check
> returned is reported by the next check"; with patch 1 the property
> holds exhaustively for 2 subscribers x 3 checks and either 1 writer
> x 4 errors (1,364,445 distinct states) or 2 writers x 2 errors
> (141,055,253 distinct states).
> - W=1 builds of lib/errseq.o and lib/tests/errseq_kunit.o for UML
> x86_64 and i386 without warnings, and checkpatch --strict.
>
> Not tested: fsync() or syncfs() on a real failing device, weakly
> ordered hardware (the model is sequentially consistent; the fix adds no
> ordering requirement beyond cmpxchg()), and architectures other than
> UML x86_64 and i386.
>
> Shashank Mohan Jain (2):
> errseq: don't let errseq_check_and_advance() hide later errors
> lib/tests: errseq: add a concurrent check_and_advance test
>
> lib/errseq.c | 29 +++++++----
> lib/tests/errseq_kunit.c | 110 +++++++++++++++++++++++++++++++++++++--
> 2 files changed, 123 insertions(+), 16 deletions(-)
>
>
> base-commit: e8d6475e77a75b7a84e42c9d23e238e002f2758f
Nice catch. I think your analysis and patch look correct to me.
Reviewed-by: Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 4+ messages in thread