Re: [PATCH v1 3/3] mm/damon/sysfs: support hugepage_mem_bp quota goal metric

On 7/15/2026 6:16 PM, SJ Park wrote:
> On Wed, 15 Jul 2026 17:47:38 +0300 Gutierrez Asier <[email protected]> wrote:
> 
>> Hi SJ,
>>
>> On 6/17/2026 7:15 AM, SeongJae Park wrote:
>>> On Tue, 16 Jun 2026 22:35:21 +0300 Gutierrez Asier <[email protected]> wrote:
>>>
>>>>
>>>>
>>>> On 6/16/2026 6:21 PM, [email protected] wrote:
>>>>> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
>>>>>
>>>>> New issues:
>>>>> - [Low] The new `DAMOS_QUOTA_HUGEPAGE_MEM_BP` metric is exposed via sysfs but lacks required documentation updates.
>>>>>
>>>>> Pre-existing issues:
>>>>> - [Critical] Potential divide-by-zero in `damos_hugepage_mem_bp()` and `damos_get_in_active_mem_bp()` due to an unhandled zero denominator.
>>>>> - [High] 32-bit integer overflow in `damos_hugepage_mem_bp()` and `damos_get_in_active_mem_bp()` when calculating memory ratios.
>>>>> --
>>>>>
>>>>> commit be304699fb94d4d73bd979237fb4d7b08b7216f7
>>>>> Author: Asier Gutierrez <[email protected]>
>>>>>
>>>>> mm/damon/sysfs: support hugepage_mem_bp quota goal metric
>>>>>
>>>>> DAMOS has a new autotune policy metric: DAMOS_QUOTA_HUGEPAGE_MEM_BP.
>>>>> This patch exposes DAMOS_QUOTA_HUGEPAGE_MEM_BP through sysfs.
>>>>>
>>>>>> diff --git a/mm/damon/sysfs-schemes.c b/mm/damon/sysfs-schemes.c
>>>>>> index 329cfd0bbe9f3..4b5429a5b4158 100644
>>>>>> --- a/mm/damon/sysfs-schemes.c
>>>>>> +++ b/mm/damon/sysfs-schemes.c
>>>>>> @@ -1267,6 +1267,10 @@ struct damos_sysfs_qgoal_metric_name damos_sysfs_qgoal_metric_names[] = {
>>>>>>  		.metric = DAMOS_QUOTA_NODE_ELIGIBLE_MEM_BP,
>>>>>>  		.name = "node_eligible_mem_bp",
>>>>>>  	},
>>>>>> +	{
>>>>>> +		.metric = DAMOS_QUOTA_HUGEPAGE_MEM_BP,
>>>>>> +		.name = "hugepage_mem_bp",
>>>>>> +	},
>>>>>
>>>>> [Severity: Critical]
>>>>> This isn't a bug introduced by this patch, but exposing this metric directly
>>>>> to userspace makes a potential divide-by-zero triggerable by users
>>>>> configuring DAMON quotas.
>>>>>
>>>>> In mm/damon/core.c:damos_hugepage_mem_bp(), the denominator is computed as:
>>>>>
>>>>>     total = totalram_pages() - global_zone_page_state(NR_FREE_PAGES);
>>>>>
>>>>> Due to per-CPU vmstat lag or an idle system, the free pages counter can equal
>>>>> totalram_pages(), resulting in total being 0. This is then passed to
>>>>> mult_frac():
>>>>>
>>>>>     return mult_frac(thp, 10000, total);
>>>>>
>>>>> This inherently computes thp / total, causing a strict division by zero and a
>>>>> kernel panic. The same issue exists in damos_get_in_active_mem_bp() where
>>>>> total = active + inactive, which can be 0 if the LRU lists are empty.
>>>>>
>>>>> Should a zero-check on total be added before performing the division?
>>>> DAMON is enabled after the system is fully loaded. total will not be 0 by then.
>>>
>>> Could you please further clarify what "system is fully loaded" means, reaards
>>> to what Sashiko is claiming, particularly the per-CPU vmstat lag?
>>>
>>>
>>> Thanks,
>>> SJ
>>>
>>> [...]
>>>
>>
>> What I meant is the system already initialized after booting. The thing
>> is that by the time the sysadmin sets up DAMON, vmstat should show at
>> least some memory usage, not 0. That was my guess, but I may be wrong.
> 
> I think that's a fair argument.  And I also not quite sure.  But having zero
> due to the races in per-CPU vmstat sounds possible in an extreme and weird lab
> environments.
> 
>>
>> If my initial thought was wrong, I can just add a guard there.
> 
> So I'm not really sure if you're wrong.  But adding the guard would be simple.
> I'd recommend just adding the guard instead of get wrestle on this thoery for
> now.  Does that make sense?
Yes, makes sense. I will add a guard.
> 
> Thanks,
> SJ
> 
> [...]

-- 
Asier Gutierrez
Huawei