Re: [RFC PATCH] mm/damon: explain "effective quota" on kernel-doc comment

[Honggyu Kim <honggyu.kim@sk.com>]

On Thu, 28 Nov 2024 09:50:46 -0800 SeongJae Park <sj@kernel.org> wrote:
> On Thu, 28 Nov 2024 18:53:55 +0900 Honggyu Kim <honggyu.kim@sk.com> wrote:
> 
> > On Tue, 26 Nov 2024 11:43:47 -0800 SeongJae Park <sj@kernel.org> wrote:
> > > On Tue, 26 Nov 2024 17:24:33 +0900 Honggyu Kim <honggyu.kim@sk.com> wrote:
> > > 
> > > > Hi SeongJae,
> > > > 
> > > > Thanks very much for the quick response.
> > > 
> > > No problem, all owing to your kind report!
> > > 
> > > > I think it looks great but I
> > > > have some minor comments so please see my inline comments below.
> > > > 
> > > > Thanks,
> > > > Honggyu
> > > > 
> > > > On Mon, 25 Nov 2024 16:29:21 -0800 SeongJae Park <sj@kernel.org> wrote:
> > > > > The kernel-doc comment for 'struct damos_quota' describes how "effective
> > > > > quota" is calculated, but does not explain what it is.  Actually there
> > > > > was an input[1] about it.  Add the explanation on the comment.
> > > > > 
> > > > > [1] https://github.com/damonitor/damo/issues/17#issuecomment-2497525043
> > > > > 
> > > > > Cc: Yunjeong Mun <yunjeong.mun@sk.com>
> > > > > Cc: Honggyu Kim <honggyu.kim@sk.com>
> > > > > Signed-off-by: SeongJae Park <sj@kernel.org>
> > > > > ---
> > > > >  include/linux/damon.h | 10 +++++++---
> > > > >  1 file changed, 7 insertions(+), 3 deletions(-)
> > > > > 
> > > > > diff --git a/include/linux/damon.h b/include/linux/damon.h
> > > > > index a67f2c4940e9..a01bfe2ff616 100644
> > > > > --- a/include/linux/damon.h
> > > > > +++ b/include/linux/damon.h
> > > > > @@ -193,9 +193,13 @@ struct damos_quota_goal {
> > > > >   * size quota is set, DAMON tries to apply the action only up to &sz bytes
> > > > >   * within &reset_interval.
> > > > >   *
> > > > > - * Internally, the time quota is transformed to a size quota using estimated
> > > > > - * throughput of the scheme's action.  DAMON then compares it against &sz and
> > > > > - * uses smaller one as the effective quota.
> > > > > + * To convince the different types of quotas and goals, DAMON internally
> > > > > + * converts those into one single size quota called "effective quota".  DAMON
> > > > 
> > > > Could we use "effective size quota" instead of "effective quota"?
> > > > IMHO, it will better give an idea this is related to "esz" in the code,
> > > > which means effective size.
> > > 
> > > The above sentence is saying it as one single "size" quota, so calling it
> > > "effective size quota" here feels like unnecessary duplicates of the word
> > > ("size") to me.  I'd like to keep this sentence as is if you don't really mind.
> > 
> > Since the time or other goals are eventually transformed into a size
> > quota, I thought the "effective size quota" makes sense but I won't
> > stick to my term here.
> 
> I understand your concern.  But I want to make it not very strictly fixed and
> well-defined term, but just somewhat understandable with common sense and given
> context, for flexibility and conciseness.  So unless this is really makes it
> difficult to understand what it means even with common senses and the context
> I'd like to keep current form.
> 
> I believe your answer to the above question is "no" since you mentioned you
> won't stick to your term.  Please let me know if I'm misreading you.

OK. I'm fine with the changes with this patch.  I won't ask more changes.

> > 
> > We originally asked this question about the term "effective" itself as
> > we didn't find an explanation what "effective" means actually in the
> > doc.  It'd be better to have more explicit explanation as well.
> 
> I think this patch makes the point pretty explicit, so my humble brain is bit
> confused what "more explicit" really means.  Could you please clarify what
> changes you want to be added?

Nevermind, your changes here with some grammar and typo fixes are enough
for this patch.

Thanks,
Honggyu

> 
> Thanks,
> SJ
> 
> [...]