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

[Andrew Morton]

On Sun, 27 Sep 2026 10:47:24 +0530 Shashank Mohan Jain <jain.sm@xxxxxxxxx> 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).

Thanks.  Let me cc:Christian.  errseq is only used by filesystems, it
was all written by Jeff, who has approved this series.  Probably
get_maintainer should say "send this to Christian".

> 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.

fyi, I usually remove the testing info from the changelogs.  It's of
course important, but it's development-time stuff.  This info will be
terribly out of date when someone is reading these changelogs in
mainline two years from now.

I just said "Testing info is at the Link:".  So our two-years-hence
reader can chase it down if they're particularly interested.

> 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

My AI review had a few nitpicks, I think they're ignorable, depends
how enthusiastic you're feeling ;)

: The lost-error case is real, and I couldn't find a functional problem
: with the retry fix. The Fixes tag is correct and the stable Cc is
: appropriate.
: 
: A few details to fix up:
: 
: - The cover letter's "each open file description gets one report" is
:   too strong. Concurrent syncfs() calls sharing an unlocked f_sb_err
:   can still both report the error. This patch doesn't serialize that
:   cursor.
: 
: - A concurrent update on every failed retry shows progress elsewhere;
:   it doesn't guarantee that this particular caller will terminate.
:   Also, only the next setter after SEEN must increment the counter,
:   not every later setter while the value remains unseen.
: 
: - In the test, breaking on the deadline skips the loop increment, so
:   printing round - 1 undercounts the completed rounds by one.
: 
: The test also doesn't ensure that the two threads run on different
: CPUs, and its five-second check is between completed rounds rather
: than a hard timeout. Please qualify those claims.