public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] dm-pcache: fix metadata indexing and persistence issues
@ 2025-12-02 12:11 Li Chen
  2025-12-02 12:11 ` [PATCH 1/2] dm pcache: fix cache info indexing Li Chen
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Li Chen @ 2025-12-02 12:11 UTC (permalink / raw)
  To: dm-devel, linux-kernel, Dongsheng Yang, Zheng Gu

From: Li Chen <chenl311@chinatelecom.cn>

dm-pcache stores metadata (cache_info and segment_info) in 4K-aligned
slots on the cache device, using sequence numbers and CRC to identify
the latest valid copy.

However, the cache_info and segment_info paths were computing their
on-media index using sizeof(struct) instead of the 4K metadata stride.
As a result:

  * cache_info updates (including gc_percent set via a dmsetup message)
    were written to invalid offsets between metadata slots and failed
    to persist across table reloads or reboots.

  * segment_info indexing became desynchronized, so rotation to the
    "next" slot no longer matched the location returned by
    pcache_meta_find_latest().

The issue can be reproduced with:

  dmsetup create pcache_vdb --table \
    "0 $(blockdev --getsz /dev/vdb) pcache /dev/pmem0 /dev/vdb 4 \
     cache_mode writeback data_crc false"

  # Check default gc_percent (70)
  dmsetup status pcache_vdb

  # Change gc_percent to 10
  dmsetup message pcache_vdb 0 "gc_percent 10"

  # Verify change is active in memory
  dmsetup status pcache_vdb

  # Reboot the system...

  # Without patch (gc_percent reverts to 70):
  dmsetup status pcache_vdb

  # With patch (gc_percent persists as 10):
  dmsetup status pcache_vdb

This series fixes the issue by deriving the metadata slot index from
the pointer returned by pcache_meta_find_latest(), using the 4K stride
(CACHE_INFO_SIZE / SEG_INFO_SIZE). This ensures that updates to
cache_info and segment_info are written to valid slots and remain
consistent with the on-media layout.

Li Chen (2):
  dm pcache: fix cache info indexing
  dm pcache: fix segment info indexing

 drivers/md/dm-pcache/cache.c         | 13 ++++++++++---
 drivers/md/dm-pcache/cache_segment.c |  6 +++++-
 2 files changed, 15 insertions(+), 4 deletions(-)

-- 
2.51.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/2] dm pcache: fix cache info indexing
  2025-12-02 12:11 [PATCH 0/2] dm-pcache: fix metadata indexing and persistence issues Li Chen
@ 2025-12-02 12:11 ` Li Chen
  2025-12-03  5:56   ` Dongsheng Yang
  2025-12-02 12:11 ` [PATCH 2/2] dm pcache: fix segment " Li Chen
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Li Chen @ 2025-12-02 12:11 UTC (permalink / raw)
  To: dm-devel, linux-kernel, Dongsheng Yang, Zheng Gu

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>
---
 drivers/md/dm-pcache/cache.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/md/dm-pcache/cache.c b/drivers/md/dm-pcache/cache.c
index 698697a7a73c..6d5001548628 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",
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/2] dm pcache: fix segment info indexing
  2025-12-02 12:11 [PATCH 0/2] dm-pcache: fix metadata indexing and persistence issues Li Chen
  2025-12-02 12:11 ` [PATCH 1/2] dm pcache: fix cache info indexing Li Chen
@ 2025-12-02 12:11 ` Li Chen
  2025-12-03  5:57   ` Dongsheng Yang
  2025-12-02 19:09 ` [PATCH 0/2] dm-pcache: fix metadata indexing and persistence issues Mikulas Patocka
  2025-12-03  5:56 ` Dongsheng Yang
  3 siblings, 1 reply; 12+ messages in thread
From: Li Chen @ 2025-12-02 12:11 UTC (permalink / raw)
  To: dm-devel, linux-kernel, Dongsheng Yang, Zheng Gu

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>
---
 drivers/md/dm-pcache/cache_segment.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

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);
 
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] dm-pcache: fix metadata indexing and persistence issues
  2025-12-02 12:11 [PATCH 0/2] dm-pcache: fix metadata indexing and persistence issues Li Chen
  2025-12-02 12:11 ` [PATCH 1/2] dm pcache: fix cache info indexing Li Chen
  2025-12-02 12:11 ` [PATCH 2/2] dm pcache: fix segment " Li Chen
@ 2025-12-02 19:09 ` Mikulas Patocka
  2025-12-03  5:56 ` Dongsheng Yang
  3 siblings, 0 replies; 12+ messages in thread
From: Mikulas Patocka @ 2025-12-02 19:09 UTC (permalink / raw)
  To: Li Chen; +Cc: dm-devel, linux-kernel, Dongsheng Yang, Zheng Gu

Both patches accepted, thanks.

Mikulas



On Tue, 2 Dec 2025, Li Chen wrote:

> From: Li Chen <chenl311@chinatelecom.cn>
> 
> dm-pcache stores metadata (cache_info and segment_info) in 4K-aligned
> slots on the cache device, using sequence numbers and CRC to identify
> the latest valid copy.
> 
> However, the cache_info and segment_info paths were computing their
> on-media index using sizeof(struct) instead of the 4K metadata stride.
> As a result:
> 
>   * cache_info updates (including gc_percent set via a dmsetup message)
>     were written to invalid offsets between metadata slots and failed
>     to persist across table reloads or reboots.
> 
>   * segment_info indexing became desynchronized, so rotation to the
>     "next" slot no longer matched the location returned by
>     pcache_meta_find_latest().
> 
> The issue can be reproduced with:
> 
>   dmsetup create pcache_vdb --table \
>     "0 $(blockdev --getsz /dev/vdb) pcache /dev/pmem0 /dev/vdb 4 \
>      cache_mode writeback data_crc false"
> 
>   # Check default gc_percent (70)
>   dmsetup status pcache_vdb
> 
>   # Change gc_percent to 10
>   dmsetup message pcache_vdb 0 "gc_percent 10"
> 
>   # Verify change is active in memory
>   dmsetup status pcache_vdb
> 
>   # Reboot the system...
> 
>   # Without patch (gc_percent reverts to 70):
>   dmsetup status pcache_vdb
> 
>   # With patch (gc_percent persists as 10):
>   dmsetup status pcache_vdb
> 
> This series fixes the issue by deriving the metadata slot index from
> the pointer returned by pcache_meta_find_latest(), using the 4K stride
> (CACHE_INFO_SIZE / SEG_INFO_SIZE). This ensures that updates to
> cache_info and segment_info are written to valid slots and remain
> consistent with the on-media layout.
> 
> Li Chen (2):
>   dm pcache: fix cache info indexing
>   dm pcache: fix segment info indexing
> 
>  drivers/md/dm-pcache/cache.c         | 13 ++++++++++---
>  drivers/md/dm-pcache/cache_segment.c |  6 +++++-
>  2 files changed, 15 insertions(+), 4 deletions(-)
> 
> -- 
> 2.51.0
> 
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] dm-pcache: fix metadata indexing and persistence issues
  2025-12-02 12:11 [PATCH 0/2] dm-pcache: fix metadata indexing and persistence issues Li Chen
                   ` (2 preceding siblings ...)
  2025-12-02 19:09 ` [PATCH 0/2] dm-pcache: fix metadata indexing and persistence issues Mikulas Patocka
@ 2025-12-03  5:56 ` Dongsheng Yang
  2025-12-03 23:38   ` Dongsheng Yang
  3 siblings, 1 reply; 12+ messages in thread
From: Dongsheng Yang @ 2025-12-03  5:56 UTC (permalink / raw)
  To: Li Chen, dm-devel, linux-kernel, Zheng Gu

Hi,

     Thanx for your patches, there are some comments inline.

According to these comments, I propose an update base on your patch as [1].

BTW, I added a test case for this problem in dtg-tests:

https://github.com/DataTravelGuide/dtg-tests/blob/main/pcache.py.data/pcache_misc_tests/case21_gc_percent_persistence_after_recreate.sh

It update gc_percent online and recreate pcache device and check gc_percen


[1]:

diff --git a/drivers/md/dm-pcache/cache.c b/drivers/md/dm-pcache/cache.c
index 6d5001548628..4d6db733c9bd 100644
--- a/drivers/md/dm-pcache/cache.c
+++ b/drivers/md/dm-pcache/cache.c
@@ -42,12 +42,7 @@ 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) {
-               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 (cache_info_addr) {
                 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",
@@ -56,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;
         }

diff --git a/drivers/md/dm-pcache/cache_segment.c 
b/drivers/md/dm-pcache/cache_segment.c
index 0b4bb08011ce..0a0016702ce7 100644
--- a/drivers/md/dm-pcache/cache_segment.c
+++ b/drivers/md/dm-pcache/cache_segment.c
@@ -60,7 +60,6 @@ static int cache_seg_info_load(struct 
pcache_cache_segment *cache_seg)
         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);


在 12/2/2025 8:11 PM, Li Chen 写道:
> From: Li Chen <chenl311@chinatelecom.cn>
>
> dm-pcache stores metadata (cache_info and segment_info) in 4K-aligned
> slots on the cache device, using sequence numbers and CRC to identify
> the latest valid copy.
>
> However, the cache_info and segment_info paths were computing their
> on-media index using sizeof(struct) instead of the 4K metadata stride.
> As a result:
>
>    * cache_info updates (including gc_percent set via a dmsetup message)
>      were written to invalid offsets between metadata slots and failed
>      to persist across table reloads or reboots.
>
>    * segment_info indexing became desynchronized, so rotation to the
>      "next" slot no longer matched the location returned by
>      pcache_meta_find_latest().
>
> The issue can be reproduced with:
>
>    dmsetup create pcache_vdb --table \
>      "0 $(blockdev --getsz /dev/vdb) pcache /dev/pmem0 /dev/vdb 4 \
>       cache_mode writeback data_crc false"
>
>    # Check default gc_percent (70)
>    dmsetup status pcache_vdb
>
>    # Change gc_percent to 10
>    dmsetup message pcache_vdb 0 "gc_percent 10"
>
>    # Verify change is active in memory
>    dmsetup status pcache_vdb
>
>    # Reboot the system...
>
>    # Without patch (gc_percent reverts to 70):
>    dmsetup status pcache_vdb
>
>    # With patch (gc_percent persists as 10):
>    dmsetup status pcache_vdb
>
> This series fixes the issue by deriving the metadata slot index from
> the pointer returned by pcache_meta_find_latest(), using the 4K stride
> (CACHE_INFO_SIZE / SEG_INFO_SIZE). This ensures that updates to
> cache_info and segment_info are written to valid slots and remain
> consistent with the on-media layout.
>
> Li Chen (2):
>    dm pcache: fix cache info indexing
>    dm pcache: fix segment info indexing
>
>   drivers/md/dm-pcache/cache.c         | 13 ++++++++++---
>   drivers/md/dm-pcache/cache_segment.c |  6 +++++-
>   2 files changed, 15 insertions(+), 4 deletions(-)
>

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] dm pcache: fix cache info indexing
  2025-12-02 12:11 ` [PATCH 1/2] dm pcache: fix cache info indexing Li Chen
@ 2025-12-03  5:56   ` Dongsheng Yang
  2025-12-03 23:38     ` Dongsheng Yang
  0 siblings, 1 reply; 12+ messages in thread
From: Dongsheng Yang @ 2025-12-03  5:56 UTC (permalink / raw)
  To: Li Chen, dm-devel, linux-kernel, Zheng Gu


在 12/2/2025 8:11 PM, Li Chen 写道:
> 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>
> ---
>   drivers/md/dm-pcache/cache.c | 13 ++++++++++---
>   1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/md/dm-pcache/cache.c b/drivers/md/dm-pcache/cache.c
> index 698697a7a73c..6d5001548628 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) {


this is unrelated change, and it introduce error in checkpatch.pl

# ./scripts/checkpatch.pl drivers/md/dm-pcache/cache.c
WARNING: please, no spaces at the start of a line
#45: FILE: drivers/md/dm-pcache/cache.c:45:
+    if (cache_info_addr) {$

WARNING: suspect code indent for conditional statements (4, 16)
#45: FILE: drivers/md/dm-pcache/cache.c:45:
+    if (cache_info_addr) {
+               int index = ((char *)cache_info_addr - (char 
*)cache->cache_info_addr) /

total: 0 errors, 2 warnings, 452 lines checked

> +		int index = ((char *)cache_info_addr - (char *)cache->cache_info_addr) /
> +				PCACHE_CACHE_INFO_SIZE;
> +
> +		cache->info_index = (index + 1) % PCACHE_META_INDEX_MAX;


Don't advance info_index at init stage, cache->info_index means the 
current index, not the next index.

It will be advanced in cache_info_write() after write cache_info into 
next slot.

In addition, cache->info_index initialization should be after the 
data_crc checking.

Thanx

> +
>   		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",

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] dm pcache: fix segment info indexing
  2025-12-02 12:11 ` [PATCH 2/2] dm pcache: fix segment " Li Chen
@ 2025-12-03  5:57   ` Dongsheng Yang
  2025-12-03 23:38     ` Dongsheng Yang
  0 siblings, 1 reply; 12+ messages in thread
From: Dongsheng Yang @ 2025-12-03  5:57 UTC (permalink / raw)
  To: Li Chen, dm-devel, linux-kernel, Zheng Gu


在 12/2/2025 8:11 PM, Li Chen 写道:
> 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>
> ---
>   drivers/md/dm-pcache/cache_segment.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
>
> 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;


Don't advance info_index at init stage.

>   out:
>   	mutex_unlock(&cache_seg->info_lock);
>   

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] dm pcache: fix cache info indexing
  2025-12-03  5:56   ` Dongsheng Yang
@ 2025-12-03 23:38     ` Dongsheng Yang
  0 siblings, 0 replies; 12+ messages in thread
From: Dongsheng Yang @ 2025-12-03 23:38 UTC (permalink / raw)
  To: Li Chen, dm-devel, linux-kernel, Zheng Gu


在 12/3/2025 1:56 PM, Dongsheng Yang 写道:
>
> 在 12/2/2025 8:11 PM, Li Chen 写道:
>> From: Li Chen <chenl311@chinatelecom.cn>

>>
>
>> +        int index = ((char *)cache_info_addr - (char 
>> *)cache->cache_info_addr) /
>> +                PCACHE_CACHE_INFO_SIZE;
>> +
>> +        cache->info_index = (index + 1) % PCACHE_META_INDEX_MAX;
>
>
> Don't advance info_index at init stage, cache->info_index means the 
> current index, not the next index.
>
> It will be advanced in cache_info_write() after write cache_info into 
> next slot.


My bad, ->info_index means next slot, so advance it is correct.

>
> In addition, cache->info_index initialization should be after the 
> data_crc checking.
>
> Thanx
>
>> +
>>           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",
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] dm pcache: fix segment info indexing
  2025-12-03  5:57   ` Dongsheng Yang
@ 2025-12-03 23:38     ` Dongsheng Yang
  0 siblings, 0 replies; 12+ messages in thread
From: Dongsheng Yang @ 2025-12-03 23:38 UTC (permalink / raw)
  To: Li Chen, dm-devel, linux-kernel, Zheng Gu


在 12/3/2025 1:57 PM, Dongsheng Yang 写道:
>
> 在 12/2/2025 8:11 PM, Li Chen 写道:
>> 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>
>> ---
>>   drivers/md/dm-pcache/cache_segment.c | 6 +++++-
>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> 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;
>
>
> Don't advance info_index at init stage.


ignore this incorrect comment, info_index means next slot, we need to 
advance it here.

>
>>   out:
>>       mutex_unlock(&cache_seg->info_lock);
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] dm-pcache: fix metadata indexing and persistence issues
  2025-12-03  5:56 ` Dongsheng Yang
@ 2025-12-03 23:38   ` Dongsheng Yang
  2025-12-04  1:38     ` Dongsheng Yang
  0 siblings, 1 reply; 12+ messages in thread
From: Dongsheng Yang @ 2025-12-03 23:38 UTC (permalink / raw)
  To: Li Chen, dm-devel, linux-kernel, Zheng Gu


在 12/3/2025 1:56 PM, Dongsheng Yang 写道:
> Hi,
>
>     Thanx for your patches, there are some comments inline.
>
> According to these comments, I propose an update base on your patch as 
> [1].
>
> BTW, I added a test case for this problem in dtg-tests:
>
> https://github.com/DataTravelGuide/dtg-tests/blob/main/pcache.py.data/pcache_misc_tests/case21_gc_percent_persistence_after_recreate.sh 
>
>
> It update gc_percent online and recreate pcache device and check 
> gc_percen
>
>
> [1]:


->info_index means next slot to write, so advancing it at init stage is 
correct


Thanx

>
> diff --git a/drivers/md/dm-pcache/cache.c b/drivers/md/dm-pcache/cache.c
> index 6d5001548628..4d6db733c9bd 100644
> --- a/drivers/md/dm-pcache/cache.c
> +++ b/drivers/md/dm-pcache/cache.c
> @@ -42,12 +42,7 @@ 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) {
> -               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 (cache_info_addr) {
>                 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",
> @@ -56,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;
>         }
>
> diff --git a/drivers/md/dm-pcache/cache_segment.c 
> b/drivers/md/dm-pcache/cache_segment.c
> index 0b4bb08011ce..0a0016702ce7 100644
> --- a/drivers/md/dm-pcache/cache_segment.c
> +++ b/drivers/md/dm-pcache/cache_segment.c
> @@ -60,7 +60,6 @@ static int cache_seg_info_load(struct 
> pcache_cache_segment *cache_seg)
>         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);
>
>
> 在 12/2/2025 8:11 PM, Li Chen 写道:
>> From: Li Chen <chenl311@chinatelecom.cn>
>>
>> dm-pcache stores metadata (cache_info and segment_info) in 4K-aligned
>> slots on the cache device, using sequence numbers and CRC to identify
>> the latest valid copy.
>>
>> However, the cache_info and segment_info paths were computing their
>> on-media index using sizeof(struct) instead of the 4K metadata stride.
>> As a result:
>>
>>    * cache_info updates (including gc_percent set via a dmsetup message)
>>      were written to invalid offsets between metadata slots and failed
>>      to persist across table reloads or reboots.
>>
>>    * segment_info indexing became desynchronized, so rotation to the
>>      "next" slot no longer matched the location returned by
>>      pcache_meta_find_latest().
>>
>> The issue can be reproduced with:
>>
>>    dmsetup create pcache_vdb --table \
>>      "0 $(blockdev --getsz /dev/vdb) pcache /dev/pmem0 /dev/vdb 4 \
>>       cache_mode writeback data_crc false"
>>
>>    # Check default gc_percent (70)
>>    dmsetup status pcache_vdb
>>
>>    # Change gc_percent to 10
>>    dmsetup message pcache_vdb 0 "gc_percent 10"
>>
>>    # Verify change is active in memory
>>    dmsetup status pcache_vdb
>>
>>    # Reboot the system...
>>
>>    # Without patch (gc_percent reverts to 70):
>>    dmsetup status pcache_vdb
>>
>>    # With patch (gc_percent persists as 10):
>>    dmsetup status pcache_vdb
>>
>> This series fixes the issue by deriving the metadata slot index from
>> the pointer returned by pcache_meta_find_latest(), using the 4K stride
>> (CACHE_INFO_SIZE / SEG_INFO_SIZE). This ensures that updates to
>> cache_info and segment_info are written to valid slots and remain
>> consistent with the on-media layout.
>>
>> Li Chen (2):
>>    dm pcache: fix cache info indexing
>>    dm pcache: fix segment info indexing
>>
>>   drivers/md/dm-pcache/cache.c         | 13 ++++++++++---
>>   drivers/md/dm-pcache/cache_segment.c |  6 +++++-
>>   2 files changed, 15 insertions(+), 4 deletions(-)
>>
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] dm-pcache: fix metadata indexing and persistence issues
  2025-12-03 23:38   ` Dongsheng Yang
@ 2025-12-04  1:38     ` Dongsheng Yang
  2025-12-05  9:12       ` Li Chen
  0 siblings, 1 reply; 12+ messages in thread
From: Dongsheng Yang @ 2025-12-04  1:38 UTC (permalink / raw)
  To: Li Chen, dm-devel, linux-kernel, Zheng Gu


在 12/4/2025 7:38 AM, Dongsheng Yang 写道:
>
> 在 12/3/2025 1:56 PM, Dongsheng Yang 写道:
>> Hi,
>>
>>     Thanx for your patches, there are some comments inline.
>>
>> According to these comments, I propose an update base on your patch 
>> as [1].
>>
>> BTW, I added a test case for this problem in dtg-tests:
>>
>> https://github.com/DataTravelGuide/dtg-tests/blob/main/pcache.py.data/pcache_misc_tests/case21_gc_percent_persistence_after_recreate.sh 
>>
>>
>> It update gc_percent online and recreate pcache device and check 
>> gc_percen
>>
>>
>> [1]:
>
>
> ->info_index means next slot to write, so advancing it at init stage 
> is correct


We need to think more thoroughly about the usage of the slot index. In 
my original design, all slot indices represent the “current slot”. So at 
initialization it is 0; if loading, we simply point it to the loaded 
slot; and on write we advance afterwards. However, the problem now is 
that in all write functions, they first write to the current slot, then 
advance.

Therefore I believe we should clarify that the meaning of the slot index 
is “the current slot,” and stick to the original design. We only need to 
modify the write functions: first perform the advance, and then write 
into the correct slot.

>
>
> Thanx
>
>>
>> diff --git a/drivers/md/dm-pcache/cache.c b/drivers/md/dm-pcache/cache.c
>> index 6d5001548628..4d6db733c9bd 100644
>> --- a/drivers/md/dm-pcache/cache.c
>> +++ b/drivers/md/dm-pcache/cache.c
>> @@ -42,12 +42,7 @@ 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) {
>> -               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 (cache_info_addr) {
>>                 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",
>> @@ -56,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;
>>         }
>>
>> diff --git a/drivers/md/dm-pcache/cache_segment.c 
>> b/drivers/md/dm-pcache/cache_segment.c
>> index 0b4bb08011ce..0a0016702ce7 100644
>> --- a/drivers/md/dm-pcache/cache_segment.c
>> +++ b/drivers/md/dm-pcache/cache_segment.c
>> @@ -60,7 +60,6 @@ static int cache_seg_info_load(struct 
>> pcache_cache_segment *cache_seg)
>>         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);
>>
>>
>> 在 12/2/2025 8:11 PM, Li Chen 写道:
>>> From: Li Chen <chenl311@chinatelecom.cn>
>>>
>>> dm-pcache stores metadata (cache_info and segment_info) in 4K-aligned
>>> slots on the cache device, using sequence numbers and CRC to identify
>>> the latest valid copy.
>>>
>>> However, the cache_info and segment_info paths were computing their
>>> on-media index using sizeof(struct) instead of the 4K metadata stride.
>>> As a result:
>>>
>>>    * cache_info updates (including gc_percent set via a dmsetup 
>>> message)
>>>      were written to invalid offsets between metadata slots and failed
>>>      to persist across table reloads or reboots.
>>>
>>>    * segment_info indexing became desynchronized, so rotation to the
>>>      "next" slot no longer matched the location returned by
>>>      pcache_meta_find_latest().
>>>
>>> The issue can be reproduced with:
>>>
>>>    dmsetup create pcache_vdb --table \
>>>      "0 $(blockdev --getsz /dev/vdb) pcache /dev/pmem0 /dev/vdb 4 \
>>>       cache_mode writeback data_crc false"
>>>
>>>    # Check default gc_percent (70)
>>>    dmsetup status pcache_vdb
>>>
>>>    # Change gc_percent to 10
>>>    dmsetup message pcache_vdb 0 "gc_percent 10"
>>>
>>>    # Verify change is active in memory
>>>    dmsetup status pcache_vdb
>>>
>>>    # Reboot the system...
>>>
>>>    # Without patch (gc_percent reverts to 70):
>>>    dmsetup status pcache_vdb
>>>
>>>    # With patch (gc_percent persists as 10):
>>>    dmsetup status pcache_vdb
>>>
>>> This series fixes the issue by deriving the metadata slot index from
>>> the pointer returned by pcache_meta_find_latest(), using the 4K stride
>>> (CACHE_INFO_SIZE / SEG_INFO_SIZE). This ensures that updates to
>>> cache_info and segment_info are written to valid slots and remain
>>> consistent with the on-media layout.
>>>
>>> Li Chen (2):
>>>    dm pcache: fix cache info indexing
>>>    dm pcache: fix segment info indexing
>>>
>>>   drivers/md/dm-pcache/cache.c         | 13 ++++++++++---
>>>   drivers/md/dm-pcache/cache_segment.c |  6 +++++-
>>>   2 files changed, 15 insertions(+), 4 deletions(-)
>>>
>>
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/2] dm-pcache: fix metadata indexing and persistence issues
  2025-12-04  1:38     ` Dongsheng Yang
@ 2025-12-05  9:12       ` Li Chen
  0 siblings, 0 replies; 12+ messages in thread
From: Li Chen @ 2025-12-05  9:12 UTC (permalink / raw)
  To: Dongsheng Yang; +Cc: dm-devel, linux-kernel, Zheng Gu

Hi Dongsheng,

 ---- On Thu, 04 Dec 2025 09:38:17 +0800  Dongsheng Yang <dongsheng.yang@linux.dev> wrote --- 
 > 
 > 在 12/4/2025 7:38 AM, Dongsheng Yang 写道:
 > >
 > > 在 12/3/2025 1:56 PM, Dongsheng Yang 写道:
 > >> Hi,
 > >>
 > >>     Thanx for your patches, there are some comments inline.
 > >>
 > >> According to these comments, I propose an update base on your patch 
 > >> as [1].
 > >>
 > >> BTW, I added a test case for this problem in dtg-tests:
 > >>
 > >> https://github.com/DataTravelGuide/dtg-tests/blob/main/pcache.py.data/pcache_misc_tests/case21_gc_percent_persistence_after_recreate.sh 
 > >>
 > >>
 > >> It update gc_percent online and recreate pcache device and check 
 > >> gc_percen
 > >>
 > >>
 > >> [1]:
 > >
 > >
 > > ->info_index means next slot to write, so advancing it at init stage 
 > > is correct
 > 
 > 
 > We need to think more thoroughly about the usage of the slot index. In 
 > my original design, all slot indices represent the “current slot”. So at 
 > initialization it is 0; if loading, we simply point it to the loaded 
 > slot; and on write we advance afterwards. However, the problem now is 
 > that in all write functions, they first write to the current slot, then 
 > advance.
 > 
 > Therefore I believe we should clarify that the meaning of the slot index 
 > is “the current slot,” and stick to the original design. We only need to 
 > modify the write functions: first perform the advance, and then write 
 > into the correct slot.
 
Sounds good, thanks.

Regards,

Li​


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2025-12-05  9:13 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-02 12:11 [PATCH 0/2] dm-pcache: fix metadata indexing and persistence issues Li Chen
2025-12-02 12:11 ` [PATCH 1/2] dm pcache: fix cache info indexing Li Chen
2025-12-03  5:56   ` Dongsheng Yang
2025-12-03 23:38     ` Dongsheng Yang
2025-12-02 12:11 ` [PATCH 2/2] dm pcache: fix segment " Li Chen
2025-12-03  5:57   ` Dongsheng Yang
2025-12-03 23:38     ` Dongsheng Yang
2025-12-02 19:09 ` [PATCH 0/2] dm-pcache: fix metadata indexing and persistence issues Mikulas Patocka
2025-12-03  5:56 ` Dongsheng Yang
2025-12-03 23:38   ` Dongsheng Yang
2025-12-04  1:38     ` Dongsheng Yang
2025-12-05  9:12       ` Li Chen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox