Re: [PATCH 0/2] errseq: fix lost writeback errors in errseq_check_and_advance()

[Jeff Layton]

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@xxxxxxxxxx>