Message-ID: <20250806161050.iggmpp4mdmcysegn@P16.>
Date: Thu, 7 Aug 2025 00:10:50 +0800
From: Coly Li <colyli@...nel.org>
To: Zhou Jifeng <zhoujifeng@...inos.com.cn>
Cc: "kent.overstreet" <kent.overstreet@...ux.dev>,
	linux-bcache <linux-bcache@...r.kernel.org>,
	linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] bcache: enhancing the security of dirty data writeback

On Wed, Aug 06, 2025 at 07:19:49PM +0800, Zhou Jifeng wrote:
> On Wed, 6 Aug 2025 at 00:29, Coly Li <colyli@...nel.org> wrote:
> >
> > On Tue, Aug 05, 2025 at 05:37:44PM +0800, Zhou Jifeng wrote:
> > > On Tue, 5 Aug 2025 at 13:00, Coly Li <colyli@...nel.org> wrote:
> > > >
> > > > On Thu, Jul 31, 2025 at 02:21:40PM +0800, Zhou Jifeng wrote:
> > > > > There is a potential data consistency risk in bcache's writeback mode:when
> > > > > the application calls fsync, bcache returns success after completing the
> > > > > log write, persisting the cache disk data, and persisting the HDD internal
> > > > > cache. However, at this point, the actual application data may still be in
> > > > > a dirty state and remain stuck in the cache disk. when these data are
> > > > > subsequently written back to the HDD asynchronously through REQ_OP_WRITE,
> > > > > there is no forced refresh mechanism to ensure physical placement on the
> > > > > disk, and there may be no power-off protection measures, which poses a risk
> > > > > of data loss. This mechanism may cause the application to misjudge that the
> > > > > data has been persisted, which is different from the actual storage state,
> > > > > and also violates the semantic agreement that fsync should ensure data
> > > > > persistence.
> > > > >
> > > >
> > > > [snipped]
> > > >
> > > >
> > > >
> > > > If before the cleared key inserted into the btree, there are new write
> > > > into overlapped LBA range of the cleared key and a dirty key inserted.
> > > > Then the cleared key is inserted and overwrites the dirty key, but the
> > > > dirty data on cache is not written back to backing device yet. How to
> > > > handle such situation?
> > > >
> > >
> > > There are indeed some issues here. I have initially come up with a
> > > solution: Utilize the existing dc->writeback_keys mechanism for
> > > protection. The general processing flow is as follows:
> > > 1. In the write_dirty_finish() function, remove the operation of
> > > updating bkey insertion, and delete the code bch_keybuf_del(&dc
> > > ->writeback_keys, w).
> > > 2. After executing the read_dirty(dc) code, perform flush, then
> > > insert the updated bkey, and finally remove the bkey from dc->
> > > writeback_keys. This process is equivalent to sending a flush
> > > every KEYBUF_NR bkeys are written back.
> > > 3. Support configurable KEYBUF_NR to indirectly control the
> > > frequency of flush.
> > >
> > > Is this plan appropriate? Or are there any better ways to handle it?
> >
> > No, I won't suggest this way. It sounds complicaed and changes the main
> > code flow too much in an implicit way, this should be avoided.
> >
> > So it seems Kent's suggestion to flush backing device before committing
> > jset is the proper method I can see now.
> >
> > Coly Li
> >
> 
> Sorry, my previous response was not rigorous enough. I have carefully
> considered your question about "the bkey being overwritten". In fact,
> there is no issue of being overwritten. The bcache has ingeniously
> designed a replace mechanism. In my code, the bkey with the dirty flag
> cleared is inserted using the replace method. This method handles
> address overlaps ingeniously during the insertion of the bkey and will
> not overwrite the bkey generated by concurrent writes. The main code
> for the replace mechanism is located in bch_btree_insert_key->bch_extent_insert_fixup
> , which calls the bch_bkey_equal_header function, which is also a
> crucial checkpoint.

I am not able to make judgement by the above description, can you post a patch
then I can see how you insert the key with replace parameter.

Coly Li

Powered by blists - more mailing lists