[PATCH 0/3] Allow for multiple pages in a single writeback event

[PATCH 0/3] Allow for multiple pages in a single writeback event

[Matthew Wilcox (Oracle)]

This is prep work for THP or Folios or whatever we end up doing --
we are going to end up somehow batching page writeback completions.
Nothing here makes that happen; we simply pass '1' from all our current
callers, but eventually we'll pass a number greater than 1.

Matthew Wilcox (Oracle) (3):
  mm/writeback: Rename __add_wb_stat to wb_stat_mod
  flex_proportions: Allow N events instead of 1
  mm/writeback: Change __wb_writeout_inc to __wb_writeout_add

 include/linux/backing-dev.h      |  6 +++---
 include/linux/flex_proportions.h |  9 +++++----
 lib/flex_proportions.c           | 15 ++++++++-------
 mm/page-writeback.c              | 24 ++++++++++++------------
 4 files changed, 28 insertions(+), 26 deletions(-)

-- 
2.30.2

[PATCH 1/3] mm/writeback: Rename __add_wb_stat to wb_stat_mod

[Matthew Wilcox (Oracle)]

Make this look like the newly renamed vmstat functions.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 include/linux/backing-dev.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/linux/backing-dev.h b/include/linux/backing-dev.h
index 44df4fcef65c..a852876bb6e2 100644
--- a/include/linux/backing-dev.h
+++ b/include/linux/backing-dev.h
@@ -64,7 +64,7 @@ static inline bool bdi_has_dirty_io(struct backing_dev_info *bdi)
 	return atomic_long_read(&bdi->tot_write_bandwidth);
 }
 
-static inline void __add_wb_stat(struct bdi_writeback *wb,
+static inline void wb_stat_mod(struct bdi_writeback *wb,
 				 enum wb_stat_item item, s64 amount)
 {
 	percpu_counter_add_batch(&wb->stat[item], amount, WB_STAT_BATCH);
@@ -72,12 +72,12 @@ static inline void __add_wb_stat(struct bdi_writeback *wb,
 
 static inline void inc_wb_stat(struct bdi_writeback *wb, enum wb_stat_item item)
 {
-	__add_wb_stat(wb, item, 1);
+	wb_stat_mod(wb, item, 1);
 }
 
 static inline void dec_wb_stat(struct bdi_writeback *wb, enum wb_stat_item item)
 {
-	__add_wb_stat(wb, item, -1);
+	wb_stat_mod(wb, item, -1);
 }
 
 static inline s64 wb_stat(struct bdi_writeback *wb, enum wb_stat_item item)
-- 
2.30.2

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

[Matthew Wilcox (Oracle)]

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(-)

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)) {
 		/*
-- 
2.30.2

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

[Matthew Wilcox]

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.

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

[Jan Kara]

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

[PATCH 3/3] mm/writeback: Change __wb_writeout_inc to __wb_writeout_add

[Matthew Wilcox (Oracle)]

Allow for accounting N pages at once instead of one page at a time.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 mm/page-writeback.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index 0cd58c7c69f2..ba4b57e260ba 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -568,12 +568,12 @@ static unsigned long wp_next_time(unsigned long cur_time)
 	return cur_time;
 }
 
-static void wb_domain_writeout_inc(struct wb_domain *dom,
+static void wb_domain_writeout_add(struct wb_domain *dom,
 				   struct fprop_local_percpu *completions,
-				   unsigned int max_prop_frac)
+				   unsigned int max_prop_frac, long nr)
 {
 	__fprop_add_percpu_max(&dom->completions, completions,
-			       max_prop_frac, 1);
+			       max_prop_frac, nr);
 	/* First event after period switching was turned off? */
 	if (unlikely(!dom->period_time)) {
 		/*
@@ -591,18 +591,18 @@ static void wb_domain_writeout_inc(struct wb_domain *dom,
  * Increment @wb's writeout completion count and the global writeout
  * completion count. Called from test_clear_page_writeback().
  */
-static inline void __wb_writeout_inc(struct bdi_writeback *wb)
+static inline void __wb_writeout_add(struct bdi_writeback *wb, long nr)
 {
 	struct wb_domain *cgdom;
 
-	inc_wb_stat(wb, WB_WRITTEN);
-	wb_domain_writeout_inc(&global_wb_domain, &wb->completions,
-			       wb->bdi->max_prop_frac);
+	wb_stat_mod(wb, WB_WRITTEN, nr);
+	wb_domain_writeout_add(&global_wb_domain, &wb->completions,
+			       wb->bdi->max_prop_frac, nr);
 
 	cgdom = mem_cgroup_wb_domain(wb);
 	if (cgdom)
-		wb_domain_writeout_inc(cgdom, wb_memcg_completions(wb),
-				       wb->bdi->max_prop_frac);
+		wb_domain_writeout_add(cgdom, wb_memcg_completions(wb),
+				       wb->bdi->max_prop_frac, nr);
 }
 
 void wb_writeout_inc(struct bdi_writeback *wb)
@@ -610,7 +610,7 @@ void wb_writeout_inc(struct bdi_writeback *wb)
 	unsigned long flags;
 
 	local_irq_save(flags);
-	__wb_writeout_inc(wb);
+	__wb_writeout_add(wb, 1);
 	local_irq_restore(flags);
 }
 EXPORT_SYMBOL_GPL(wb_writeout_inc);
@@ -2739,7 +2739,7 @@ int test_clear_page_writeback(struct page *page)
 				struct bdi_writeback *wb = inode_to_wb(inode);
 
 				dec_wb_stat(wb, WB_WRITEBACK);
-				__wb_writeout_inc(wb);
+				__wb_writeout_add(wb, 1);
 			}
 		}
 
-- 
2.30.2

Re: [PATCH 0/3] Allow for multiple pages in a single writeback event

[Jan Kara]

On Mon 22-03-21 20:11:13, Matthew Wilcox (Oracle) wrote:
> This is prep work for THP or Folios or whatever we end up doing --
> we are going to end up somehow batching page writeback completions.
> Nothing here makes that happen; we simply pass '1' from all our current
> callers, but eventually we'll pass a number greater than 1.

The patches look good to me besides that one problem in the writeback
proportions code. After that is fixed, I can either take the patches
through my tree or we can route them through Andrew with my Reviewed-by.

								Honza

> 
> Matthew Wilcox (Oracle) (3):
>   mm/writeback: Rename __add_wb_stat to wb_stat_mod
>   flex_proportions: Allow N events instead of 1
>   mm/writeback: Change __wb_writeout_inc to __wb_writeout_add
> 
>  include/linux/backing-dev.h      |  6 +++---
>  include/linux/flex_proportions.h |  9 +++++----
>  lib/flex_proportions.c           | 15 ++++++++-------
>  mm/page-writeback.c              | 24 ++++++++++++------------
>  4 files changed, 28 insertions(+), 26 deletions(-)
> 
> -- 
> 2.30.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR