Here's a rewritten version of your email, aiming for clarity,
conciseness, and a professional tone:

Subject: Question about dm-cache promotion decision in dm-cache-policy-smq.c
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

Reply via email to