Hi Ming-Hung, Thanks for clarifying the cache decision and the rationale behind using max() in update_promote_levels(). That helped me understand the whole min() vs. max() effect on the latter read/write_promote_level I was wondering about.
Regarding promotion decisions, I read about dm-cache's slow approach to promoting candidates, as described in [1]. What are your thoughts on good dm-cache setup and configuration to achieve optimal cache performance? Thanks, Robert Pang [1] https://www.redhat.com/en/blog/improving-read-performance-dm-cache On Tue, Jul 15, 2025 at 8:59 AM Ming Hung Tsai <mt...@redhat.com> wrote: > > On Sat, Jul 12, 2025 at 5:14 AM Robert Pang <robertp...@google.com> wrote: > > > > Hi, > > > > I'm currently investigating how dm-cache makes promotion decisions and > > have a question regarding a specific line in the > > update_promote_levels() function within dm-cache-policy-smq.c. > > > > Specifically, I'm looking at line [1] in the code snippet below: > > > > static void update_promote_levels(struct smq_policy *mq) > > { > > /* > > * If there are unused cache entries then we want to be really > > * eager to promote. > > */ > > unsigned int threshold_level = allocator_empty(&mq->cache_alloc) ? > > default_promote_level(mq) : (NR_HOTSPOT_LEVELS / 2u); > > > > /* line [1] */ > > threshold_level = max(threshold_level, NR_HOTSPOT_LEVELS); > > ... > > > > mq->read_promote_level = NR_HOTSPOT_LEVELS - threshold_level; > > mq->write_promote_level = (NR_HOTSPOT_LEVELS - threshold_level); > > } > > > > My understanding is that NR_HOTSPOT_LEVELS is defined as 64, and > > default_promote_level() returns a value between 1 and 8. If this is > > correct, then threshold_level (before line [1]) would always be less > > than or equal to NR_HOTSPOT_LEVELS (e.g., 8 or 32). Consequently, > > max(threshold_level, NR_HOTSPOT_LEVELS) would always evaluate to > > NR_HOTSPOT_LEVELS, effectively setting threshold_level to 64 > > regardless of its initial calculation. > > > > I've traced line [1] back to commit b29d498 ("dm cache: significant > > rework to leverage dm-bio-prison-v2") [2]. It seems like the line's > > purpose is to cap threshold_level to prevent underflow in the > > subsequent subtractions for read_promote_level and > > write_promote_level. > > > > Given this intended purpose and the typical behavior of min() and > > max() functions, shouldn't line [1] instead be: > > > > threshold_level = min(threshold_level, NR_HOTSPOT_LEVELS); > > > > This would ensure threshold_level doesn't exceed NR_HOTSPOT_LEVELS, > > while still allowing its initial calculated value to be used if it's > > lower. > > > > Could someone please clarify if my interpretation of this logic is > > correct, or if I'm missing something? > > > > Best regards > > Robert Pang > > > > [1] > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/md/dm-cache-policy-smq.c#n1057 > > [2] > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b29d498 > > > > Hi, > > Currently, dm-cache makes promotion decisions based on short-term > access patterns. The read/write_promote_level determines how > frequently a missed block must be accessed before it is promoted. If > we modify line [1] to use min(threshold_level, NR_HOTSPOT_LEVELS), it > would increase the read/write_promote_level, making the cache less > responsive to varying IO patterns. That may be why commit b29d498 > changed the behavior. > > I’ve noticed this ambiguity before, just haven’t submitted a patch to > clean it up. The default_promote_level() might have been kept as a > reminder of the older approach. > > > Thanks, > Ming-Hung Tsai >
