Re: [PATCH 1/3] dm-pcache: advance slot index before writing slot

[Mikulas Patocka <mpatocka@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2379 bytes --]



On Fri, 5 Dec 2025, Dongsheng Yang wrote:

> 
> 在 12/5/2025 12:13 AM, Mikulas Patocka 写道:
> > 
> > On Thu, 4 Dec 2025, Dongsheng Yang wrote:
> > 
> > > In dm-pcache, in order to ensure crash-consistency, a dual-copy scheme
> > > is used to alternately update metadata, and there is a slot index that
> > > records the current slot. However, in the write path the current
> > > implementation writes directly to the current slot indexed by slot
> > > index, and then advances the slot — which ends up overwriting the
> > > existing slot, violating the crash-consistency guarantee.
> > > 
> > > This patch fixes that behavior, preventing metadata from being
> > > overwritten incorrectly.
> > > 
> > > Signed-off-by: Dongsheng Yang <dongsheng.yang@linux.dev>
> > > ---
> > >   drivers/md/dm-pcache/cache.c         | 8 ++++----
> > >   drivers/md/dm-pcache/cache_segment.c | 8 ++++----
> > >   2 files changed, 8 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/drivers/md/dm-pcache/cache.c b/drivers/md/dm-pcache/cache.c
> > > index 698697a7a73c..9289c016b7c9 100644
> > > --- a/drivers/md/dm-pcache/cache.c
> > > +++ b/drivers/md/dm-pcache/cache.c
> > > @@ -21,10 +21,10 @@ static void cache_info_write(struct pcache_cache
> > > *cache)
> > >   	cache_info->header.crc = pcache_meta_crc(&cache_info->header,
> > >   						sizeof(struct
> > > pcache_cache_info));
> > >   +	cache->info_index = (cache->info_index + 1) % PCACHE_META_INDEX_MAX;
> > > +
> > >   	memcpy_flushcache(get_cache_info_addr(cache), cache_info,
> > >   			sizeof(struct pcache_cache_info));
> > > -
> > > -	cache->info_index = (cache->info_index + 1) % PCACHE_META_INDEX_MAX;
> > >   }
> > >     static void cache_info_init_default(struct pcache_cache *cache);
> > I'm not sure whether this is bug or not
> 
> 
> It's a bug, if in-place updating for slot happen, there is a potential
> metadata corruption while powercut happening at the same time.
> 
> 
> so I will add
> 
> Cc: stable@vger.kernel.org      # 6.18
> 
> in V2.
> 
> > - but memcpy_flushcache doesn't
> > guarantee that the cache will be flushed (despite it's name) or that
> > writes would be ordered.
> > 
> > You need pmem_wmb(). I see that you are using it at other places.
> 
> 
> Good catch, I will send v2 to add the missing pmem_wmb().
> 
> 
> Thanx

OK. I staged all 3 patches and I'll send to Linus soon.

Mikulas