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

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

[Dongsheng Yang]

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);
@@ -93,10 +93,10 @@ void cache_pos_encode(struct pcache_cache *cache,
 	pos_onmedia.header.seq = seq;
 	pos_onmedia.header.crc = cache_pos_onmedia_crc(&pos_onmedia);
 
+	*index = (*index + 1) % PCACHE_META_INDEX_MAX;
+
 	memcpy_flushcache(pos_onmedia_addr, &pos_onmedia, sizeof(struct pcache_cache_pos_onmedia));
 	pmem_wmb();
-
-	*index = (*index + 1) % PCACHE_META_INDEX_MAX;
 }
 
 int cache_pos_decode(struct pcache_cache *cache,
diff --git a/drivers/md/dm-pcache/cache_segment.c b/drivers/md/dm-pcache/cache_segment.c
index f0b58980806e..ae57cc261422 100644
--- a/drivers/md/dm-pcache/cache_segment.c
+++ b/drivers/md/dm-pcache/cache_segment.c
@@ -26,11 +26,11 @@ static void cache_seg_info_write(struct pcache_cache_segment *cache_seg)
 	seg_info->header.seq++;
 	seg_info->header.crc = pcache_meta_crc(&seg_info->header, sizeof(struct pcache_segment_info));
 
+	cache_seg->info_index = (cache_seg->info_index + 1) % PCACHE_META_INDEX_MAX;
+
 	seg_info_addr = get_seg_info_addr(cache_seg);
 	memcpy_flushcache(seg_info_addr, seg_info, sizeof(struct pcache_segment_info));
 	pmem_wmb();
-
-	cache_seg->info_index = (cache_seg->info_index + 1) % PCACHE_META_INDEX_MAX;
 	mutex_unlock(&cache_seg->info_lock);
 }
 
@@ -129,10 +129,10 @@ static void cache_seg_ctrl_write(struct pcache_cache_segment *cache_seg)
 	cache_seg_gen.header.crc = pcache_meta_crc(&cache_seg_gen.header,
 						 sizeof(struct pcache_cache_seg_gen));
 
+	cache_seg->gen_index = (cache_seg->gen_index + 1) % PCACHE_META_INDEX_MAX;
+
 	memcpy_flushcache(get_cache_seg_gen_addr(cache_seg), &cache_seg_gen, sizeof(struct pcache_cache_seg_gen));
 	pmem_wmb();
-
-	cache_seg->gen_index = (cache_seg->gen_index + 1) % PCACHE_META_INDEX_MAX;
 }
 
 static void cache_seg_ctrl_init(struct pcache_cache_segment *cache_seg)
-- 
2.43.0

[PATCH 2/3] dm pcache: fix cache info indexing

[Dongsheng Yang]

From: Li Chen <chenl311@chinatelecom.cn>

The on-media cache_info index used sizeof(struct) instead of the
4K metadata stride, so gc_percent updates from dmsetup message
were written between slots and lost after reboot. Use
PCACHE_CACHE_INFO_SIZE in get_cache_info_addr() and align
info_index with the slot returned by pcache_meta_find_latest().

Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
Signed-off-by: Dongsheng Yang <dongsheng.yang@linux.dev>
---
 drivers/md/dm-pcache/cache.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/md/dm-pcache/cache.c b/drivers/md/dm-pcache/cache.c
index 9289c016b7c9..bcc4e509aa69 100644
--- a/drivers/md/dm-pcache/cache.c
+++ b/drivers/md/dm-pcache/cache.c
@@ -8,9 +8,11 @@
 
 struct kmem_cache *key_cache;
 
-static inline struct pcache_cache_info *get_cache_info_addr(struct pcache_cache *cache)
+static inline struct pcache_cache_info *
+get_cache_info_addr(struct pcache_cache *cache)
 {
-	return cache->cache_info_addr + cache->info_index;
+	return (struct pcache_cache_info *)((char *)cache->cache_info_addr +
+						(size_t)cache->info_index * PCACHE_CACHE_INFO_SIZE);
 }
 
 static void cache_info_write(struct pcache_cache *cache)
@@ -49,6 +51,8 @@ static int cache_info_init(struct pcache_cache *cache, struct pcache_cache_optio
 			return -EINVAL;
 		}
 
+		cache->info_index = ((char *)cache_info_addr - (char *)cache->cache_info_addr) / PCACHE_CACHE_INFO_SIZE;
+
 		return 0;
 	}
 
-- 
2.43.0

[PATCH 3/3] dm pcache: fix segment info indexing

[Dongsheng Yang]

From: Li Chen <chenl311@chinatelecom.cn>

Segment info indexing also used sizeof(struct) instead of the
4K metadata stride, so info_index could point between slots and
subsequent writes would advance incorrectly. Derive info_index
from the pointer returned by the segment meta search using
PCACHE_SEG_INFO_SIZE and advance to the next slot for future
updates.

Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
Signed-off-by: Dongsheng Yang <dongsheng.yang@linux.dev>
---
 drivers/md/dm-pcache/cache_segment.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/md/dm-pcache/cache_segment.c b/drivers/md/dm-pcache/cache_segment.c
index ae57cc261422..9d92e2b067ed 100644
--- a/drivers/md/dm-pcache/cache_segment.c
+++ b/drivers/md/dm-pcache/cache_segment.c
@@ -56,7 +56,10 @@ static int cache_seg_info_load(struct pcache_cache_segment *cache_seg)
 		ret = -EIO;
 		goto out;
 	}
-	cache_seg->info_index = cache_seg_info_addr - cache_seg_info_addr_base;
+
+	cache_seg->info_index =
+		((char *)cache_seg_info_addr - (char *)cache_seg_info_addr_base) /
+		PCACHE_SEG_INFO_SIZE;
 out:
 	mutex_unlock(&cache_seg->info_lock);
 
-- 
2.43.0

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

[Zheng Gu]

On Thu, Dec 4, 2025 at 10:57 AM Dongsheng Yang <dongsheng.yang@linux.dev> 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);
> @@ -93,10 +93,10 @@ void cache_pos_encode(struct pcache_cache *cache,
>         pos_onmedia.header.seq = seq;
>         pos_onmedia.header.crc = cache_pos_onmedia_crc(&pos_onmedia);
>
> +       *index = (*index + 1) % PCACHE_META_INDEX_MAX;
> +
>         memcpy_flushcache(pos_onmedia_addr, &pos_onmedia, sizeof(struct pcache_cache_pos_onmedia));
>         pmem_wmb();
> -
> -       *index = (*index + 1) % PCACHE_META_INDEX_MAX;
>  }
>
>  int cache_pos_decode(struct pcache_cache *cache,
> diff --git a/drivers/md/dm-pcache/cache_segment.c b/drivers/md/dm-pcache/cache_segment.c
> index f0b58980806e..ae57cc261422 100644
> --- a/drivers/md/dm-pcache/cache_segment.c
> +++ b/drivers/md/dm-pcache/cache_segment.c
> @@ -26,11 +26,11 @@ static void cache_seg_info_write(struct pcache_cache_segment *cache_seg)
>         seg_info->header.seq++;
>         seg_info->header.crc = pcache_meta_crc(&seg_info->header, sizeof(struct pcache_segment_info));
>
> +       cache_seg->info_index = (cache_seg->info_index + 1) % PCACHE_META_INDEX_MAX;
> +
>         seg_info_addr = get_seg_info_addr(cache_seg);
>         memcpy_flushcache(seg_info_addr, seg_info, sizeof(struct pcache_segment_info));
>         pmem_wmb();
> -
> -       cache_seg->info_index = (cache_seg->info_index + 1) % PCACHE_META_INDEX_MAX;
>         mutex_unlock(&cache_seg->info_lock);
>  }
>
> @@ -129,10 +129,10 @@ static void cache_seg_ctrl_write(struct pcache_cache_segment *cache_seg)
>         cache_seg_gen.header.crc = pcache_meta_crc(&cache_seg_gen.header,
>                                                  sizeof(struct pcache_cache_seg_gen));
>
> +       cache_seg->gen_index = (cache_seg->gen_index + 1) % PCACHE_META_INDEX_MAX;
> +
>         memcpy_flushcache(get_cache_seg_gen_addr(cache_seg), &cache_seg_gen, sizeof(struct pcache_cache_seg_gen));
>         pmem_wmb();
> -
> -       cache_seg->gen_index = (cache_seg->gen_index + 1) % PCACHE_META_INDEX_MAX;
>  }
>
>  static void cache_seg_ctrl_init(struct pcache_cache_segment *cache_seg)
> --
> 2.43.0
>

Nice catch!

Reviewed-by:  Zheng Gu <cengku@gmail.com>

Regards,
Gu

Re: [PATCH 2/3] dm pcache: fix cache info indexing

[Zheng Gu]

On Thu, Dec 4, 2025 at 10:57 AM Dongsheng Yang <dongsheng.yang@linux.dev> wrote:
>
> From: Li Chen <chenl311@chinatelecom.cn>
>
> The on-media cache_info index used sizeof(struct) instead of the
> 4K metadata stride, so gc_percent updates from dmsetup message
> were written between slots and lost after reboot. Use
> PCACHE_CACHE_INFO_SIZE in get_cache_info_addr() and align
> info_index with the slot returned by pcache_meta_find_latest().
>
> Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
> Signed-off-by: Dongsheng Yang <dongsheng.yang@linux.dev>
> ---
>  drivers/md/dm-pcache/cache.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/md/dm-pcache/cache.c b/drivers/md/dm-pcache/cache.c
> index 9289c016b7c9..bcc4e509aa69 100644
> --- a/drivers/md/dm-pcache/cache.c
> +++ b/drivers/md/dm-pcache/cache.c
> @@ -8,9 +8,11 @@
>
>  struct kmem_cache *key_cache;
>
> -static inline struct pcache_cache_info *get_cache_info_addr(struct pcache_cache *cache)
> +static inline struct pcache_cache_info *
> +get_cache_info_addr(struct pcache_cache *cache)
>  {
> -       return cache->cache_info_addr + cache->info_index;
> +       return (struct pcache_cache_info *)((char *)cache->cache_info_addr +
> +                                               (size_t)cache->info_index * PCACHE_CACHE_INFO_SIZE);
>  }
>
>  static void cache_info_write(struct pcache_cache *cache)
> @@ -49,6 +51,8 @@ static int cache_info_init(struct pcache_cache *cache, struct pcache_cache_optio
>                         return -EINVAL;
>                 }
>
> +               cache->info_index = ((char *)cache_info_addr - (char *)cache->cache_info_addr) / PCACHE_CACHE_INFO_SIZE;
> +
>                 return 0;
>         }
>
> --
> 2.43.0
>

Reviewed-by:  Zheng Gu <cengku@gmail.com>

Regards,
Gu

Re: [PATCH 3/3] dm pcache: fix segment info indexing

[Zheng Gu]

On Thu, Dec 4, 2025 at 10:57 AM Dongsheng Yang <dongsheng.yang@linux.dev> wrote:
>
> From: Li Chen <chenl311@chinatelecom.cn>
>
> Segment info indexing also used sizeof(struct) instead of the
> 4K metadata stride, so info_index could point between slots and
> subsequent writes would advance incorrectly. Derive info_index
> from the pointer returned by the segment meta search using
> PCACHE_SEG_INFO_SIZE and advance to the next slot for future
> updates.
>
> Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
> Signed-off-by: Dongsheng Yang <dongsheng.yang@linux.dev>
> ---
>  drivers/md/dm-pcache/cache_segment.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/md/dm-pcache/cache_segment.c b/drivers/md/dm-pcache/cache_segment.c
> index ae57cc261422..9d92e2b067ed 100644
> --- a/drivers/md/dm-pcache/cache_segment.c
> +++ b/drivers/md/dm-pcache/cache_segment.c
> @@ -56,7 +56,10 @@ static int cache_seg_info_load(struct pcache_cache_segment *cache_seg)
>                 ret = -EIO;
>                 goto out;
>         }
> -       cache_seg->info_index = cache_seg_info_addr - cache_seg_info_addr_base;
> +
> +       cache_seg->info_index =
> +               ((char *)cache_seg_info_addr - (char *)cache_seg_info_addr_base) /
> +               PCACHE_SEG_INFO_SIZE;
>  out:
>         mutex_unlock(&cache_seg->info_lock);
>
> --
> 2.43.0
>

Reviewed-by:  Zheng Gu <cengku@gmail.com>

Regards,
Gu

Re: [PATCH 3/3] dm pcache: fix segment info indexing

[Mikulas Patocka]

On Thu, 4 Dec 2025, Dongsheng Yang wrote:

> From: Li Chen <chenl311@chinatelecom.cn>
> 
> Segment info indexing also used sizeof(struct) instead of the
> 4K metadata stride, so info_index could point between slots and
> subsequent writes would advance incorrectly. Derive info_index
> from the pointer returned by the segment meta search using
> PCACHE_SEG_INFO_SIZE and advance to the next slot for future
> updates.
> 
> Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
> Signed-off-by: Dongsheng Yang <dongsheng.yang@linux.dev>
> ---
>  drivers/md/dm-pcache/cache_segment.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/md/dm-pcache/cache_segment.c b/drivers/md/dm-pcache/cache_segment.c
> index ae57cc261422..9d92e2b067ed 100644
> --- a/drivers/md/dm-pcache/cache_segment.c
> +++ b/drivers/md/dm-pcache/cache_segment.c
> @@ -56,7 +56,10 @@ static int cache_seg_info_load(struct pcache_cache_segment *cache_seg)
>  		ret = -EIO;
>  		goto out;
>  	}
> -	cache_seg->info_index = cache_seg_info_addr - cache_seg_info_addr_base;
> +
> +	cache_seg->info_index =
> +		((char *)cache_seg_info_addr - (char *)cache_seg_info_addr_base) /
> +		PCACHE_SEG_INFO_SIZE;
>  out:
>  	mutex_unlock(&cache_seg->info_lock);
>  

Hi

I already staged the patch below. Which patch is valid? Your new patch or 
the patch that I staged?

Mikulas



diff --git a/drivers/md/dm-pcache/cache_segment.c 
b/drivers/md/dm-pcache/cache_segment.c
index f0b58980806e..0b4bb08011ce 100644
--- a/drivers/md/dm-pcache/cache_segment.c
+++ b/drivers/md/dm-pcache/cache_segment.c
@@ -56,7 +56,11 @@ static int cache_seg_info_load(struct 
pcache_cache_segment *cache_seg)
                ret = -EIO;
                goto out;
        }
-       cache_seg->info_index = cache_seg_info_addr - cache_seg_info_addr_base;
+
+       cache_seg->info_index =
+               ((char *)cache_seg_info_addr - (char *)cache_seg_info_addr_base) /
+               PCACHE_SEG_INFO_SIZE;
+       cache_seg->info_index = (cache_seg->info_index + 1) % PCACHE_META_INDEX_MAX;
 out:
        mutex_unlock(&cache_seg->info_lock);

Re: [PATCH 2/3] dm pcache: fix cache info indexing

[Mikulas Patocka]

On Thu, 4 Dec 2025, Dongsheng Yang wrote:

> From: Li Chen <chenl311@chinatelecom.cn>
> 
> The on-media cache_info index used sizeof(struct) instead of the
> 4K metadata stride, so gc_percent updates from dmsetup message
> were written between slots and lost after reboot. Use
> PCACHE_CACHE_INFO_SIZE in get_cache_info_addr() and align
> info_index with the slot returned by pcache_meta_find_latest().
> 
> Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
> Signed-off-by: Dongsheng Yang <dongsheng.yang@linux.dev>
> ---
>  drivers/md/dm-pcache/cache.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/md/dm-pcache/cache.c b/drivers/md/dm-pcache/cache.c
> index 9289c016b7c9..bcc4e509aa69 100644
> --- a/drivers/md/dm-pcache/cache.c
> +++ b/drivers/md/dm-pcache/cache.c
> @@ -8,9 +8,11 @@
>  
>  struct kmem_cache *key_cache;
>  
> -static inline struct pcache_cache_info *get_cache_info_addr(struct pcache_cache *cache)
> +static inline struct pcache_cache_info *
> +get_cache_info_addr(struct pcache_cache *cache)
>  {
> -	return cache->cache_info_addr + cache->info_index;
> +	return (struct pcache_cache_info *)((char *)cache->cache_info_addr +
> +						(size_t)cache->info_index * PCACHE_CACHE_INFO_SIZE);
>  }
>  
>  static void cache_info_write(struct pcache_cache *cache)
> @@ -49,6 +51,8 @@ static int cache_info_init(struct pcache_cache *cache, struct pcache_cache_optio
>  			return -EINVAL;
>  		}
>  
> +		cache->info_index = ((char *)cache_info_addr - (char *)cache->cache_info_addr) / PCACHE_CACHE_INFO_SIZE;
> +
>  		return 0;
>  	}
>  
> -- 
> 2.43.0

I already staged the patch below? Should I revert it and stage your new 
patch?

Mikulas


diff --git a/drivers/md/dm-pcache/cache.c b/drivers/md/dm-pcache/cache.c
index d8e92367d947..9554d1158450 100644
--- a/drivers/md/dm-pcache/cache.c
+++ b/drivers/md/dm-pcache/cache.c
@@ -8,9 +8,11 @@

 struct kmem_cache *key_cache;

-static inline struct pcache_cache_info *get_cache_info_addr(struct pcache_cache *cache)
+static inline struct pcache_cache_info *
+get_cache_info_addr(struct pcache_cache *cache)
 {
-       return cache->cache_info_addr + cache->info_index;
+       return (struct pcache_cache_info *)((char *)cache->cache_info_addr 
+
+                                               (size_t)cache->info_index * PCACHE_CACHE_INFO_SIZE);
 }

 static void cache_info_write(struct pcache_cache *cache)
@@ -40,7 +42,12 @@ static int cache_info_init(struct pcache_cache *cache, 
struct pcache_cache_optio
        if (IS_ERR(cache_info_addr))
                return PTR_ERR(cache_info_addr);

-       if (cache_info_addr) {
+    if (cache_info_addr) {
+               int index = ((char *)cache_info_addr - (char *)cache->cache_info_addr) /
+                               PCACHE_CACHE_INFO_SIZE;
+
+               cache->info_index = (index + 1) % PCACHE_META_INDEX_MAX;
+
                if (opts->data_crc !=
                                (cache->cache_info.flags & PCACHE_CACHE_FLAGS_DATA_CRC)) {
                        pcache_dev_err(pcache, "invalid option for data_crc: %s, expected: %s",

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

[Mikulas Patocka]

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



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 - 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.

Mikulas

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

[Dongsheng Yang]

在 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

Re: [PATCH 2/3] dm pcache: fix cache info indexing

[Dongsheng Yang]

在 12/5/2025 12:01 AM, Mikulas Patocka 写道:
>
> On Thu, 4 Dec 2025, Dongsheng Yang wrote:
>
>> From: Li Chen <chenl311@chinatelecom.cn>
>>
>> The on-media cache_info index used sizeof(struct) instead of the
>> 4K metadata stride, so gc_percent updates from dmsetup message
>> were written between slots and lost after reboot. Use
>> PCACHE_CACHE_INFO_SIZE in get_cache_info_addr() and align
>> info_index with the slot returned by pcache_meta_find_latest().
>>
>> Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
>> Signed-off-by: Dongsheng Yang <dongsheng.yang@linux.dev>
>> ---
>>   drivers/md/dm-pcache/cache.c | 8 ++++++--
>>   1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/md/dm-pcache/cache.c b/drivers/md/dm-pcache/cache.c
>> index 9289c016b7c9..bcc4e509aa69 100644
>> --- a/drivers/md/dm-pcache/cache.c
>> +++ b/drivers/md/dm-pcache/cache.c
>> @@ -8,9 +8,11 @@
>>   
>>   struct kmem_cache *key_cache;
>>   
>> -static inline struct pcache_cache_info *get_cache_info_addr(struct pcache_cache *cache)
>> +static inline struct pcache_cache_info *
>> +get_cache_info_addr(struct pcache_cache *cache)
>>   {
>> -	return cache->cache_info_addr + cache->info_index;
>> +	return (struct pcache_cache_info *)((char *)cache->cache_info_addr +
>> +						(size_t)cache->info_index * PCACHE_CACHE_INFO_SIZE);
>>   }
>>   
>>   static void cache_info_write(struct pcache_cache *cache)
>> @@ -49,6 +51,8 @@ static int cache_info_init(struct pcache_cache *cache, struct pcache_cache_optio
>>   			return -EINVAL;
>>   		}
>>   
>> +		cache->info_index = ((char *)cache_info_addr - (char *)cache->cache_info_addr) / PCACHE_CACHE_INFO_SIZE;
>> +
>>   		return 0;
>>   	}
>>   
>> -- 
>> 2.43.0
> I already staged the patch below? Should I revert it and stage your new
> patch?


Yes, please revert it, I will send a v2 patchset, please pick the v2 patch.


Thanx

>
> Mikulas
>
>
> diff --git a/drivers/md/dm-pcache/cache.c b/drivers/md/dm-pcache/cache.c
> index d8e92367d947..9554d1158450 100644
> --- a/drivers/md/dm-pcache/cache.c
> +++ b/drivers/md/dm-pcache/cache.c
> @@ -8,9 +8,11 @@
>
>   struct kmem_cache *key_cache;
>
> -static inline struct pcache_cache_info *get_cache_info_addr(struct pcache_cache *cache)
> +static inline struct pcache_cache_info *
> +get_cache_info_addr(struct pcache_cache *cache)
>   {
> -       return cache->cache_info_addr + cache->info_index;
> +       return (struct pcache_cache_info *)((char *)cache->cache_info_addr
> +
> +                                               (size_t)cache->info_index * PCACHE_CACHE_INFO_SIZE);
>   }
>
>   static void cache_info_write(struct pcache_cache *cache)
> @@ -40,7 +42,12 @@ static int cache_info_init(struct pcache_cache *cache,
> struct pcache_cache_optio
>          if (IS_ERR(cache_info_addr))
>                  return PTR_ERR(cache_info_addr);
>
> -       if (cache_info_addr) {
> +    if (cache_info_addr) {
> +               int index = ((char *)cache_info_addr - (char *)cache->cache_info_addr) /
> +                               PCACHE_CACHE_INFO_SIZE;
> +
> +               cache->info_index = (index + 1) % PCACHE_META_INDEX_MAX;
> +
>                  if (opts->data_crc !=
>                                  (cache->cache_info.flags & PCACHE_CACHE_FLAGS_DATA_CRC)) {
>                          pcache_dev_err(pcache, "invalid option for data_crc: %s, expected: %s",
>

Re: [PATCH 3/3] dm pcache: fix segment info indexing

[Dongsheng Yang]

在 12/4/2025 11:59 PM, Mikulas Patocka 写道:
>
> On Thu, 4 Dec 2025, Dongsheng Yang wrote:
>
>> From: Li Chen <chenl311@chinatelecom.cn>
>>
>> Segment info indexing also used sizeof(struct) instead of the
>> 4K metadata stride, so info_index could point between slots and
>> subsequent writes would advance incorrectly. Derive info_index
>> from the pointer returned by the segment meta search using
>> PCACHE_SEG_INFO_SIZE and advance to the next slot for future
>> updates.
>>
>> Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
>> Signed-off-by: Dongsheng Yang <dongsheng.yang@linux.dev>
>> ---
>>   drivers/md/dm-pcache/cache_segment.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/md/dm-pcache/cache_segment.c b/drivers/md/dm-pcache/cache_segment.c
>> index ae57cc261422..9d92e2b067ed 100644
>> --- a/drivers/md/dm-pcache/cache_segment.c
>> +++ b/drivers/md/dm-pcache/cache_segment.c
>> @@ -56,7 +56,10 @@ static int cache_seg_info_load(struct pcache_cache_segment *cache_seg)
>>   		ret = -EIO;
>>   		goto out;
>>   	}
>> -	cache_seg->info_index = cache_seg_info_addr - cache_seg_info_addr_base;
>> +
>> +	cache_seg->info_index =
>> +		((char *)cache_seg_info_addr - (char *)cache_seg_info_addr_base) /
>> +		PCACHE_SEG_INFO_SIZE;
>>   out:
>>   	mutex_unlock(&cache_seg->info_lock);
>>   
> Hi
>
> I already staged the patch below. Which patch is valid? Your new patch or
> the patch that I staged?


Yes, please revert it, I will send a v2 patchset, please pick the v2 patch.


Thanx


>
> Mikulas
>
>
>
> diff --git a/drivers/md/dm-pcache/cache_segment.c
> b/drivers/md/dm-pcache/cache_segment.c
> index f0b58980806e..0b4bb08011ce 100644
> --- a/drivers/md/dm-pcache/cache_segment.c
> +++ b/drivers/md/dm-pcache/cache_segment.c
> @@ -56,7 +56,11 @@ static int cache_seg_info_load(struct
> pcache_cache_segment *cache_seg)
>                  ret = -EIO;
>                  goto out;
>          }
> -       cache_seg->info_index = cache_seg_info_addr - cache_seg_info_addr_base;
> +
> +       cache_seg->info_index =
> +               ((char *)cache_seg_info_addr - (char *)cache_seg_info_addr_base) /
> +               PCACHE_SEG_INFO_SIZE;
> +       cache_seg->info_index = (cache_seg->info_index + 1) % PCACHE_META_INDEX_MAX;
>   out:
>          mutex_unlock(&cache_seg->info_lock);
>
>

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

[Mikulas Patocka]

[-- 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