Re: [PATCH] mm/damon: update expired description of damos_action

[SeongJae Park <sj@kernel.org>]

Hello Sang-Heon,

On Thu, 31 Jul 2025 22:22:30 +0900 Sang-Heon Jeon <ekffu200098@gmail.com> wrote:

> Nowadays, damos operation actions support more various operation set.
> But comments(also, generated documentation) doesn't updated.
> So, fix the comments with current support status.

Thank you for catching this and fixing!

> 
> Signed-off-by: Sang-Heon Jeon <ekffu200098@gmail.com>
> ---
> I also found below patch(https://lore.kernel.org/damon/20250730200239.60984-1-sj@kernel.org/T/#t)
> So, Is it means that DAMOS_STAT is only supported by paddr now?

No.  'vaddr' supports DAMOS_STAT.  The patch is not adding DAMOS_STAT support
on vaddr, but extending the support for DAMOS filters.

Thank you for checking the thread and asking this question, though.  I see this
as an important signal of another rooms to improve on the documentation.

> If my understanding is correct, then we need to fix Design documentation
> and comments both.
> 
> However, since it will be fixed soon. Keeping current status is also
> affordable. So could you check this point as well? 
> 
> ---
> 
>  include/linux/damon.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/damon.h b/include/linux/damon.h
> index cddf82eaac4e..0870e9f48b0f 100644
> --- a/include/linux/damon.h
> +++ b/include/linux/damon.h
> @@ -128,7 +128,7 @@ struct damon_access_report {

Seems you are using damon/next as your baseline.  If there is no real
dependencies to damon/next, please use mm-new as your baseline.  For more
contexts about that, please refer to the DAMON maintainer-profile[1].

>   *
>   * @DAMOS_WILLNEED:	Call ``madvise()`` for the region with MADV_WILLNEED.
>   * @DAMOS_COLD:		Call ``madvise()`` for the region with MADV_COLD.
> - * @DAMOS_PAGEOUT:	Call ``madvise()`` for the region with MADV_PAGEOUT.
> + * @DAMOS_PAGEOUT:	Reclaim the region.

Nice!

>   * @DAMOS_HUGEPAGE:	Call ``madvise()`` for the region with MADV_HUGEPAGE.
>   * @DAMOS_NOHUGEPAGE:	Call ``madvise()`` for the region with MADV_NOHUGEPAGE.
>   * @DAMOS_LRU_PRIO:	Prioritize the region on its LRU lists.
> @@ -146,8 +146,8 @@ struct damon_access_report {
>   * The support of each action is up to running &struct damon_operations.
>   * &enum DAMON_OPS_VADDR and &enum DAMON_OPS_FVADDR supports all actions except
>   * &enum DAMOS_LRU_PRIO and &enum DAMOS_LRU_DEPRIO.  &enum DAMON_OPS_PADDR
> - * supports only &enum DAMOS_PAGEOUT, &enum DAMOS_LRU_PRIO, &enum
> - * DAMOS_LRU_DEPRIO, and &DAMOS_STAT.
> + * supports all actions except &enum DAMOS_WILLNEED, &enum DAMOS_COLD,
> + * &enum DAMOS_HUGEPAGE, &enum DAMOS_NOHUGEPAGE.

Thank you for updating this!

But, now I find this place is having outdated information that partially
duplicated with the DAMON design document[2].  I think the design document is
better to maintain this kind of details.  What about keeping the high level
information but giving a reference to the documentation for details, for
example, like below?

    @@ -143,11 +143,9 @@ struct damon_access_report {
      * @DAMOS_STAT:                Do nothing but count the stat.
      * @NR_DAMOS_ACTIONS:  Total number of DAMOS actions
      *
    - * The support of each action is up to running &struct damon_operations.
    - * &enum DAMON_OPS_VADDR and &enum DAMON_OPS_FVADDR supports all actions except
    - * &enum DAMOS_LRU_PRIO and &enum DAMOS_LRU_DEPRIO.  &enum DAMON_OPS_PADDR
    - * supports only &enum DAMOS_PAGEOUT, &enum DAMOS_LRU_PRIO, &enum
    - * DAMOS_LRU_DEPRIO, and &DAMOS_STAT.
    + * The support of each action is up to running &struct damon_operations.  Refer
    + * to 'Operation Action' section of Documentation/mm/damon/design.rst for
    + * status of the supports.
      */

Thanks,
SJ


[1] https://origin.kernel.org/doc/html/next/mm/damon/maintainer-profile.html#scm-trees
[2] https://origin.kernel.org/doc/html/next/mm/damon/design.html#operation-action