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