linux-kernel - Re: [PATCH] mm/damon: add a min_sz_region parameter to damon_set_region_biggest_system_ram

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4c2e5879-e554-49c0-8f05-094031cbb64a@huawei.com>
Date: Fri, 17 Oct 2025 11:12:00 +0800
From: Quanmin Yan <yanquanmin1@...wei.com>
To: SeongJae Park <sj@...nel.org>
CC: <akpm@...ux-foundation.org>, <damon@...ts.linux.dev>,
	<linux-kernel@...r.kernel.org>, <linux-mm@...ck.org>,
	<wangkefeng.wang@...wei.com>, <zuoze1@...wei.com>
Subject: Re: [PATCH] mm/damon: add a min_sz_region parameter to
 damon_set_region_biggest_system_ram_default()

Hi SJ,

在 2025/10/17 3:48, SeongJae Park 写道:
> On Thu, 16 Oct 2025 18:47:17 +0800 Quanmin Yan <yanquanmin1@...wei.com> wrote:
>
>> After adding addr_unit support for DAMON_LRU_SORT and DAMON_RECLAIM,
>> the related region setup now requires alignment based on min_sz_region.
>>
>> Add min_sz_region to damon_set_region_biggest_system_ram_default()
>> and use it when calling damon_set_regions(), replacing the previously
>> hardcoded DAMON_MIN_REGION.
> Can we add more detailed description of the end user issue on the commit
> message?  My understanding of the issue is that the monitoring target address
> ranges for DAMON_LRU_SORT and DAMON_RECLAIM would be aligned on
> DAMON_MIN_REGION * addr_unit.
>
> For example, if user sets the monitoring target address range as [4, 8) and
> addr_unit as 1024, the aimed monitoring target address range is [4 KiB, 8 KiB).
> But damon_set_regions() will apply DAMON_MIN_REGION as the core address
> alignment.  Assuming DAMON_MIN_REGION is 4096, so resulting target address
> range will be [0, 4096) in the DAMON core layer address system, and [0, 4 MiB)
> in the physical address space.
>
> So the end user effect is that DAMON_LRU_SORT and DAMON_RECLAIM could work for
> unexpectedly large physical address ranges, when they 1) set addr_unit to a
> value larger than 1, and 2) set the monitoring target address range as not
> aligned in 4096*addr_unit.
>
> Let me know if I'm misunderstanding something.
>
> Also, if you encountered the issue in a real or a realistic use case, adding
> that on the commit message together would be very helpful.

Thank you for the additional explanation! Your understanding and description of
the issue are entirely correct.

Our ultimate goal is to have the monitoring target address range aligned to
DAMON_MIN_REGION. In the original logic, however, damon_set_regions() did not
take the newly introduced addr_unit into account, and directly performed core
address alignment based only on DAMON_MIN_REGION. As a result, the monitoring
target address range was aligned to DAMON_MIN_REGION * addr_unit, causing
DAMON_LRU_SORT and DAMON_RECLAIM to potentially operate on incorrect physical
address ranges. Therefore, we need to use min_sz_region instead of
DAMON_MIN_REGION in damon_set_regions().

I will add the detailed commit description in the v2 patch.

>> Fixes: 2e0fe9245d6b ("mm/damon/lru_sort: support addr_unit for DAMON_LRU_SORT")
>> Fixes: 7db551fcfb2a ("mm/damon/reclaim: support addr_unit for DAMON_RECLAIM")
> Let's break this patch into two patches, so that we have one fix per broken
> commit.

Yes, I actually considered splitting it up before submitting this patch, but found that
doing so might make the patches look odd. Since we're modifying the input parameters
of damon_set_region_biggest_system_ram_default(), all the call sites of this function
need to be updated accordingly. It seems we might need to split this into three patches:
1. Preparation patch: Add the min_sz_region parameter to
    damon_set_region_biggest_system_ram_default(), passing ctx->min_sz_region in stat.c,
    and passing DAMON_MIN_REGION when calling this function in reclaim.c/lru_sort.c?
2. Fixes patch 1: Modify lru_sort.c to pass param_ctx->min_sz_region.
3. Fixes patch 2: Modify reclaim.c to pass param_ctx->min_sz_region.

I'm not entirely comfortable with this approach. Would it be acceptable to submit this
as a single patch?


Thanks,
Quanmin Yan

>> Signed-off-by: Quanmin Yan <yanquanmin1@...wei.com>
>> ---
>>   include/linux/damon.h | 3 ++-
>>   mm/damon/core.c       | 6 ++++--
>>   mm/damon/lru_sort.c   | 3 ++-
>>   mm/damon/reclaim.c    | 3 ++-
>>   mm/damon/stat.c       | 3 ++-
>>   5 files changed, 12 insertions(+), 6 deletions(-)
> The code change looks good to me.
>
>
> Thanks,
> SJ