| Message ID | 20210322201116.2149432-3-willy@infradead.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Allow for multiple pages in a single writeback event | expand |
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.
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
diff --git a/include/linux/flex_proportions.h b/include/linux/flex_proportions.h index c12df59d3f5f..3e378b1fb0bc 100644 --- a/include/linux/flex_proportions.h +++ b/include/linux/flex_proportions.h @@ -83,9 +83,10 @@ struct fprop_local_percpu { int fprop_local_init_percpu(struct fprop_local_percpu *pl, gfp_t gfp); void fprop_local_destroy_percpu(struct fprop_local_percpu *pl); -void __fprop_inc_percpu(struct fprop_global *p, struct fprop_local_percpu *pl); -void __fprop_inc_percpu_max(struct fprop_global *p, struct fprop_local_percpu *pl, - int max_frac); +void __fprop_add_percpu(struct fprop_global *p, struct fprop_local_percpu *pl, + long nr); +void __fprop_add_percpu_max(struct fprop_global *p, + struct fprop_local_percpu *pl, int max_frac, long nr); void fprop_fraction_percpu(struct fprop_global *p, struct fprop_local_percpu *pl, unsigned long *numerator, unsigned long *denominator); @@ -96,7 +97,7 @@ void fprop_inc_percpu(struct fprop_global *p, struct fprop_local_percpu *pl) unsigned long flags; local_irq_save(flags); - __fprop_inc_percpu(p, pl); + __fprop_add_percpu(p, pl, 1); local_irq_restore(flags); } diff --git a/lib/flex_proportions.c b/lib/flex_proportions.c index 451543937524..60029bf541c6 100644 --- a/lib/flex_proportions.c +++ b/lib/flex_proportions.c @@ -217,11 +217,12 @@ static void fprop_reflect_period_percpu(struct fprop_global *p, } /* Event of type pl happened */ -void __fprop_inc_percpu(struct fprop_global *p, struct fprop_local_percpu *pl) +void __fprop_add_percpu(struct fprop_global *p, struct fprop_local_percpu *pl, + long nr) { fprop_reflect_period_percpu(p, pl); - percpu_counter_add_batch(&pl->events, 1, PROP_BATCH); - percpu_counter_add(&p->events, 1); + percpu_counter_add_batch(&pl->events, nr, PROP_BATCH); + percpu_counter_add(&p->events, nr); } void fprop_fraction_percpu(struct fprop_global *p, @@ -253,11 +254,11 @@ void fprop_fraction_percpu(struct fprop_global *p, } /* - * Like __fprop_inc_percpu() except that event is counted only if the given + * Like __fprop_add_percpu() except that event is counted only if the given * type has fraction smaller than @max_frac/FPROP_FRAC_BASE */ -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)) { unsigned long numerator, denominator; @@ -268,5 +269,5 @@ void __fprop_inc_percpu_max(struct fprop_global *p, return; } - __fprop_inc_percpu(p, pl); + __fprop_add_percpu(p, pl, nr); } diff --git a/mm/page-writeback.c b/mm/page-writeback.c index 55d6ec0e8d93..0cd58c7c69f2 100644 --- a/mm/page-writeback.c +++ b/mm/page-writeback.c @@ -572,8 +572,8 @@ static void wb_domain_writeout_inc(struct wb_domain *dom, struct fprop_local_percpu *completions, unsigned int max_prop_frac) { - __fprop_inc_percpu_max(&dom->completions, completions, - max_prop_frac); + __fprop_add_percpu_max(&dom->completions, completions, + max_prop_frac, 1); /* First event after period switching was turned off? */ if (unlikely(!dom->period_time)) { /*
When batching events (such as writing back N pages in a single I/O), it is better to do one flex_proportion operation instead of N. There is only one caller of __fprop_inc_percpu_max(), and it's the one we're going to change in the next patch, so rename it instead of adding a compatibility wrapper. Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> --- include/linux/flex_proportions.h | 9 +++++---- lib/flex_proportions.c | 15 ++++++++------- mm/page-writeback.c | 4 ++-- 3 files changed, 15 insertions(+), 13 deletions(-)