All of lore.kernel.org
 help / color / mirror / Atom feed
From: SJ Park <sj@kernel.org>
To: sashiko-bot@kernel.org
Cc: SJ Park <sj@kernel.org>, damon@lists.linux.dev
Subject: Re: [RFC PATCH 8/9] mm/damon/core: handle extreme memory state in get_node_memcg_used_bp()
Date: Sat,  1 Aug 2026 13:26:56 -0700	[thread overview]
Message-ID: <20260801202657.117135-1-sj@kernel.org> (raw)
In-Reply-To: <20260801193101.E8AA11F00AC4@smtp.kernel.org>

On Sat, 01 Aug 2026 19:31:01 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Fallback values for invalid nodes and missing memcgs incorrectly encourage aggressive scheme application instead of throttling.
> - [High] Division by zero in `damos_get_in_active_mem_bp()` when total LRU memory is zero.
> - [High] Division by zero in `damos_quota_score()` when a quota goal has a `target_value` of 0.
> --
> 
> --- Patch [8]: [RFC PATCH 8/9] mm/damon/core: handle extreme memory state in get_node_memcg_used_bp() ---
> commit 7261d4dd3fd172b597cdefa3462cc680deabe142
> Author: SJ Park <sj@kernel.org>
> 
> mm/damon/core: handle extreme memory state in get_node_memcg_used_bp()
> 
> This commit adds fallback logic to handle extreme situations where total 
> memory is zero, or when lruvec_page_state() returns used memory larger 
> than total memory. It returns 100% and 0% respectively to avoid division 
> by zero or underflow in damos_get_node_memcg_used_bp().
> 
> Link: https://lore.kernel.org/20260329154813.47382-1-sj@kernel.org
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 269865eb8fbfa..76764a2056f7c 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> > @@ -2858,10 +2858,16 @@ static unsigned long damos_get_node_memcg_used_bp(
> >  	mem_cgroup_put(memcg);
> >  
> >  	si_meminfo_node(&i, goal->nid);
> > -	if (goal->metric == DAMOS_QUOTA_NODE_MEMCG_USED_BP)
> > +	if (!i.totalram)
> > +		return 10000;
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but returning 0 in other fallback 
> paths for missing memcgs or invalid nodes incorrectly encourages aggressive 
> scheme application. 
> 
> In the DAMOS quota auto-tuner (damon_feed_loop_next_input()), a metric score
> of 0 evaluates as severe under-achievement, causing the tuner to aggressively
> maximize the quota. To safely halt a scheme for invalid targets, the metric
> getter should probably return a value that yields a high score (e.g., 10000).
> 
> However, multiple pre-existing fallback paths incorrectly return 0:
> 
> mm/damon/core.c:damos_get_node_memcg_used_bp()
> 	if (goal->metric == DAMOS_QUOTA_NODE_MEMCG_USED_BP)
> 		return 0;
> 
> mm/damon/core.c:damos_get_node_mem_bp()
> 	else	/* DAMOS_QUOTA_NODE_MEM_FREE_BP */
> 		numerator = i.freeram;
> 	return mult_frac(numerator, 10000, i.totalram);
> (when !totalram, this evaluates to 0)
> 
> mm/damon/core.c:damos_get_node_mem_bp()
> 	static __kernel_ulong_t damos_get_node_mem_bp(
> 			struct damos_quota_goal *goal)
> 	{
> 		return 0;
> 	}
> 
> Could these fallbacks cause the auto-tuner to unleash maximum operations and
> lead to severe over-reclaim or wasted CPU when a node becomes invalid or a
> memcg is destroyed?

This is an intended behavior.

> 
> > +	if (goal->metric == DAMOS_QUOTA_NODE_MEMCG_USED_BP) {
> >  		numerator = used_pages;
> > -	else	/* DAMOS_QUOTA_NODE_MEMCG_FREE_BP */
> > +	} else {
> > +		/* DAMOS_QUOTA_NODE_MEMCG_FREE_BP */
> > +		if (i.totalram < used_pages)
> > +			return 0;
> >  		numerator = i.totalram - used_pages;
> > +	}
> >  	return mult_frac(numerator, 10000, i.totalram);
> >  }
> 
> [Severity: High]
> This is a pre-existing issue with a similar root cause, but does this same
> division by zero problem exist in damos_get_in_active_mem_bp()?
> 
> mm/damon/core.c:damos_get_in_active_mem_bp()
> 	if (active_ratio)
> 		return mult_frac(active, 10000, total);
> 	return mult_frac(inactive, 10000, total);
> 
> If the system has 0 pages on the LRU (e.g., during early boot, in extremely
> constrained environments, or if all memory is unevictable), total evaluates
> to 0. Would this result in a kernel panic when passed to mult_frac()?

A later patch in this series will fix it.

> 
> 
> [Severity: High]
> This is another pre-existing issue, but does damos_quota_score() also suffer
> from a division by zero when a quota goal has a target_value of 0?
> 
> mm/damon/core.c:damos_quota_score()
> 	highest_score = max(highest_score,
> 		mult_frac(goal->current_value, 10000,
> 			goal->target_value));
> 
> While the sysfs interface drops goals with a 0 target value, the core
> allocation function damos_new_quota_goal() does not validate it. The mtier
> sample module exposes these as module parameters:
> 
> samples/damon/mtier.c:damon_sample_mtier_build_ctx()
> 	quota_goal = damos_new_quota_goal(
> 		promote ? DAMOS_QUOTA_NODE_MEM_USED_BP :
> 		DAMOS_QUOTA_NODE_MEM_FREE_BP,
> 		promote ? node0_mem_used_bp : node0_mem_free_bp);
> 
> If an administrator or privileged user writes 0 to the parameter and enables
> the module, would this immediately crash the kernel when the DAMOS worker
> thread evaluates the quota?

Good catch, I will work on this separately or as a part of this series.

> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260801173554.94710-1-sj@kernel.org?part=8


Thanks,
SJ

  reply	other threads:[~2026-08-01 20:27 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-01 17:35 [RFC PATCH 0/9] mm/damon: fix DAMOS bugs in core, paddr and vaddr SJ Park
2026-08-01 17:35 ` [RFC PATCH 1/9] mm/damon/core: skip applying scheme if region split for quota fails SJ Park
2026-08-01 17:53   ` sashiko-bot
2026-08-01 20:01     ` SJ Park
2026-08-01 20:31       ` SJ Park
2026-08-01 17:35 ` [RFC PATCH 2/9] mm/damon/core: initialize damos_quota_goal->last_psi_total SJ Park
2026-08-01 17:35 ` [RFC PATCH 3/9] mm/damon/paddr: respect folio end for DAMOS_STAT SJ Park
2026-08-01 18:21   ` sashiko-bot
2026-08-01 20:07     ` SJ Park
2026-08-01 17:35 ` [RFC PATCH 4/9] mm/damon/paddr: respect folio end for DAMOS actions except STAT SJ Park
2026-08-01 17:35 ` [RFC PATCH 5/9] mm/damon/vaddr: respect folio end for DAMOS_STAT SJ Park
2026-08-01 18:50   ` sashiko-bot
2026-08-01 20:12     ` SJ Park
2026-08-01 17:35 ` [RFC PATCH 6/9] mm/damon/vaddr: respect folio end for DAMOS_MIGRATE_{HOT,COLD} SJ Park
2026-08-01 17:35 ` [RFC PATCH 7/9] mm/damon/core: handle extreme memory state in damon_get_node_mem_bp() SJ Park
2026-08-01 19:11   ` sashiko-bot
2026-08-01 20:18     ` SJ Park
2026-08-01 17:35 ` [RFC PATCH 8/9] mm/damon/core: handle extreme memory state in get_node_memcg_used_bp() SJ Park
2026-08-01 19:31   ` sashiko-bot
2026-08-01 20:26     ` SJ Park [this message]
2026-08-01 17:35 ` [RFC PATCH 9/9] mm/damon/core: handle extreme memory state in get_in_active_mem_bp() SJ Park
2026-08-01 19:39   ` sashiko-bot
2026-08-01 20:29     ` SJ Park

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260801202657.117135-1-sj@kernel.org \
    --to=sj@kernel.org \
    --cc=damon@lists.linux.dev \
    --cc=sashiko-bot@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.