All of lore.kernel.org
 help / color / mirror / Atom feed
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

  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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.