All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "SJ Park" <sj@kernel.org>
Cc: damon@lists.linux.dev
Subject: Re: [RFC PATCH v1.1 1/7] mm/damon/core: remove obsolete comment for nr_to_bp() divide-by-zero
Date: Tue, 07 Jul 2026 13:57:50 +0000	[thread overview]
Message-ID: <20260707135750.DD21B1F00A3A@smtp.kernel.org> (raw)
In-Reply-To: <20260707135038.90068-2-sj@kernel.org>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Removing the `!attrs.aggr_interval` skip guard causes `damon_test_nr_accesses_to_accesses_bp` to fail deterministically on 32-bit architectures.
--

--- Patch [1]: [RFC PATCH v1.1 1/7] mm/damon/core: remove obsolete comment for nr_to_bp() divide-by-zero ---
commit a31fcddef9e05dc323b42f64143ffe58dc702dee
Author: SJ Park <sj@kernel.org>

mm/damon/core: remove obsolete comment for nr_to_bp() divide-by-zero

This commit removes obsolete comments about division-by-zero in
damon_nr_accesses_to_accesses_bp() and its related test code. A previous
commit modified the underlying calculation to always return a non-zero value,
preventing the division-by-zero issue.

> diff --git a/mm/damon/tests/core-kunit.h b/mm/damon/tests/core-kunit.h
> index 0124f83b39b83..224a3ecaa0701 100644
> --- a/mm/damon/tests/core-kunit.h
> +++ b/mm/damon/tests/core-kunit.h
[ ... ]
> @@ -582,18 +582,6 @@ static void damon_test_nr_accesses_to_accesses_bp(struct kunit *test)
>  		.aggr_interval = ((unsigned long)UINT_MAX + 1) * 10
>  	};
>  
> -	/*
> -	 * In some cases such as 32bit architectures where UINT_MAX is
> -	 * ULONG_MAX, attrs.aggr_interval becomes zero.  Calling
> -	 * damon_nr_accesses_to_accesses_bp() in the case will cause
> -	 * divide-by-zero.  Such case is prohibited in normal execution since
> -	 * the caution is documented on the comment for the function, and
> -	 * damon_update_monitoring_results() does the check.  Skip the test in
> -	 * the case.
> -	 */
> -	if (!attrs.aggr_interval)
> -		kunit_skip(test, "aggr_interval is zero.");
> -
>  	KUNIT_EXPECT_EQ(test, damon_nr_accesses_to_accesses_bp(123, &attrs), 0);

[Severity: Medium]
Will removing this skip guard cause a deterministic test failure on 32-bit
architectures?

When the test runs on a 32-bit architecture, attrs.aggr_interval is initialized
to ((unsigned long)UINT_MAX + 1) * 10, which overflows to 0. With the division
by zero fix in place, damon_max_nr_accesses(&attrs) will return 1 instead of 0.

This causes damon_nr_accesses_to_accesses_bp(123, &attrs) to evaluate to
(123 * 10000) / 1 = 1230000. Since the test unconditionally expects the result
to be 0, the KUNIT_EXPECT_EQ assertion will fail.

>  }

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

  reply	other threads:[~2026-07-07 13:57 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-07 13:50 [RFC PATCH v1.1 0/7] mm/damon: update probe hits for runtime parameter commits SJ Park
2026-07-07 13:50 ` [RFC PATCH v1.1 1/7] mm/damon/core: remove obsolete comment for nr_to_bp() divide-by-zero SJ Park
2026-07-07 13:57   ` sashiko-bot [this message]
2026-07-07 14:19     ` SJ Park
2026-07-07 13:50 ` [RFC PATCH v1.1 2/7] mm/damon/core: s/damon_max_nr_accesses()/damon_nr_samples_per_aggr()/ SJ Park
2026-07-07 13:58   ` sashiko-bot
2026-07-07 14:21     ` SJ Park
2026-07-07 13:50 ` [RFC PATCH v1.1 3/7] mm/damon/core: s/accesses_bp_to_nr_accesses/sample_bp_to_count/ SJ Park
2026-07-07 13:50 ` [RFC PATCH v1.1 4/7] mm/damon/core: s/nr_accesses_to_accesses_bp/sample_count_to_bp/ SJ Park
2026-07-07 14:02   ` sashiko-bot
2026-07-07 14:26     ` SJ Park
2026-07-07 13:50 ` [RFC PATCH v1.1 5/7] mm/damon/core: s/nr_accesses_for_new_attrs/nr_samples_for_new_attrs/ SJ Park
2026-07-07 14:05   ` sashiko-bot
2026-07-07 14:27     ` SJ Park
2026-07-07 13:50 ` [RFC PATCH v1.1 6/7] mm/damon/core: update probe hits for new parameter commit SJ Park
2026-07-07 14:06   ` sashiko-bot
2026-07-07 14:30     ` SJ Park
2026-07-07 13:50 ` [RFC PATCH v1.1 7/7] mm/damon/core: handle unreset probe_hits in probe_hits_mvsum() 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=20260707135750.DD21B1F00A3A@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=damon@lists.linux.dev \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=sj@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.