[PATCH 0/2] Increase the size of LRU pagevecs

[PATCH 0/2] Increase the size of LRU pagevecs

[Matthew Wilcox (Oracle)]

This attempts to address some of the criticisms of Tim's original
patch back in June:

https://lore.kernel.org/linux-mm/d1cc9f12a8ad6c2a52cb600d93b06b064f2bbc57.1593205965.git.tim.c.chen@linux.intel.com/

I don't love how messy this is.  Better suggestions (dynamic allocation of
pagevecs?) welcome.  It'd be even better if we could measure contention
and resize the LRUvecs on demand.

I do intend to add pagevec_alloc() and pagevec_free(), but that's to
solve a different problem.

Matthew Wilcox (Oracle) (2):
  pagevec: Allow pagevecs to be different sizes
  pagevec: Increase the size of LRU pagevecs

 include/linux/pagevec.h | 25 +++++++++++++++++++++----
 mm/swap.c               | 23 +++++++++++++++++------
 2 files changed, 38 insertions(+), 10 deletions(-)

-- 
2.28.0

[PATCH 1/2] pagevec: Allow pagevecs to be different sizes

[Matthew Wilcox (Oracle)]

Declaring a pagevec continues to create a pagevec which is the same size,
but functions which manipulate pagevecs no longer rely on this.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 include/linux/pagevec.h | 20 ++++++++++++++++----
 mm/swap.c               |  8 ++++++++
 2 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/include/linux/pagevec.h b/include/linux/pagevec.h
index 875a3f0d9dd2..ee5d3c4da8da 100644
--- a/include/linux/pagevec.h
+++ b/include/linux/pagevec.h
@@ -18,9 +18,15 @@ struct page;
 struct address_space;
 
 struct pagevec {
-	unsigned char nr;
-	bool percpu_pvec_drained;
-	struct page *pages[PAGEVEC_SIZE];
+	union {
+		struct {
+			unsigned char sz;
+			unsigned char nr;
+			bool percpu_pvec_drained;
+			struct page *pages[PAGEVEC_SIZE];
+		};
+		void *__p[PAGEVEC_SIZE + 1];
+	};
 };
 
 void __pagevec_release(struct pagevec *pvec);
@@ -41,6 +47,7 @@ static inline unsigned pagevec_lookup_tag(struct pagevec *pvec,
 
 static inline void pagevec_init(struct pagevec *pvec)
 {
+	pvec->sz = PAGEVEC_SIZE;
 	pvec->nr = 0;
 	pvec->percpu_pvec_drained = false;
 }
@@ -50,6 +57,11 @@ static inline void pagevec_reinit(struct pagevec *pvec)
 	pvec->nr = 0;
 }
 
+static inline unsigned pagevec_size(struct pagevec *pvec)
+{
+	return pvec->sz;
+}
+
 static inline unsigned pagevec_count(struct pagevec *pvec)
 {
 	return pvec->nr;
@@ -57,7 +69,7 @@ static inline unsigned pagevec_count(struct pagevec *pvec)
 
 static inline unsigned pagevec_space(struct pagevec *pvec)
 {
-	return PAGEVEC_SIZE - pvec->nr;
+	return pvec->sz - pvec->nr;
 }
 
 /*
diff --git a/mm/swap.c b/mm/swap.c
index 2ee3522a7170..d093fb30f038 100644
--- a/mm/swap.c
+++ b/mm/swap.c
@@ -52,6 +52,7 @@ struct lru_rotate {
 };
 static DEFINE_PER_CPU(struct lru_rotate, lru_rotate) = {
 	.lock = INIT_LOCAL_LOCK(lock),
+	.pvec.sz = PAGEVEC_SIZE,
 };
 
 /*
@@ -70,6 +71,13 @@ struct lru_pvecs {
 };
 static DEFINE_PER_CPU(struct lru_pvecs, lru_pvecs) = {
 	.lock = INIT_LOCAL_LOCK(lock),
+	.lru_add.sz = PAGEVEC_SIZE,
+	.lru_deactivate_file.sz = PAGEVEC_SIZE,
+	.lru_deactivate.sz = PAGEVEC_SIZE,
+	.lru_lazyfree.sz = PAGEVEC_SIZE,
+#ifdef CONFIG_SMP
+	.activate_page.sz = PAGEVEC_SIZE,
+#endif
 };
 
 /*
-- 
2.28.0

Re: [PATCH 1/2] pagevec: Allow pagevecs to be different sizes

[Tim Chen]

>  
> +static inline unsigned pagevec_size(struct pagevec *pvec)
> +{
> +	return pvec->sz;
> +}
> +
>  static inline unsigned pagevec_count(struct pagevec *pvec)
>  {

Willy,

I was expecting pagevec_size to be used in pagevec_lookup_range*
where we hardcoded the pagevec's top range to look for pages
as PAGEVEC_SIZE.  You define pagevec_size but seems like it is not used.
Or am I missing something?  

Thanks.

Tim

Re: [PATCH 1/2] pagevec: Allow pagevecs to be different sizes

[Matthew Wilcox]

On Fri, Nov 06, 2020 at 09:17:43AM -0800, Tim Chen wrote:
> 
> >  
> > +static inline unsigned pagevec_size(struct pagevec *pvec)
> > +{
> > +	return pvec->sz;
> > +}
> > +
> >  static inline unsigned pagevec_count(struct pagevec *pvec)
> >  {
> 
> Willy,
> 
> I was expecting pagevec_size to be used in pagevec_lookup_range*
> where we hardcoded the pagevec's top range to look for pages
> as PAGEVEC_SIZE.  You define pagevec_size but seems like it is not used.
> Or am I missing something?  

I have a number of changes already in that area that are currently in-flight.

https://lore.kernel.org/linux-mm/20201026041408.25230-1-willy@infradead.org/

(although I should modify patch 9/12 to not use PAGEVEC_SIZE directly)

I have some patches that I haven't bothered sending yet since I have so
many other outstanding patches to do something similar to find_get_pages()
/ find_get_pages_range() / pagevec_lookup() / pagevec_lookup_range().


The trace you sent showed the problem being with the lru_add pagevec,
and that's never used with the pagevec_lookup_* APIs (afaik), so I didn't
see it as urgent to fix those.

Re: [PATCH 1/2] pagevec: Allow pagevecs to be different sizes

[Tim Chen]

On 11/6/20 11:12 AM, Matthew Wilcox wrote:
 
> 
> I have a number of changes already in that area that are currently in-flight.
> 
> https://lore.kernel.org/linux-mm/20201026041408.25230-1-willy@infradead.org/
> 
> (although I should modify patch 9/12 to not use PAGEVEC_SIZE directly)

Okay. I assume that you are using pagevec_size somewhere to replace those
hardcoded PAGEVEC_SIZE somewhere.

> 
> I have some patches that I haven't bothered sending yet since I have so
> many other outstanding patches to do something similar to find_get_pages()
> / find_get_pages_range() / pagevec_lookup() / pagevec_lookup_range().
> 
> 
> The trace you sent showed the problem being with the lru_add pagevec,
> and that's never used with the pagevec_lookup_* APIs (afaik), so I didn't
> see it as urgent to fix those.
> 

Okay.

Thanks for allowing the bigger pagevec size tweak in your patches.

Tim

Re: [PATCH 1/2] pagevec: Allow pagevecs to be different sizes

[Matthew Wilcox]

On Fri, Nov 06, 2020 at 02:18:40PM -0800, Tim Chen wrote:
> On 11/6/20 11:12 AM, Matthew Wilcox wrote:
> > I have a number of changes already in that area that are currently in-flight.
> > https://lore.kernel.org/linux-mm/20201026041408.25230-1-willy@infradead.org/
> > 
> > (although I should modify patch 9/12 to not use PAGEVEC_SIZE directly)
> 
> Okay. I assume that you are using pagevec_size somewhere to replace those
> hardcoded PAGEVEC_SIZE somewhere.

Ah.  I felt I should add it, as a counterpart to pagevec_count().
But most users don't need to ask what size the pagevec is; they just
add pages to it until it's full.  eg:

        pvec = this_cpu_ptr(&lru_pvecs.lru_add);
        if (!pagevec_add(pvec, page) || PageCompound(page))
                __pagevec_lru_add(pvec);

so as long as it's right in pagevec_add(), it'll be fine:

static inline unsigned pagevec_add(struct pagevec *pvec, struct page *page)
{
        pvec->pages[pvec->nr++] = page;
        return pagevec_space(pvec);
}

and I fix pagevec_space in patch 1:

        return pvec->sz - pvec->nr;

[PATCH 2/2] pagevec: Increase the size of LRU pagevecs

[Matthew Wilcox (Oracle)]

Tim Chen reports that increasing the size of LRU pagevecs is advantageous
with a workload he has:
https://lore.kernel.org/linux-mm/d1cc9f12a8ad6c2a52cb600d93b06b064f2bbc57.1593205965.git.tim.c.chen@linux.intel.com/

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
---
 include/linux/pagevec.h |  5 +++++
 mm/swap.c               | 27 +++++++++++++++------------
 2 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/include/linux/pagevec.h b/include/linux/pagevec.h
index ee5d3c4da8da..4dc45392d776 100644
--- a/include/linux/pagevec.h
+++ b/include/linux/pagevec.h
@@ -29,6 +29,11 @@ struct pagevec {
 	};
 };
 
+#define declare_pagevec(name, size) union {				\
+	struct pagevec name;						\
+	void *__p ##name [size + 1];					\
+}
+
 void __pagevec_release(struct pagevec *pvec);
 void __pagevec_lru_add(struct pagevec *pvec);
 void pagevec_remove_exceptionals(struct pagevec *pvec);
diff --git a/mm/swap.c b/mm/swap.c
index d093fb30f038..1e6f50b312ea 100644
--- a/mm/swap.c
+++ b/mm/swap.c
@@ -45,14 +45,17 @@
 /* How many pages do we try to swap or page in/out together? */
 int page_cluster;
 
+#define LRU_PAGEVEC_SIZE 63
+#define lru_pagevec(name)	declare_pagevec(name, LRU_PAGEVEC_SIZE)
+
 /* Protecting only lru_rotate.pvec which requires disabling interrupts */
 struct lru_rotate {
 	local_lock_t lock;
-	struct pagevec pvec;
+	lru_pagevec(pvec);
 };
 static DEFINE_PER_CPU(struct lru_rotate, lru_rotate) = {
 	.lock = INIT_LOCAL_LOCK(lock),
-	.pvec.sz = PAGEVEC_SIZE,
+	.pvec.sz = LRU_PAGEVEC_SIZE,
 };
 
 /*
@@ -61,22 +64,22 @@ static DEFINE_PER_CPU(struct lru_rotate, lru_rotate) = {
  */
 struct lru_pvecs {
 	local_lock_t lock;
-	struct pagevec lru_add;
-	struct pagevec lru_deactivate_file;
-	struct pagevec lru_deactivate;
-	struct pagevec lru_lazyfree;
+	lru_pagevec(lru_add);
+	lru_pagevec(lru_deactivate_file);
+	lru_pagevec(lru_deactivate);
+	lru_pagevec(lru_lazyfree);
 #ifdef CONFIG_SMP
-	struct pagevec activate_page;
+	lru_pagevec(activate_page);
 #endif
 };
 static DEFINE_PER_CPU(struct lru_pvecs, lru_pvecs) = {
 	.lock = INIT_LOCAL_LOCK(lock),
-	.lru_add.sz = PAGEVEC_SIZE,
-	.lru_deactivate_file.sz = PAGEVEC_SIZE,
-	.lru_deactivate.sz = PAGEVEC_SIZE,
-	.lru_lazyfree.sz = PAGEVEC_SIZE,
+	.lru_add.sz = LRU_PAGEVEC_SIZE,
+	.lru_deactivate_file.sz = LRU_PAGEVEC_SIZE,
+	.lru_deactivate.sz = LRU_PAGEVEC_SIZE,
+	.lru_lazyfree.sz = LRU_PAGEVEC_SIZE,
 #ifdef CONFIG_SMP
-	.activate_page.sz = PAGEVEC_SIZE,
+	.activate_page.sz = LRU_PAGEVEC_SIZE,
 #endif
 };
 
-- 
2.28.0

Re: [PATCH 0/2] Increase the size of LRU pagevecs

[Tim Chen]

On 11/5/20 9:26 AM, Matthew Wilcox (Oracle) wrote:
> This attempts to address some of the criticisms of Tim's original
> patch back in June:
> 
> https://lore.kernel.org/linux-mm/d1cc9f12a8ad6c2a52cb600d93b06b064f2bbc57.1593205965.git.tim.c.chen@linux.intel.com/
> 
> I don't love how messy this is.  

I actually attempted a similar patch to allocate different sizes
for pagevec.  But it is so ugly that
I didn't try to post it.  This version looks much better than mine.

Tim

> Better suggestions (dynamic allocation of
> pagevecs?) welcome.  It'd be even better if we could measure contention
> and resize the LRUvecs on demand.
> 
> I do intend to add pagevec_alloc() and pagevec_free(), but that's to
> solve a different problem.
> 
> Matthew Wilcox (Oracle) (2):
>   pagevec: Allow pagevecs to be different sizes
>   pagevec: Increase the size of LRU pagevecs
> 
>  include/linux/pagevec.h | 25 +++++++++++++++++++++----
>  mm/swap.c               | 23 +++++++++++++++++------
>  2 files changed, 38 insertions(+), 10 deletions(-)
>