Re: [PATCH v3 1/2] mm/damon/core: split a fraction of regions when nr_regions exceeds max/2

[SJ Park <sj@kernel.org>]

On Mon, 29 Jun 2026 15:08:10 +0000 sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Cross-context state corruption and data races due to the use of a static local variable `rotation`.
> --
> 
> commit 696204689b6ff0a9b6ca3153698c49bfe10cff24
> Author: Jiayuan Chen <jiayuan.chen@shopee.com>
> 
> mm/damon/core: split a fraction of regions when nr_regions exceeds max/2
> 
> This commit modifies the kdamond_split_regions logic to split only a fraction
> of regions when the region count exceeds half the maximum, rather than
> returning early. It uses a rotating offset to select which regions to split,
> refining the count smoothly as it approaches the maximum.
> 
> Link: https://lore.kernel.org/20260626085851.70754-2-jiayuan.chen@linux.dev
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 027250e43c66f..9bbc60a2a8c05 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> [ ... ]
> > @@ -3234,6 +3234,37 @@ static void damon_split_regions_of(struct damon_ctx *ctx,
> >  	}
> >  }
> >  
> > +/* Split one in every @split_step regions into two, from a rotating offset */
> > +static void damon_split_some_regions(struct damon_ctx *ctx,
> > +				     unsigned long split_step)
> > +{
> > +	static unsigned long rotation;
> > +	struct damon_target *t;
> > +	struct damon_region *r, *next;
> > +	unsigned long offset = rotation++ % split_step;
> 
> [Severity: Medium]
> Will this shared static variable cause data races and logic flaws if
> multiple DAMON contexts are active?
> 
> Since rotation is declared statically, multiple kdamond threads could
> concurrently increment it without synchronization. This might lead to
> data races and cause different contexts to overwrite each other's
> rotation offsets, breaking the isolation of the splitting algorithm and
> resulting in uneven or skipped splits in some contexts.
> 
> Should this state be kept per-context instead of using a static local
> variable?

As I replied [1] to same question on v2, We don't need a strict round robin
here.  We need just a level of fairness.  I think this is no problem.  Of
course we could revisit this if it turns out to a real problem in the real
world.

[1] https://lore.kernel.org/20260626144906.86649-1-sj@kernel.org


Thanks,
SJ

[...]