Re: [RFC PATCH v4 4/4] Documentation/admin-guide/mm/damon: add DAMON-based Hugepage Management

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [Medium] The commit message incorrectly states that the module collapses hot memory regions 'back to regular pages'.
- [Medium] The documentation incorrectly claims the page collapsing limit is up to 1 GiB per second, whereas the code restricts it to 128 MiB per second.
- [Medium] The documentation incorrectly describes the huge page quota percentage metric as a ratio against anonymous memory.
- [Medium] The documentation claims administrators can provide dynamic manual feedback to adjust the quota, but the module lacks runtime update capabilities.
- [Low] Missing newline at the end of the newly added documentation file.
- [Low] Multiple typographical and grammatical errors in the documentation.
--

commit 9d20df125ff887031458f9f9da39b696dcac8419
Author: Asier Gutierrez <gutierrez.asier@huawei-partners.com>

Documentation/admin-guide/mm/damon: add DAMON-based Hugepage Management

This patch adds documentation for the DAMON-based Hugepage Management
feature. It covers the module's features, operation, and all available
module parameters.

> Add documentation for the DAMON-based Hugepage Management
> (SAMPLE_DAMON_HUGEPAGE) feature, which automatically manages
> huge pages by identifying hot memory regions and collapsing
> them back to regular pages.

[Severity: Medium]
Does this module actually collapse huge pages into regular pages?

The documentation below correctly notes that the module collapses regular
pages into huge pages. Could the commit message be updated to reflect the
actual operation?

> diff --git a/Documentation/admin-guide/mm/damon/hugepage.rst b/Documentation/admin-guide/mm/damon/hugepage.rst
> new file mode 100644
> index 0000000000000..470ae2bc8cc31
> --- /dev/null
> +++ b/Documentation/admin-guide/mm/damon/hugepage.rst

[ ... ]

> +quota_autotune_feedback
> +-----------------------
> +
> +User-specifiable feedback for auto-tuning of the effective quota.
> +
> +While keeping the caps that set by other quotas, SAMPLE_DAMON_HUGEPAGE
                     ^^^^^^^^^^^^^^^^^^

[Severity: Low]
This isn't a bug, but there is a typo here. Should this be "caps set by"?

> +automatically increases and decreases the effective level of the quota
> +aiming receiving this feedback of value 10,000 from the user.
   ^^^^^^^^^^^^^^^^

[Severity: Low]
This isn't a bug, but there is a typo here. Should this be "aiming to receive"?

[Severity: Medium]
Does the module actually accept runtime feedback for the quota?

Unlike other DAMON modules like reclaim, this module does not seem to have a
commit_inputs mechanism to apply runtime updates to the DAMON context. 

Since quota_autotune_feedback is only read once during
damon_sample_hugepage_start(), is this dynamic feedback mechanism supported
by the code?

> +SAMPLE_DAMON_HUGEPAGE assumes the feedback value and the quota are positively
> +proportional.  Value zero means disabling this auto-tuning feature.
> +
> +Disabled by default.
> +
> +quota_percentage_hugepage
> +-------------------------
> +
> +Huge page consumption to total memory anonymous memory consumption ratio goal
> +in bp (10,000). SAMPLE_DAMON_HUGEPAGE automatically increases and

[Severity: Medium]
Is this ratio actually based on anonymous memory consumption?

Looking at damos_hugepage_mem_bp(), it calculates the ratio against the total
system used memory:
totalram_pages() - global_zone_page_state(NR_FREE_PAGES)

Could this be updated to reflect it does not just use anonymous memory?

[ ... ]

> +Example
> +=======
> +
> +Below runtime example commands make SAMPLE_DAMON_HUGEPAGE to find memory
> +regions of  the task with PID 1234 that have been accessed in the last 100
              ^^

[Severity: Low]
This isn't a bug, but there is an extra space here.

> +milliseconds or more and collpases those pages into huge pages. The page
                            ^^^^^^^^^

[Severity: Low]
This isn't a bug, but there is a typo here. Should this be "collapses"?

> +collapsing is limited to be done only up to 1 GiB per second to avoid

[Severity: Medium]
Is the limit actually 1 GiB per second?

The underlying module statically sets the quota limit (damos_quota.sz) to
128 * 1024 * 1024 (128 MiB) with a reset_interval of 1000 ms.

> +SAMPLE_DAMON_HUGEPAGE consuming too much CPU time for the collapse operation. ::
> +
> +    # cd /sys/module/damon_sample_hugepage/parameters
> +    # echo 1234 > target_pid
> +    # echo Y > enabled
> +
> +Note that this module (SAMPLE_DAMON_HUGEPAGE) cannot run simultaneously
> +with other DAMON-based special-purpose modules.  Refer to
> +:ref:`DAMON design special purpose modules exclusivity
> +<damon_design_special_purpose_modules_exclusivity>` for more details.
> \ No newline at end of file

[Severity: Low]
This isn't a bug, but standard POSIX text file and Linux kernel coding
guidelines require a trailing newline at the end of the file.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260611150244.3454699-1-gutierrez.asier@huawei-partners.com?part=4