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