2020-05-06 11:22:41

by Tan Hu

Subject: [PATCH] lib/flex_proportions.c: aging counts when fraction smaller than max_frac/FPROP_FRAC_BASE

If the given type has fraction smaller than max_frac/FPROP_FRAC_BASE,
__fprop_inc_percpu_max should follow the design formula and aging
fraction too.

Signed-off-by: Tan Hu <[email protected]>
---
lib/flex_proportions.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/lib/flex_proportions.c b/lib/flex_proportions.c
index 7852bfff5..451543937 100644
--- a/lib/flex_proportions.c
+++ b/lib/flex_proportions.c
@@ -266,8 +266,7 @@ void __fprop_inc_percpu_max(struct fprop_global *p,
if (numerator >
(((u64)denominator) * max_frac) >> FPROP_FRAC_SHIFT)
return;
- } else
- fprop_reflect_period_percpu(p, pl);
- percpu_counter_add_batch(&pl->events, 1, PROP_BATCH);
- percpu_counter_add(&p->events, 1);
+ }
+
+ __fprop_inc_percpu(p, pl);
}
--
2.19.1

2020-05-07 23:48:01

by Andrew Morton

Subject: Re: [PATCH] lib/flex_proportions.c: aging counts when fraction smaller than max_frac/FPROP_FRAC_BASE

On Wed, 6 May 2020 14:21:28 +0800 Tan Hu <[email protected]> wrote:

> If the given type has fraction smaller than max_frac/FPROP_FRAC_BASE,
> __fprop_inc_percpu_max should follow the design formula and aging
> fraction too.
>
> Signed-off-by: Tan Hu <[email protected]>
> ---
> lib/flex_proportions.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/lib/flex_proportions.c b/lib/flex_proportions.c
> index 7852bfff5..451543937 100644
> --- a/lib/flex_proportions.c
> +++ b/lib/flex_proportions.c
> @@ -266,8 +266,7 @@ void __fprop_inc_percpu_max(struct fprop_global *p,
> if (numerator >
> (((u64)denominator) * max_frac) >> FPROP_FRAC_SHIFT)
> return;
> - } else
> - fprop_reflect_period_percpu(p, pl);
> - percpu_counter_add_batch(&pl->events, 1, PROP_BATCH);
> - percpu_counter_add(&p->events, 1);
> + }
> +
> + __fprop_inc_percpu(p, pl);
> }

(Cc Jan)

2020-05-08 09:24:13

by Jan Kara

Subject: Re: [PATCH] lib/flex_proportions.c: aging counts when fraction smaller than max_frac/FPROP_FRAC_BASE

Thanks for CC Andrew.

On Thu 07-05-20 16:46:14, Andrew Morton wrote:
> On Wed, 6 May 2020 14:21:28 +0800 Tan Hu <[email protected]> wrote:
>
> > If the given type has fraction smaller than max_frac/FPROP_FRAC_BASE,
> > __fprop_inc_percpu_max should follow the design formula and aging
> > fraction too.
> >
> > Signed-off-by: Tan Hu <[email protected]>
> > ---
> > lib/flex_proportions.c | 7 +++----
> > 1 file changed, 3 insertions(+), 4 deletions(-)
> >
> > diff --git a/lib/flex_proportions.c b/lib/flex_proportions.c
> > index 7852bfff5..451543937 100644
> > --- a/lib/flex_proportions.c
> > +++ b/lib/flex_proportions.c
> > @@ -266,8 +266,7 @@ void __fprop_inc_percpu_max(struct fprop_global *p,
> > if (numerator >
> > (((u64)denominator) * max_frac) >> FPROP_FRAC_SHIFT)
> > return;
> > - } else
> > - fprop_reflect_period_percpu(p, pl);
> > - percpu_counter_add_batch(&pl->events, 1, PROP_BATCH);
> > - percpu_counter_add(&p->events, 1);
> > + }
> > +
> > + __fprop_inc_percpu(p, pl);

So the code is actually correct as is because if max_frac <
FPROP_FRAC_BASE, we call fprop_fraction_percpu() which calls
fprop_reflect_period_percpu(). So the 'else' is there to avoid calling the
function twice. That being said calling fprop_reflect_period_percpu() twice
as we would with your patch does no big harm as we'd just quickly bail on
pl->period == p->period test. So I think the code is easier to understand
the way you suggest without significant downside. But please update the
changelog to explain that this is just code cleanup (with the above
reasoning) and not a functional fix.

Honza

--
Jan Kara <[email protected]>
SUSE Labs, CR