[PATCH] mm/damon: validate addr_unit to be power of 2

[PATCH] mm/damon: validate addr_unit to be power of 2

[Liew Rui Yan]

Problem
=======
The 'addr_unit' must be a power of 2 for correct address alignment
calculations. Previously, writing a non-power-of-2 value
(e.g., addr_unit=3) would be accepted by the sysfs store callback, but
cause kdamond to exit unexpectedly during parameter application, failing
silently without returning an error to userspace.

Solution
========
Add an is_power_of_2() check in damon_commit_ctx() to reject invalid
inputs immediately with -EINVAL. 

When damon_commit_ctx() fails, the kdamond thread terminates as
designed.

The issue is found by sashiko [1].

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

Signed-off-by: Liew Rui Yan <aethernet65535@gmail.com>
---
 mm/damon/core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/mm/damon/core.c b/mm/damon/core.c
index db6c67e52d2b..6bad85a47a79 100644
--- a/mm/damon/core.c
+++ b/mm/damon/core.c
@@ -1330,6 +1330,8 @@ int damon_commit_ctx(struct damon_ctx *dst, struct damon_ctx *src)
 	dst->maybe_corrupted = true;
 	if (!is_power_of_2(src->min_region_sz))
 		return -EINVAL;
+	if (!src->addr_unit || !is_power_of_2(src->addr_unit))
+		return -EINVAL;
 
 	err = damon_commit_schemes(dst, src);
 	if (err)
-- 
2.53.0

Re: [PATCH] mm/damon: validate addr_unit to be power of 2

[Liew Rui Yan]

Hi SeongJae,

While testing, I noticed that when damon_commit_ctx() returns -EINVAL
due to an invalid 'addr_unit', the kdamond stops running. I recall you
mentioned [1] that the kdamond should only stop for "internal error":

"
I found an issue in the current behavior, and just posted a fix.  Assuming the
fix is merged, the behavior will again be changed.  So, please do this only
after the fix is merged or completely abandoned.

If the fix is merged, the changed behavior will be something like,

"It will continue running with old valid parameters if the parameters are
wrong.  It will stop running if the parameters update was unable to be
completed due to internal error."
"

Is this termination behavior expected for this case?

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

Best regards,
Rui Yan

Re: [PATCH] mm/damon: validate addr_unit to be power of 2

[Liew Rui Yan]

Hi SeongJae,

Regarding the kdamond termination, I have an idea:
Should we avoid terminating kdamond just because of invalid parameters
from user input (e.g., non-power-of-2 'min_region_sz' or 'addr_unit')?

Since a terminated kdamond cannot be easily restarted currently (if it
is not terminated by 'echo N > enabled'), we should probably try to
reduce the probability of such unexpected exits caused by user input.

I'm thinking of validating these parameters early in damon_commit_ctx().
If they are invalid, we could reset 'dst->maybe_corrupted' to 'false'
and return -EINVAL.

This way, since no actual modifications to 'dst' were made before the
check, we can safely clear the 'maybe_corrupted' flag. User will then
receive the error but kdamond should be able to keep running with its
existing configuration instead of exiting.

Does this approach make sense to you?

Best regards,
Rui Yan

Re: (sashiko review) [PATCH] mm/damon: validate addr_unit to be power of 2

[Liew Rui Yan]

Forwarding Sashiko review for doing discussions via mails.

# review url: https://sashiko.dev/#/patchset/20260327062627.66426-1-aethernet65535@gmail.com
# start of sashiko.dev inline review
commit ceb9522e072d53117ff67157d6f0be7d35919759
Author: Liew Rui Yan <aethernet65535@gmail.com>

mm/damon: validate addr_unit to be power of 2

This commit adds a validation check in damon_commit_ctx() to ensure
addr_unit is a power of 2. This prevents kdamond from exiting
unexpectedly during parameter application when given invalid inputs.

Link: https://lore.kernel.org/20260325025317.86571-1-sj@kernel.org

> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index db6c67e52d2b..6bad85a47a79 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c

[ ... ]

> @@ -1330,6 +1330,8 @@ int damon_commit_ctx(struct damon_ctx *dst, struct damon_ctx *src)
>  	dst->maybe_corrupted = true;
>  	if (!is_power_of_2(src->min_region_sz))
>  		return -EINVAL;
> +	if (!src->addr_unit || !is_power_of_2(src->addr_unit))

This isn't a bug, but is the !src->addr_unit check needed here?

Looking at is_power_of_2(), it already evaluates to false for an input
of zero, so the compound condition could perhaps be simplified to just
!is_power_of_2(src->addr_unit).

> +		return -EINVAL;
>  
>  	err = damon_commit_schemes(dst, src);
>  	if (err)


# end of sashiko.dev inline review
# review url: https://sashiko.dev/#/patchset/20260327062627.66426-1-aethernet65535@gmail.com
#
# hkml [1] generated a draft of this mail.  It can be regenerated
# using below command:
#
#     hkml patch sashiko_dev --for_forwarding \
#             20260327062627.66426-1-aethernet65535@gmail.com
#
# [1] https://github.com/sjp38/hackermail

Re: (sashiko review) [PATCH] mm/damon: validate addr_unit to be power of 2

[Liew Rui Yan]

> [...]
> > @@ -1330,6 +1330,8 @@ int damon_commit_ctx(struct damon_ctx *dst, struct damon_ctx *src)
> >  	dst->maybe_corrupted = true;
> >  	if (!is_power_of_2(src->min_region_sz))
> >  		return -EINVAL;
> > +	if (!src->addr_unit || !is_power_of_2(src->addr_unit))
> 
> This isn't a bug, but is the !src->addr_unit check needed here?
> 
> Looking at is_power_of_2(), it already evaluates to false for an input
> of zero, so the compound condition could perhaps be simplified to just
> !is_power_of_2(src->addr_unit).

You are right. Futhermore, addr_unit_store() also prevents 'addr_unit'
from being 0.

Best regards,
Rui Yan

Re: [PATCH] mm/damon: validate addr_unit to be power of 2

[SeongJae Park]

Hi Liew,


Unfortunately I'm bit confused about this patch.  Please answer below
questions.

On Fri, 27 Mar 2026 14:26:27 +0800 Liew Rui Yan <aethernet65535@gmail.com> wrote:

> Problem
> =======
> The 'addr_unit' must be a power of 2 for correct address alignment
> calculations.

It is required to be a power of 2 only if it is smaller than PAGE_SIZE.  Refer
to commit d8fe5d2a3379 ("Docs/mm/damon/design: document the power-of-two
limitation for addr_unit") for more details.

> Previously,

Can we make the timing more explicitly?  I'm confusing when is the time you are
mentioning, since a behavioral change was made recently, by commit c80f46ac228b
("mm/damon/core: disallow non-power of two min_region_sz").

> writing a non-power-of-2 value
> (e.g., addr_unit=3) would be accepted by the sysfs store callback, but
> cause kdamond to exit unexpectedly during parameter application, failing
> silently without returning an error to userspace.

I'm being more confused.  DAMON_SYSFS handles commits synchronously, and return
error to user space when it failed, iirc.  Am I missing something?

> 
> Solution
> ========
> Add an is_power_of_2() check in damon_commit_ctx() to reject invalid
> inputs immediately with -EINVAL. 

If invalid addr_unit already makes damon_commit_ctx() fails and therefore
kdamond exit, why we need more check?

> 
> When damon_commit_ctx() fails, the kdamond thread terminates as
> designed.

Again, I don't get it...


Thanks,
SJ

[...]

Re: [PATCH] mm/damon: validate addr_unit to be power of 2

[Liew Rui Yan]

Hi SeongJae,

Thank you for the detailed explaination and for pointing out the
'PAGE_SIZE' limitation. You are right; and I should have accounted for
that.

However, I performend further testing to address your confusion
regarding kdamond's termination. My patch didn't fix anything.

Here is my reproduction:

# Log (Both with and without this patch):

	# cd /sys/module/damon_lru_sort/parameters/
	# echo Y > enabled 
	# echo 3 > addr_unit 
	# ps aux | rg "[k]damond"
	root    71  0.0 0.0 0   0   ?   I   22:26   0:00 [kdamond.0]
	# echo Y > commit_inputs 
	bash: echo: write error: Invalid argument
	# cat kdamond_pid 
	71
	# ps aux | rg "[k]damond"
	# ... kdamond terminated

I am very sorry for the noise caused by my misunderstanding of the
sashiko review.

The real question I am facing now is:
Should kdamond terminate itself when damon_commit_ctx() fails due to
invalid user inputs?

Best regards,
Rui Yan

Re: [PATCH] mm/damon: validate addr_unit to be power of 2

[SeongJae Park]

On Fri, 27 Mar 2026 22:56:27 +0800 Liew Rui Yan <aethernet65535@gmail.com> wrote:

> Hi SeongJae,
> 
> Thank you for the detailed explaination and for pointing out the
> 'PAGE_SIZE' limitation. You are right; and I should have accounted for
> that.
> 
> However, I performend further testing to address your confusion
> regarding kdamond's termination. My patch didn't fix anything.
> 
> Here is my reproduction:
> 
> # Log (Both with and without this patch):
> 
> 	# cd /sys/module/damon_lru_sort/parameters/
> 	# echo Y > enabled 
> 	# echo 3 > addr_unit 
> 	# ps aux | rg "[k]damond"
> 	root    71  0.0 0.0 0   0   ?   I   22:26   0:00 [kdamond.0]
> 	# echo Y > commit_inputs 
> 	bash: echo: write error: Invalid argument
> 	# cat kdamond_pid 
> 	71
> 	# ps aux | rg "[k]damond"
> 	# ... kdamond terminated
> 
> I am very sorry for the noise caused by my misunderstanding of the
> sashiko review.
> 
> The real question I am facing now is:
> Should kdamond terminate itself when damon_commit_ctx() fails due to
> invalid user inputs?

It would be better to not terminate.  But not a big deal.  Users could be wary
of the consequence, as long as it is well documented.

From my perspective, therefore, code maintenance is more important.  If we can
make DAMON not terminated by invalid user inputs caused damon_commit_ctx() but
it introduces a significant amount of code complexity, I don't think that's a
good deal.


Thanks,
SJ

[...]

Re: [PATCH] mm/damon: validate addr_unit to be power of 2

[Liew Rui Yan]

Hi SeongJae,

> It would be better to not terminate.  But not a big deal.  Users could be wary
> of the consequence, as long as it is well documented.
> 
> From my perspective, therefore, code maintenance is more important.  If we can
> make DAMON not terminated by invalid user inputs caused damon_commit_ctx() but
> it introduces a significant amount of code complexity, I don't think that's a
> good deal.

Thank you so much for the thoughtful response! I completely agree that
code maintainability should be the priority and we should avoid
unnecessary complexity.

Just to make sure I captured the full context, in my earlier follow-ups
[1][2] (in case they got folded in your email client):

- Does the current kdamond termination on invalid user input align with
  your design goal mentioned in [3] (stopping only for "internal
  errors")?

- Would a lightweight fix be acceptable? For example, performing
  validation at the very beginning of damon_commit_ctx(), and returning
  -EINVAL before setting 'maybe_corrupted' to true. Since no
  modifications to 'dst' would have occurred, kdamond could safely
  continue with its old configuration.

Thank you again for your time, and guidance. :>

[1] https://lore.kernel.org/20260327064517.68131-1-aethernet65535@gmail.com
[2] https://lore.kernel.org/20260327121009.38374-1-aethernet65535@gmail.com
[3] https://lore.kernel.org/20260319151528.86490-1-sj@kernel.org

Best regards,
Rui Yan

Re: [PATCH] mm/damon: validate addr_unit to be power of 2

[SeongJae Park]

On Sat, 28 Mar 2026 10:26:51 +0800 Liew Rui Yan <aethernet65535@gmail.com> wrote:

[...]
> Just to make sure I captured the full context, in my earlier follow-ups
> [1][2] (in case they got folded in your email client):
> 
> - Does the current kdamond termination on invalid user input align with
>   your design goal mentioned in [3] (stopping only for "internal
>   errors")?

No.  I still think it would be better to avoid wrong input-caused termination.
But in a simple way.  Also, it would be better to be consistent.

> 
> - Would a lightweight fix be acceptable? For example, performing
>   validation at the very beginning of damon_commit_ctx(), and returning
>   -EINVAL before setting 'maybe_corrupted' to true. Since no
>   modifications to 'dst' would have occurred, kdamond could safely
>   continue with its old configuration.

I suggested you to try something similar to what DAMON_SYSFS is doing.  But I
didn't get your response to the idea yet.  Could you please let me know what do
you think about the approach?

> 
> Thank you again for your time, and guidance. :>

You're welcome.

> 
> [1] https://lore.kernel.org/20260327064517.68131-1-aethernet65535@gmail.com
> [2] https://lore.kernel.org/20260327121009.38374-1-aethernet65535@gmail.com
> [3] https://lore.kernel.org/20260319151528.86490-1-sj@kernel.org


Thanks,
SJ

[...]

Re: [PATCH] mm/damon: validate addr_unit to be power of 2

[SeongJae Park]

On Sat, 28 Mar 2026 06:29:37 -0700 SeongJae Park <sj@kernel.org> wrote:

> On Sat, 28 Mar 2026 10:26:51 +0800 Liew Rui Yan <aethernet65535@gmail.com> wrote:
> 
> [...]
> > Just to make sure I captured the full context, in my earlier follow-ups
> > [1][2] (in case they got folded in your email client):
> > 
> > - Does the current kdamond termination on invalid user input align with
> >   your design goal mentioned in [3] (stopping only for "internal
> >   errors")?
> 
> No.  I still think it would be better to avoid wrong input-caused termination.
> But in a simple way.  Also, it would be better to be consistent.
> 
> > 
> > - Would a lightweight fix be acceptable? For example, performing
> >   validation at the very beginning of damon_commit_ctx(), and returning
> >   -EINVAL before setting 'maybe_corrupted' to true. Since no
> >   modifications to 'dst' would have occurred, kdamond could safely
> >   continue with its old configuration.
> 
> I suggested you to try something similar to what DAMON_SYSFS is doing.  But I
> didn't get your response to the idea yet.  Could you please let me know what do
> you think about the approach?

I was thinking this one more time.  So you want to ensure DAMON_RECLAIM and
DAMON_LRU_SORT not passing wrong addr_unit to damon_commit_ctx(), right?  And
that's required because exisitng inputs validations of DAMON_RECLAIM and
DAMON_LRU_SORT are not checking that.  Why don't you add the just one more
check there?

I remember I was saying this kind of change would be a kind of whack-a-mole and
therefore prefer DAMON_SYSFS type damon_commit_ctx() based checking.  But
that's not a small change.  Just adding yet another simple check on existing
validation logic should be fine and small.


Thanks,
SJ

[...]

Re: [PATCH] mm/damon: validate addr_unit to be power of 2

[Liew Rui Yan]

On Sat, 28 Mar 2026 07:13:23 -0700 SeongJae Park <sj@kernel.org> wrote:

> > On Sat, 28 Mar 2026 10:26:51 +0800 Liew Rui Yan <aethernet65535@gmail.com> wrote:
> >
> > > [...]
> > > - Would a lightweight fix be acceptable? For example, performing
> > >   validation at the very beginning of damon_commit_ctx(), and returning
> > >   -EINVAL before setting 'maybe_corrupted' to true. Since no
> > >   modifications to 'dst' would have occurred, kdamond could safely
> > >   continue with its old configuration.
> > 
> > I suggested you to try something similar to what DAMON_SYSFS is doing.  But I
> > didn't get your response to the idea yet.  Could you please let me know what do
> > you think about the approach?
> 
> I was thinking this one more time.  So you want to ensure DAMON_RECLAIM and
> DAMON_LRU_SORT not passing wrong addr_unit to damon_commit_ctx(), right?  And
> that's required because exisitng inputs validations of DAMON_RECLAIM and
> DAMON_LRU_SORT are not checking that.  Why don't you add the just one more
> check there?

Apologize for the confusion in my previous explanation. To clarify, my
proposal was indeed focused on implementing a lightweight fix directly
within damon_commit_ctx(), rather than adding separate checks in
DAMON_RECLAIM and DAMON_LRU_SORT.

The goal is to safely handle the context state transition. Specifically,
I want to validate inputs before marking the context as potentially
corrupted. Here are two approaches to achieve this:

Option 1 - Using a label for cleanup:

    damon_commit_ctx(...)
    {
        err = 0;

        dst->maybe_corrupted = true;
        if (...) {
            err = -EINVAL;
            goto out_reset;
        }

        ... other code ...

    out_reset:
        dst->maybe_corrupted = false;
        return err;
    }

Option 2 - Deferring the 'maybe_corrupted' assignment:

    damon_commit_ctx(...)
    {
        err = 0;

        if (...)
            return -EINVAL;

        /*
         * Only mark as pontentially corrupted once
         * we start modifying dst.
         */
        dst->maybe_corrupted = true;

        ... other code ...
    }

I think Option 2 is cleaner as it avoids unnecessary state flipping.

> 
> I remember I was saying this kind of change would be a kind of whack-a-mole and
> therefore prefer DAMON_SYSFS type damon_commit_ctx() based checking.  But
> that's not a small change.  Just adding yet another simple check on existing
> validation logic should be fine and small.
>
> [...]

Best regards,
Rui Yan

Re: [PATCH] mm/damon: validate addr_unit to be power of 2

[SeongJae Park]

On Sun, 29 Mar 2026 01:44:09 +0800 Liew Rui Yan <aethernet65535@gmail.com> wrote:

> On Sat, 28 Mar 2026 07:13:23 -0700 SeongJae Park <sj@kernel.org> wrote:
> 
> > > On Sat, 28 Mar 2026 10:26:51 +0800 Liew Rui Yan <aethernet65535@gmail.com> wrote:
> > >
> > > > [...]
> > > > - Would a lightweight fix be acceptable? For example, performing
> > > >   validation at the very beginning of damon_commit_ctx(), and returning
> > > >   -EINVAL before setting 'maybe_corrupted' to true. Since no
> > > >   modifications to 'dst' would have occurred, kdamond could safely
> > > >   continue with its old configuration.
> > > 
> > > I suggested you to try something similar to what DAMON_SYSFS is doing.  But I
> > > didn't get your response to the idea yet.  Could you please let me know what do
> > > you think about the approach?
> > 
> > I was thinking this one more time.  So you want to ensure DAMON_RECLAIM and
> > DAMON_LRU_SORT not passing wrong addr_unit to damon_commit_ctx(), right?  And
> > that's required because exisitng inputs validations of DAMON_RECLAIM and
> > DAMON_LRU_SORT are not checking that.  Why don't you add the just one more
> > check there?
> 
> Apologize for the confusion in my previous explanation. To clarify, my
> proposal was indeed focused on implementing a lightweight fix directly
> within damon_commit_ctx(), rather than adding separate checks in
> DAMON_RECLAIM and DAMON_LRU_SORT.
> 
> The goal is to safely handle the context state transition. Specifically,
> I want to validate inputs before marking the context as potentially
> corrupted. Here are two approaches to achieve this:
> 
> Option 1 - Using a label for cleanup:
> 
>     damon_commit_ctx(...)
>     {
>         err = 0;
> 
>         dst->maybe_corrupted = true;
>         if (...) {
>             err = -EINVAL;
>             goto out_reset;
>         }
> 
>         ... other code ...
> 
>     out_reset:
>         dst->maybe_corrupted = false;
>         return err;
>     }
> 
> Option 2 - Deferring the 'maybe_corrupted' assignment:
> 
>     damon_commit_ctx(...)
>     {
>         err = 0;
> 
>         if (...)
>             return -EINVAL;
> 
>         /*
>          * Only mark as pontentially corrupted once
>          * we start modifying dst.
>          */
>         dst->maybe_corrupted = true;
> 
>         ... other code ...
>     }
> 
> I think Option 2 is cleaner as it avoids unnecessary state flipping.

Liew, I suggeted two options before.  But you are skipping providing your
opinion to those, and adding yet more options.  That makes me difficult to
follow the conversation.  Could you please answer to my suggestions and make a
consensus about those, first?


Thanks,
SJ

[...]

Re: [PATCH] mm/damon: validate addr_unit to be power of 2

[Liew Rui Yan]

> Liew, I suggeted two options before.  But you are skipping providing your
> opinion to those, and adding yet more options.  That makes me difficult to
> follow the conversation.  Could you please answer to my suggestions and make a
> consensus about those, first?

Apologize if my previous email was unclear. Let me directly address your
two suggestion.

1. DAMON_SYSFS Type [1]:
   I fully agree with this. Centralizing the validation in
   damon_commit_ctx() is the right approach to avoid "whack-a-mole"
   problem. This is exactly what I am proposing.

2. Adding a simple check on existing validation logic (in callers?) [2]:
   While this is simpler to implement, I prefer avoiding it for the
   "whack-a-mole".

So, to clarify, I choose your first option (centralized check), and I
believe my "Option 2" is the simple way to implement it.

Does ths align with your expectation? If so, I will proceed with this
approach.

[1] https://lore.kernel.org/20260328132937.9580-1-sj@kernel.org
[2] https://lore.kernel.org/20260328141323.10540-1-sj@kernel.org
[3] https://lore.kernel.org/20260328174409.6786-1-aethernet65535@gmail.com

Best regards,
Rui Yan

Re: [PATCH] mm/damon: validate addr_unit to be power of 2

[SeongJae Park]

On Sun, 29 Mar 2026 02:43:19 +0800 Liew Rui Yan <aethernet65535@gmail.com> wrote:

> > Liew, I suggeted two options before.  But you are skipping providing your
> > opinion to those, and adding yet more options.  That makes me difficult to
> > follow the conversation.  Could you please answer to my suggestions and make a
> > consensus about those, first?
> 
> Apologize if my previous email was unclear. Let me directly address your
> two suggestion.
> 
> 1. DAMON_SYSFS Type [1]:
>    I fully agree with this. Centralizing the validation in
>    damon_commit_ctx() is the right approach to avoid "whack-a-mole"
>    problem. This is exactly what I am proposing.

Thank you for clarifying this.  Thanks to that I can show where you are coming
from.  You are misunderstanding what I'm suggesting.  I should have explained
it in more detail.  With this option, I'm not suggesting to update
damon_copmmit_ctx() but the callers, in a way similar to that for DAMON_SYSFS.

> 
> 2. Adding a simple check on existing validation logic (in callers?) [2]:
>    While this is simpler to implement, I prefer avoiding it for the
>    "whack-a-mole".

I suggested option 1 as a way to avoid "whack-a-mole".  I didn't suggest
updting damon_commit_ctx() as the way.

But, the given problem is clear and local.  Validation of addr_unit in
DAMON_RECLAIM and DAMON_LRU_SORT.  There is no problem in DAMON_SYSFS.  So I'd
prefer simpler appraoch on local callers that having problem.

In future, we can make centuralized appraoch, in a way somewhat similar to what
DAMON_SYSFS is doing.  But that's somewhat we can think in future.  For a given
problem, let's fix it first.

> 
> So, to clarify, I choose your first option (centralized check), and I
> believe my "Option 2" is the simple way to implement it.
> 
> Does ths align with your expectation? If so, I will proceed with this
> approach.

So, no, I think we were misunderstanding each other, and I think I understand
you more now, thanks to your clarification.  Also, please don't hesitate at
asking more questions to me if any of my suggestion is unclear.

In short, for this given specific issue, I'd prefer the option 2.  Is this
clear?


Thanks,
SJ

[...]

Re: [PATCH] mm/damon: validate addr_unit to be power of 2

[Liew Rui Yan]

Hi SeongJae,

On Sat, 28 Mar 2026 20:20:54 -0700 SeongJae Park <sj@kernel.org> wrote:

> On Sun, 29 Mar 2026 02:43:19 +0800 Liew Rui Yan <aethernet65535@gmail.com> wrote:
> 
> > Apologize if my previous email was unclear. Let me directly address your
> > two suggestion.
> > 
> > 1. DAMON_SYSFS Type [1]:
> >    I fully agree with this. Centralizing the validation in
> >    damon_commit_ctx() is the right approach to avoid "whack-a-mole"
> >    problem. This is exactly what I am proposing.
> 
> Thank you for clarifying this.  Thanks to that I can show where you are coming
> from.  You are misunderstanding what I'm suggesting.  I should have explained
> it in more detail.  With this option, I'm not suggesting to update
> damon_copmmit_ctx() but the callers, in a way similar to that for DAMON_SYSFS.
> 
> > 
> > 2. Adding a simple check on existing validation logic (in callers?) [2]:
> >    While this is simpler to implement, I prefer avoiding it for the
> >    "whack-a-mole".
> 
> I suggested option 1 as a way to avoid "whack-a-mole".  I didn't suggest
> updting damon_commit_ctx() as the way.
> 
> But, the given problem is clear and local.  Validation of addr_unit in
> DAMON_RECLAIM and DAMON_LRU_SORT.  There is no problem in DAMON_SYSFS.  So I'd
> prefer simpler appraoch on local callers that having problem.
> 
> In future, we can make centuralized appraoch, in a way somewhat similar to what
> DAMON_SYSFS is doing.  But that's somewhat we can think in future.  For a given
> problem, let's fix it first.
> 
> > 
> > So, to clarify, I choose your first option (centralized check), and I
> > believe my "Option 2" is the simple way to implement it.
> > 
> > Does ths align with your expectation? If so, I will proceed with this
> > approach.
> 
> So, no, I think we were misunderstanding each other, and I think I understand
> you more now, thanks to your clarification.  Also, please don't hesitate at
> asking more questions to me if any of my suggestion is unclear.
> 
> In short, for this given specific issue, I'd prefer the option 2.  Is this
> clear?

Thank you for the clarification! I may understand your point now.

So, you prefer to keep the fix local to the modules that have the issue
(addr_unit_store()), rather than changing the CORE damon_commit_ctx()?

To confirm, are you suggesting something like the first approach [1]?

    if (input_addr_unit < PAGE_SIZE && !is_power_of_2(input_addr_unit))
        return -EINVAL;

[1] https://lore.kernel.org/20260325071709.9699-1-aethernet65535@gmail.com

Best regards,
Rui Yan

Re: [PATCH] mm/damon: validate addr_unit to be power of 2

[SeongJae Park]

On Sun, 29 Mar 2026 15:51:07 +0800 Liew Rui Yan <aethernet65535@gmail.com> wrote:
[...]
> Thank you for the clarification! I may understand your point now.
> 
> So, you prefer to keep the fix local to the modules that have the issue
> (addr_unit_store()), rather than changing the CORE damon_commit_ctx()?
> 
> To confirm, are you suggesting something like the first approach [1]?
> 
>     if (input_addr_unit < PAGE_SIZE && !is_power_of_2(input_addr_unit))
>         return -EINVAL;

Yes.  But the real constraint is min_region_sz, so testing it would be more
simple and effective.  E.g.,

	if (!is_power_of_2(param_ctx->min_region_sz))
		return -EINVAL;
> 
> [1] https://lore.kernel.org/20260325071709.9699-1-aethernet65535@gmail.com


Thanks,
SJ

[...]