Re: [PATCH 1/1] bcache: fix stale data race between read cache miss and bypass write

[Coly Li]

Hi Ankit,

Yes, I confirm this is an issue that must be solved. Nice catch!

On Thu, May 21, 2026 at 04:39:25PM +0800, Ankit Kapoor wrote:
> A race condition exists between a read cache miss and a bypass write
> due to either congestion or sequential bypass, that causes stale data
> to be cached when the read cache miss runs concurrently with a bypass
> write targeting the same sectors. If the read cache miss fetches data
> from the backing device before the write to the backing device,
> stale data populates the cache.
> 
> The root cause is that bcache currently executes btree key
> invalidation in parallel with (or prior to) writing the actual data
> payload to the backing device. Under this sequence, a concurrent
> read path can register a cache miss and insert a placeholder key.
> If the write's btree key invalidation completes before the read finishes
> fetching old data from the backing device, the read's subsequent
> key replacement will not detect a collision, allowing stale data
> to persist in the cache.
> 
> Fix this by deferring the btree key invalidation until after the
> backing device write completes successfully. Enforcing this
> sequential execution ensures that a stale read is always detected
> and invalidated.
>

This patch fixes the stale data issue in run time, but if power failure
happens inside the race window, after boot up again, the stale data
still exists in cache for following read hits.

And your fix invalidate the key after on-disk bio completed, which makes
such stale data window by power failure longer.
 
To solve all the stale data race both for run time and power failure
condition, could you please consider the following proposal.

Maintain a data structure to hold all invalidate range from by-pass
write, record/insert the invalidation range before bch_data_insert(),
and after cached_dev_write_complete(), clear/remove the invalidation
range.

For a cache-miss read, if there is any invalidation range refcount
exists, check all non-zero refcount ranges, if any range overlaps with
the cache-miss read range, do NOT update the missing bkey back to btree
and only read data from backing device.

Here you need to design a efficient data structure both for performance
and memory consumption. I would sugguest to maintain chunk refcounts
which mapping multiple 32MB ranges on cache device (current max key size
if I remember correctly) range. You may look at how md raid maintains
the legacy bitmap refcount, hope that code can give you any hint.

Finally thank you for report this issue, nick catch!

Coly Li


> Signed-off-by: Ankit Kapoor <ankitkap@xxxxxxxxxx>
> ---
>  drivers/md/bcache/request.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/md/bcache/request.c b/drivers/md/bcache/request.c
> index af345dc6fde1..ef2cf55df3bb 100644
> --- a/drivers/md/bcache/request.c
> +++ b/drivers/md/bcache/request.c
> @@ -978,6 +978,14 @@ static CLOSURE_CALLBACK(cached_dev_write_complete)
>  	cached_dev_bio_complete(&cl->work);
>  }
>  
> +static CLOSURE_CALLBACK(backing_device_bypass_write_complete)
> +{
> +	closure_type(s, struct search, cl);
> +
> +	closure_call(&s->iop.cl, bch_data_insert, NULL, cl);
> +	continue_at(cl, cached_dev_write_complete, NULL);
> +}
> +
>  static void cached_dev_write(struct cached_dev *dc, struct search *s)
>  {
>  	struct closure *cl = &s->cl;
> @@ -1058,6 +1066,11 @@ static void cached_dev_write(struct cached_dev *dc, struct search *s)
>  	}
>  
>  insert_data:
> +	if (s->iop.bypass) {
> +		continue_at(cl, backing_device_bypass_write_complete, NULL);
> +		return;
> +	}
> +
>  	closure_call(&s->iop.cl, bch_data_insert, NULL, cl);
>  	continue_at(cl, cached_dev_write_complete, NULL);
>  }
> -- 
> 2.54.0.669.g59709faab0-goog