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

[Liew Rui Yan <aethernet65535@gmail.com>]

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