Re: (sashiko review) [RFC PATCH v2 3/9] Docs/mm/damon/design: document fail_charge_{num,denom}

[SeongJae Park]

TL; DR: sashiko gave me a sounds-good suggestion.  I will make a change on the
next revision of this patch.

Forwarding full sashiko review in a reply format with my inline
comments below, for sharing details of my view and doing
followup discussions via mails if needed.

> # review url: https://sashiko.dev/#/patchset/20260405151232.102690-4-sj@xxxxxxxxxx
> # start of sashiko.dev inline review
> commit c4d090b9bc99178548ec06da52d6f75e0b392a34
> Author: SeongJae Park <sj@xxxxxxxxxx>
> 
> Docs/mm/damon/design: document fail_charge_{num,denom}
> 
> Update the DAMON design document to describe the new fail_charge_num and
> fail_charge_denom parameters. These parameters configure the DAMOS action
> failed region quota charge ratio.
> 
> > diff --git a/Documentation/mm/damon/design.rst b/Documentation/mm/damon/design.rst
> > index afc7d52bda2f7..8a8b04b5e8537 100644
> > --- a/Documentation/mm/damon/design.rst
> > +++ b/Documentation/mm/damon/design.rst
> > @@ -565,6 +565,27 @@ interface <sysfs_interface>`, refer to :ref:`weights <sysfs_quotas>` part of
> >  the documentation.
> >  
> >  
> > +.. _damon_design_damos_quotas_failed_memory_charging_ratio:
> > +
> > +Action-failed Memory Charging Ratio
> > +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > +
> > +DAMOS action to a given region can fail for some subsets of the memory of the
> > +region.  For example, if the action is ``pageout`` and the region has some
> > +unreclaimable pages, applying the action to the pages will fail.  The amount of
> > +system resource that is taken for such failed action applications is usually
> > +different from that for successful action applications.  For such cases, users
> > +can set different charging ratio for such failed memory.  The ratio can be
> > +specified using ``fail_charge_num`` and ``fail_charge_denom`` parameters.  The
> > +two parameters represent the numerator and denominator of the ratio.
> 
> Should this documentation include the fallback and default behavior when
> fail_charge_denom is set to 0?
> 
> Looking at the core kernel implementation in
> mm/damon/core.c:damos_charge_quota(), the code checks for
> !quota->fail_charge_denom and falls back to charging the entire region size to
> avoid a division-by-zero error.
> 
> Additionally, since struct damon_sysfs_quotas is allocated using kzalloc(),
> fail_charge_denom is 0 by default. Would it be helpful to explain this default
> 0 state and the resulting fallback behavior here?

Good suggestion, I will add that.

> 
> > +
> > +For example, let's suppose a DAMOS action is applied to a region of 1,000 MiB
> > +size.  The action is successfully applied to only 700 MiB of the region.
> > +``fail_charge_num`` and ``fail_charge_denom`` are set to ``1`` and ``1024``,
> > +respectively.  Then only 700 MiB and 300 KiB of size (``700 MiB + 300 MiB * 1 /
> > +1024``) will be charged.
> > +
> 
> [ ... ]
> 
> 
> # end of sashiko.dev inline review
> # review url: https://sashiko.dev/#/patchset/20260405151232.102690-4-sj@xxxxxxxxxx


Thanks,
SJ

# hkml [1] generated a draft of this mail.  You can regenerate
# this using below command:
#
#     hkml patch sashiko_dev --for_forwarding \
#             20260405151232.102690-4-sj@xxxxxxxxxx
#
# [1] https://github.com/sjp38/hackermail