Re: [PATCH RFC v4] bcache: flush backing device before cleaning the writeback dirty keys

> 2026=E5=B9=B47=E6=9C=889=E6=97=A5 14:23=EF=BC=8CZhou Jifeng <zhoujifeng@k=
ylinos.com.cn> =E5=86=99=E9=81=93=EF=BC=9A
>=20
> From: Zhou Jifeng <[email protected]>
>=20
> Currently, when writeback dirty data completes, write_dirty_finish()
> immediately inserts the clean key into the btree.  If a power failure
> occurs after the clean-key insert but before the backing device has
> flushed its volatile write cache, the btree contains a clean key that
> points at backing data that never reached stable media.  On the next
> read, bcache serves stale data from the backing device.
>=20
> Fix this by deferring the clean-key btree insert until after an
> explicit backing-device flush (REQ_PREFLUSH).  Written keys are first
> parked in a new rbtree (writeback_flush_pending) keyed by START_KEY.
> After enough read_dirty() passes, a full btree scan, or a wrap-around
> in refill_dirty(), writeback_finish_batch() issues a synchronous
> backing-device flush, and only on success inserts the clean keys into
> the btree and removes them from the pending tree.  On flush failure
> the entries are re-inserted into the current tree for a later retry.
>=20
> Key changes:
>=20
> - Introduce struct writeback_pending (separate kmalloc'd allocation,
>  decoupled from the dirty_io closure so the closure can be freed
>  immediately and the pending entry lives until the flush).
>=20
> - Add __writeback_pending_insert() =E2=80=94 inserts into the START_KEY-
>  ordered rbtree with overlap detection.  The rbtree is also used as
>  an interval tree by bch_writeback_drop_pending() for front-end
>  write overlap queries; that search is correct only because dirty
>  btree keys are pairwise disjoint, an invariant enforced at insert
>  time.
>=20
> - writeback_add_to_pending() parks a key after successful writeback
>  IO.  Uses GFP_NOIO because the caller runs on the WQ_MEM_RECLAIM
>  writeback workqueue.  kmalloc failure silently drops the key (it
>  stays dirty and is re-selected by the next scan).
>=20
> - writeback_finish_batch() detaches the entire pending tree into a
>  local list under spinlock, issues writeback_flush(), then either
>  cleans the keys (on success) or re-inserts them (on failure).
>=20
> - writeback_flush() open-codes blkdev_issue_flush() to call
>  bch_count_backing_io_errors() on failure.
>=20
> - bch_writeback_drop_pending() drops overlapping entries from the
>  pending tree.  Called from cached_dev_write() alongside the
>  existing bch_keybuf_check_overlapping(); overlapping front-end
>  writes now force writeback instead of bypassing the cache.
>=20
> - refill_dirty() now returns enum writeback_scan_result (PARTIAL,
>  PARTIAL_WRAP, FULL_SEARCHED) instead of a boolean.  Before
>  wrapping around for a second scan pass, it flushes the pending
>  tree so the wrap scan won't re-select keys that are still
>  KEY_DIRTY in the btree but already written to backing and
>  awaiting flush.  writeback_lock may be transiently released
>  mid-function during this pre-wrap flush.
>=20
> - refill_full_stripes() no longer wraps internally; it returns
>  at the end of the stripe range and delegates the wrap to
>  refill_dirty().  buf->last_scanned is no longer rewound past
>  the current position to avoid re-selecting keys already in
>  writeback_flush_pending.
>=20
> - read_dirty() gates deferred flush-and-clean on PASS_PER_FLUSH
>  (default 5) passes, a full btree scan, or a PARTIAL_WRAP.  It
>  accepts the scan_result from refill_dirty() as a parameter.
>=20
> - write_dirty_finish() is simplified: it no longer inserts clean
>  keys inline; it only calls writeback_add_to_pending().
>=20
> - BDEV_STATE_CLEAN eligibility in the main loop now also requires
>  writeback_pending_empty(), so a device is not marked clean while
>  entries are still in the pending tree.
>=20
> - Thread exit: the workqueue is destroyed first, then a final
>  writeback_finish_batch() is attempted (skipped when the cache
>  set is disabled or the device is detaching), then any remaining
>  pending entries are drained.  The exit path intentionally does
>  NOT set BDEV_STATE_CLEAN because it lacks the WB_SCAN_FULL_SEARCHED
>  gate that the in-loop check requires; a kthread_stop mid-scan
>  (e.g. from bch_cached_dev_attach's error path) must not mark a
>  still-dirty device clean.
>=20
> Signed-off-by: Coly Li <[email protected]>
> Signed-off-by: Zhou Jifeng <[email protected]>

You cannot add my SOB for me. I know you mean the base idea of this version=
 was
from me, but it is fine to only do your own SOB. I am good with this, since=
 you already
do a lot of analysis and testing.

The v4 version format is much more easier to review.
I will try my best to response you soon.

Thanks for the fix up and testing.

Coly Li