From: "Coly Li" <colyli@fygo.io>
To: "Ankit Kapoor" <ankitkap@google.com>
Cc: <linux-bcache@vger.kernel.org>, <kent.overstreet@linux.dev>,
<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 0/1] bcache: track active bypass writes to prevent stale cache reads
Date: Mon, 6 Jul 2026 15:39:38 +0800 [thread overview]
Message-ID: <aktYzuXWQ89EPM55@studio.local> (raw)
In-Reply-To: <20260617190152.3436375-1-ankitkap@google.com>
On Wed, Jun 17, 2026 at 07:01:52PM +0800, Ankit Kapoor wrote:
> Hi Coly,
>
> A quick update on my cover letter questions: after thinking more about
> those edge cases, I realized my initial reasoning was flawed and
> could lead to the exact stale-data reads this patch aims to prevent.
>
> kzalloc failures: Bypassing the counter increment on memory failure
> means the chunk is untracked. A subsequent read will incorrectly fail
> to bypass the cache, but worse, if a later write does get tracked, the
> decrements will become asymmetrical. I am wondering if a pre-allocated
> fallback pool could be a potential solution other than the hijack
> approach used by md-bitmap.
>
Yes, you may consider to use a deciate mempool to alocate these refcounter
pages with GFP_NOIO. If I missed this point in replying your patch, please
add this into my review comments.
> u16 counters: I realized clamping at U16_MAX completely breaks the
> decrement path. If an I/O burst exceeds 65,535 writes, clamping loses
> the true count. The counter will drop to 0 prematurely while writes
> are still in flight, leaving a window for stale reads.
>
Using u32 is good. For 100TB cache, the refcount pages may occupy around
100MB more or less memory. IMHO that's acceptable.
> To ensure absolute robustness, using u32 and a fallback memory pool
> seems necessary. I will hold off on sending a v3 so you can review
> the core page-lock tracking logic in v2 first and provide your
> suggestions on these two cases, but I wanted to correct my own
> reasoning before you spend time reviewing those specific questions.
>
Yes, that's what I think of at this moment. Also you should think of
using RCU in bch_has_active_bypass_writes() to replace the spin_lock on
read path. I will explain that in the patch.
Thanks.
Coly Li
next prev parent reply other threads:[~2026-07-06 7:39 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-17 10:33 [PATCH v2 0/1] bcache: track active bypass writes to prevent stale cache reads Ankit Kapoor
2026-06-17 10:33 ` [PATCH v2 1/1] " Ankit Kapoor
2026-07-07 5:14 ` Coly Li
2026-06-17 10:41 ` [PATCH v2 0/1] " Coly Li
2026-06-17 19:01 ` Ankit Kapoor
2026-07-06 7:39 ` Coly Li [this message]
2026-07-06 7:51 ` Coly Li
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aktYzuXWQ89EPM55@studio.local \
--to=colyli@fygo.io \
--cc=ankitkap@google.com \
--cc=kent.overstreet@linux.dev \
--cc=linux-bcache@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox