Re: [RFC PATCH v2.2 21/28] Docs/admin-guide/mm/damon/usage: document data attributes monitoring

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] The documentation contains a contradiction regarding the sysfs layout for `tried_regions/<N>/`. The sysfs hierarchy tree diagram shows a new `probes` directory containing numbered subdirectories with `hits` files (e.g., `probes/0/hits`), but the text section incorrectly claims a single `probe_hits` file is added directly in the region directory.
- [Low] There are several typographical, grammatical, and formatting errors in the newly added documentation.
--

commit 1f54a921d1c61b444501227d66d664dd4513eef3
Author: SeongJae Park <sj@kernel.org>

Docs/admin-guide/mm/damon/usage: document data attributes monitoring

Update DAMON usage document for the newly added data attributes
monitoring feature.

> diff --git a/Documentation/admin-guide/mm/damon/usage.rst b/Documentation/admin-guide/mm/damon/usage.rst
> index 534e1199cf091..44224da62533e 100644
> --- a/Documentation/admin-guide/mm/damon/usage.rst
> +++ b/Documentation/admin-guide/mm/damon/usage.rst
> @@ -71,6 +71,11 @@ comma (",").
>      │ │ │ │ │ │ intervals/sample_us,aggr_us,update_us
>      │ │ │ │ │ │ │ intervals_goal/access_bp,aggrs,min_sample_us,max_sample_us
>      │ │ │ │ │ │ nr_regions/min,max
> +    │ │ │ │ │ │ :ref:`probes <damon_usage_sysfs_probes>`/nr_probes
> +    │ │ │ │ │ │ │ 0/filters/nr_filters
> +    │ │ │ │ │ │ │ │ │ 0/type,matching,allow

This isn't a bug, but there's a formatting issue here. The indentation skips
from 7 vertical bars on the previous line to 9 vertical bars on this line.

> +    │ │ │ │ │ │ │ │ │ ...
> +    │ │ │ │ │ │ │ │ ...
>      │ │ │ │ │ :ref:`targets <sysfs_targets>`/nr_targets
>      │ │ │ │ │ │ :ref:`0 <sysfs_target>`/pid_target,obsolete_target
>      │ │ │ │ │ │ │ :ref:`regions <sysfs_regions>`/nr_regions

[ ... ]

> @@ -94,7 +99,10 @@ comma (",").
>      │ │ │ │ │ │ │ │ 0/id,weight
>      │ │ │ │ │ │ │ :ref:`stats <sysfs_schemes_stats>`/nr_tried,sz_tried,nr_applied,sz_applied,sz_ops_filter_passed,qt_exceeds,nr_snapshots,max_nr_snapshots
>      │ │ │ │ │ │ │ :ref:`tried_regions <sysfs_schemes_tried_regions>`/total_bytes
> -    │ │ │ │ │ │ │ │ 0/start,end,nr_accesses,age,sz_filter_passed
> +    │ │ │ │ │ │ │ │ 0/start,end,nr_accesses,age,sz_filter_passed,

This isn't a bug, but there is an erroneous trailing comma left at the end of
this line.

> +    │ │ │ │ │ │ │ │ │ probes
> +    │ │ │ │ │ │ │ │ │ │ 0/hits
> +    │ │ │ │ │ │ │ │ │ │ ...
>      │ │ │ │ │ │ │ │ ...
>      │ │ │ │ │ │ ...

[ ... ]

> @@ -221,8 +229,8 @@ contexts/<N>/monitoring_attrs/
>  
>  Files for specifying attributes of the monitoring including required quality
>  and efficiency of the monitoring are in ``monitoring_attrs`` directory.
> -Specifically, two directories, ``intervals`` and ``nr_regions`` exist in this
> -directory.
> +Specifically, two directories, ``intervals``, ``nr_regions`` and ``probes``
> +exist in this directory.

This isn't a bug, but the text now lists three directories ("intervals",
"nr_regions", and "probes") while still stating there are "two directories".

>  Under ``intervals`` directory, three files for DAMON's sampling interval
>  (``sample_us``), aggregation interval (``aggr_us``), and update interval

[ ... ]

> @@ -256,6 +264,27 @@ tuning-applied current values of the two intervals can be read from the
>  ``sample_us`` and ``aggr_us`` files after writing ``update_tuned_intervals`` to
>  the ``state`` file.
>  
> +.. _damon_usage_sysfs_probes:
> +
> +contexts/<N>/monitoring_attrs/probes/
> +-------------------------------------
> +
> +A directory for registering :ref:`data attributes monitoring
> +<damon_design_data_attrs_monitoring>` probes.
> +
> +In the beginning, this directory has only one file, ``nr_probes``.  Writing a
> +number (``N``) to the file creates the number of child directories named ``0``
> +to ``N-1``.  Each directory represents each monitoring probe.
> +
> +In each probe directory, one directory, ``filters`` exist.  The directory

This isn't a bug, but "exist" should probably be "exists" to agree with the
singular subject "one directory".

> +contains files for installingt filters for the probe, that is used to determine

This isn't a bug, but "installingt" appears to be a typo.

> +the data attribute for the probe.
> +
> +In the beginning, ``filters`` directory has only one file, ``nr_filters``.
> +Writing a number (``N``) to the file creates the number of child directories
> +named ``0`` to ``N-1``.  Each directory represents each filter and work in a

This isn't a bug, but "work" should probably be "works" to agree with "Each
directory".

> +way similar to that for :ref:`DAMOS filter <sysfs_filters>`.
> +
>  .. _sysfs_targets:

[ ... ]

> @@ -600,10 +629,13 @@ set the ``access pattern`` as their interested pattern that they want to query.
>  tried_regions/<N>/
>  ------------------
>  
> -In each region directory, you will find five files (``start``, ``end``,
> -``nr_accesses``, ``age``, and ``sz_filter_passed``).  Reading the files will
> -show the properties of the region that corresponding DAMON-based operation
> -scheme ``action`` has tried to be applied.
> +In each region directory, you will find six files (``start``, ``end``,
> +``nr_accesses``, ``age``, ``sz_filter_passed`` and ``probe_hits``).  Reading
> +the files will show the properties of the region that corresponding DAMON-based
> +operation scheme ``action`` has tried to be applied.

Is there a contradiction regarding the sysfs layout for tried_regions/<N>/?

The sysfs hierarchy tree diagram updated earlier in this patch shows a new
"probes" directory containing numbered subdirectories with "hits" files:

    │ │ │ │ │ │ │ │ 0/start,end,nr_accesses,age,sz_filter_passed,
    │ │ │ │ │ │ │ │ │ probes
    │ │ │ │ │ │ │ │ │ │ 0/hits

However, the text here incorrectly claims a single "probe_hits" file is added
directly in the region directory instead of the "probes" subdirectory tree.

> +
> +Reading ``probe_hists`` shows the number of data attributes monitoring
> +probe-hit positive samples of the region.

This isn't a bug, but "probe_hists" is a typo.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260515004433.128933-1-sj@kernel.org?part=21