在 7/29/2025 5:23 AM, Mikulas Patocka 写道:
Hi
On Fri, 25 Jul 2025, Dongsheng Yang wrote:
* get_n_vecs: what if "data" is not page-aligned? is that possible?
Yes, it's possible, so I am using DIV_ROUND_UP() to calculate the n_vecs.
DIV_ROUND_UP is not sufficient - suppose that "data" is 0xf00 and "len" is
0x200. DIV_ROUND_UP(len, PAGE_SIZE) returns "1", while you need two bio
vector entries in fact.
I'd change DIV_ROUND_UP(len, PAGE_SIZE) to
DIV_ROUND_UP(offset_in_page(data) + len, PAGE_SIZE)
Yes, good point, you are right. I will call bio_add_max_vecs() from
bio.h in next version, it works same as you mentioned above.
In addition, to expose this kind of bug in my testing environment, I
will setup one of my testing machine
to use striped pmem as cache dev.
* cache_dev_get_empty_segment_id: change set_bit to __set_bit because it
doesn't have to be atomic - you already hold the lock
Okey
* you usually hold seg_map_lock around modifications of seg_map bitmap,
but not in kset_replay and cache_replay - that seems like a bug. When
you hold the spinlock always, you can change set_bit/clear_bit to
__set_bit and __clear_bit
actually, that's intented, because there is no other accessor in replaying
stage. So I dont hold the lock here, that would be faster.
OK, so you don't have to hold the lock, you can just add a comment that
the lock it not needed because the code is single-threaded.
okey , I will add comment here.
Just a suggestion for performance improvement: when you hold a spinlock a
you need to allocate some memory, you must drop the spinlock, allocate it
with GFP_NOIO, reacquire the spinlock and recheck the state. You can
improve this logic by allocating with mempool_alloc(GFP_NOWAIT) while you
hold the spinlock (GFP_NOWAIT will not sleep, so it's allowed), and
fallback to dropping the spinlock only if the GFP_NOWAIT allocation fails.
Sounds a good suggestion, I will try it in V5.
Yes. When you implement it, don't forget to test the code path that drops
the spinlock and uses mempool_alloc(GFP_NOIO), so that it doesn't bitrot
over time.
Sure, the failslab case can cover this path:
https://github.com/DataTravelGuide/dtg-tests/blob/main/pcache.py.data/pcache_failslab.sh
Dongsheng
Mikulas
