Re: [PATCH 2/3] flex_proportions: Allow N events instead of 1

[Jan Kara <jack@suse.cz>]

On Mon 22-03-21 22:35:27, Matthew Wilcox wrote:
> On Mon, Mar 22, 2021 at 08:11:15PM +0000, Matthew Wilcox (Oracle) wrote:
> > -void __fprop_inc_percpu_max(struct fprop_global *p,
> > -			    struct fprop_local_percpu *pl, int max_frac)
> > +void __fprop_add_percpu_max(struct fprop_global *p,
> > +		struct fprop_local_percpu *pl, int max_frac, long nr)
> >  {
> >  	if (unlikely(max_frac < FPROP_FRAC_BASE)) {
> 
> Oh, I meant to ask ... should this change?  Should it be:
> 
> 	if (unlikely(max_frac < FPROP_FRAC_BASE / nr))
> (or something similar that copes with overflow properly)
> I must confess to not understanding exactly how flex_proportions works.

No, __fprop_inc_percpu_max() implements "saturation" arithmetics on
fractions. So if the fraction tracked by 'pl' exceeds
max_frac/FPROP_FRAC_BASE, we don't want to increment 'pl' further. If 'nr'
is going to be small, we probably don't care that we somewhat exceed the
max_frac/FPROP_FRAC_BASE. But I suppose 'nr' can be say 512 at which point
(given FPROP_FRAC_BASE is 1024) one addition can make a very significant
difference. So we probably need to be more clever like:

	if (unlikely(max_frac < FPROP_FRAC_BASE)) {
		unsigned long numerator, denominator;
		s64 tmp;

		fprop_fraction_percpu(p, pl, &numerator, &denominator);
		/* Adding 'nr' to fraction exceeds max_frac/FPROP_FRAC_BASE? */
		tmp = (u64)denominator * max_frac -
					((u64)numerator << FPROP_FRAC_SHIFT);
		if (tmp < 0) {
			/* Maximum fraction already exceeded? */
			return;
		} else if (tmp < nr * (FPROP_FRAC_BASE - max_frac)) {
			/* Add just enough for the fraction to saturate */
			nr = div_u64(tmp + FPROP_FRAC_BASE - max_frac - 1,
				     FPROP_FRAC_BASE - max_frac);
		}
	}

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR