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

[Dongsheng Yang <dongsheng.yang@linux.dev>]

在 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

>
> Mikulas