linux-kernel - Re: [PATCH] lib/flex_proportions.c: aging counts when fraction smaller than max_frac/FPROP_FRAC

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Fri, 8 May 2020 11:20:03 +0200
From:   Jan Kara <jack@...e.cz>
To:     Andrew Morton <akpm@...ux-foundation.org>
Cc:     Tan Hu <tan.hu@....com.cn>, linux-kernel@...r.kernel.org,
        xue.zhihong@....com.cn, wang.yi59@....com.cn,
        wang.liang82@....com.cn, Jan Kara <jack@...e.cz>
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 <tan.hu@....com.cn> 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 <tan.hu@....com.cn>
> > ---
> >  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 <jack@...e.com>
SUSE Labs, CR