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

[SeongJae Park <sj@kernel.org>]

Dropped individual Cc's except myself to reduce unnecessary traffic.  I
initailly thought it would be good to keep Andrew in the Cc list, but even
Andrew may not really interested in each sashiko review for RFC patch, so I
dropped him from the list, too.

sashiko found issues.  I will fix those in the next revision.

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

> # review url: https://sashiko.dev/#/patchset/20260404163943.89278-4-sj@kernel.org
> # start of sashiko.dev inline review
> commit bab641e86f0e9820b2ac0a7d4ae493d23c4c50f4
> Author: SeongJae Park <sj@kernel.org>
> Docs/mm/damon/design: document fail_charge_{num,denom}
> 
> This commit updates the DAMON design document to explain the action-failed
> memory charging ratio. It provides an example of how to calculate the
> charged memory when an action fails for a portion of a region.
> 
> > diff --git a/Documentation/mm/damon/design.rst b/Documentation/mm/damon/design.rst
> > index afc7d52bda2f7..01047e8431d17 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 sub subsets of the memory of the
> 
> This isn't a bug, but should "sub subsets" just be "some subsets" or
> "subsets"?

Good eye.  I will fix this in the next revision.

> 
> > +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.
> > +
> > +For example, let's suppose a DAMOS action is applied to a region of 1 GiB 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.
> 
> Is this calculation accurate? Since 1 GiB is 1024 MiB, if 700 MiB are
> successfully applied, the remaining failed region would be 324 MiB rather
> than 300 MiB. Does the example intentionally use 1 GiB = 1000 MiB, or should
> the remaining portion be updated to 324 MiB?

Nice finding.  I will fix this in the next revision.


Thanks,
SJ

> 
> 
> # end of sashiko.dev inline review
> # review url: https://sashiko.dev/#/patchset/20260404163943.89278-4-sj@kernel.org

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