[patch] mm: unlockless reclaim

[patch] mm: unlockless reclaim

[Nick Piggin]

unlock_page is pretty expensive. Even after my patches to optimise the
memory order and away the waitqueue hit for uncontended pages, it is
still a locked operation, which may be anywhere up to hundreds of cycles
on some CPUs.

When we reclaim a page, we don't need to "unlock" it as such, because
we know there will be no contention (if there was, it would be a bug
because the page is just about to get freed).

Signed-off-by: Nick Piggin <npiggin@suse.de>

Index: linux-2.6/include/linux/page-flags.h
===================================================================
--- linux-2.6.orig/include/linux/page-flags.h
+++ linux-2.6/include/linux/page-flags.h
@@ -115,6 +115,8 @@
 		test_and_set_bit(PG_locked, &(page)->flags)
 #define ClearPageLocked(page)		\
 		clear_bit(PG_locked, &(page)->flags)
+#define __ClearPageLocked(page)		\
+		__clear_bit(PG_locked, &(page)->flags)
 #define TestClearPageLocked(page)	\
 		test_and_clear_bit(PG_locked, &(page)->flags)
 
Index: linux-2.6/mm/vmscan.c
===================================================================
--- linux-2.6.orig/mm/vmscan.c
+++ linux-2.6/mm/vmscan.c
@@ -576,7 +576,7 @@ static unsigned long shrink_page_list(st
 			goto keep_locked;
 
 free_it:
-		unlock_page(page);
+		__ClearPageLocked(page);
 		nr_reclaimed++;
 		if (!pagevec_add(&freed_pvec, page))
 			__pagevec_release_nonlru(&freed_pvec);

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch] mm: unlockless reclaim

[Andrew Morton]

On Thu, 12 Jul 2007 06:11:15 +0200 Nick Piggin <npiggin@suse.de> wrote:

> unlock_page is pretty expensive. Even after my patches to optimise the
> memory order and away the waitqueue hit for uncontended pages, it is
> still a locked operation, which may be anywhere up to hundreds of cycles
> on some CPUs.
> 
> When we reclaim a page, we don't need to "unlock" it as such, because
> we know there will be no contention (if there was, it would be a bug
> because the page is just about to get freed).
> 
> Signed-off-by: Nick Piggin <npiggin@suse.de>
> 
> Index: linux-2.6/include/linux/page-flags.h
> ===================================================================
> --- linux-2.6.orig/include/linux/page-flags.h
> +++ linux-2.6/include/linux/page-flags.h
> @@ -115,6 +115,8 @@
>  		test_and_set_bit(PG_locked, &(page)->flags)
>  #define ClearPageLocked(page)		\
>  		clear_bit(PG_locked, &(page)->flags)
> +#define __ClearPageLocked(page)		\
> +		__clear_bit(PG_locked, &(page)->flags)
>  #define TestClearPageLocked(page)	\
>  		test_and_clear_bit(PG_locked, &(page)->flags)
>  
> Index: linux-2.6/mm/vmscan.c
> ===================================================================
> --- linux-2.6.orig/mm/vmscan.c
> +++ linux-2.6/mm/vmscan.c
> @@ -576,7 +576,7 @@ static unsigned long shrink_page_list(st
>  			goto keep_locked;
>  
>  free_it:
> -		unlock_page(page);
> +		__ClearPageLocked(page);
>  		nr_reclaimed++;
>  		if (!pagevec_add(&freed_pvec, page))
>  			__pagevec_release_nonlru(&freed_pvec);

mutter.

So why does __pagevec_release_nonlru() check the page refcount?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch] mm: unlockless reclaim

[Nick Piggin]

On Thu, Jul 12, 2007 at 12:43:39AM -0700, Andrew Morton wrote:
> On Thu, 12 Jul 2007 06:11:15 +0200 Nick Piggin <npiggin@suse.de> wrote:
> 
> > unlock_page is pretty expensive. Even after my patches to optimise the
> > memory order and away the waitqueue hit for uncontended pages, it is
> > still a locked operation, which may be anywhere up to hundreds of cycles
> > on some CPUs.
> > 
> > When we reclaim a page, we don't need to "unlock" it as such, because
> > we know there will be no contention (if there was, it would be a bug
> > because the page is just about to get freed).
> > 
> > Signed-off-by: Nick Piggin <npiggin@suse.de>
> > 
> > Index: linux-2.6/include/linux/page-flags.h
> > ===================================================================
> > --- linux-2.6.orig/include/linux/page-flags.h
> > +++ linux-2.6/include/linux/page-flags.h
> > @@ -115,6 +115,8 @@
> >  		test_and_set_bit(PG_locked, &(page)->flags)
> >  #define ClearPageLocked(page)		\
> >  		clear_bit(PG_locked, &(page)->flags)
> > +#define __ClearPageLocked(page)		\
> > +		__clear_bit(PG_locked, &(page)->flags)
> >  #define TestClearPageLocked(page)	\
> >  		test_and_clear_bit(PG_locked, &(page)->flags)
> >  
> > Index: linux-2.6/mm/vmscan.c
> > ===================================================================
> > --- linux-2.6.orig/mm/vmscan.c
> > +++ linux-2.6/mm/vmscan.c
> > @@ -576,7 +576,7 @@ static unsigned long shrink_page_list(st
> >  			goto keep_locked;
> >  
> >  free_it:
> > -		unlock_page(page);
> > +		__ClearPageLocked(page);
> >  		nr_reclaimed++;
> >  		if (!pagevec_add(&freed_pvec, page))
> >  			__pagevec_release_nonlru(&freed_pvec);
> 
> mutter.
> 
> So why does __pagevec_release_nonlru() check the page refcount?

It doesn't, although it will have to return the count to zero of course.

I don't want to submit that because the lockless pagecache always needs
the refcount to be checked :) And which I am actually going to submit to
you after you chuck out a few patches.

But unlock_page is really murderous on my powerpc (with all the
unlock-speeup patches, dd if=/dev/zero of=/dev/null of a huge sparse file
goes up by 10% throughput on the G5!!).

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch] mm: unlockless reclaim

[Andrew Morton]

On Thu, 12 Jul 2007 09:55:32 +0200 Nick Piggin <npiggin@suse.de> wrote:

> On Thu, Jul 12, 2007 at 12:43:39AM -0700, Andrew Morton wrote:
> > On Thu, 12 Jul 2007 06:11:15 +0200 Nick Piggin <npiggin@suse.de> wrote:
> > 
> > > unlock_page is pretty expensive. Even after my patches to optimise the
> > > memory order and away the waitqueue hit for uncontended pages, it is
> > > still a locked operation, which may be anywhere up to hundreds of cycles
> > > on some CPUs.
> > > 
> > > When we reclaim a page, we don't need to "unlock" it as such, because
> > > we know there will be no contention (if there was, it would be a bug
> > > because the page is just about to get freed).
> > > 
> > > Signed-off-by: Nick Piggin <npiggin@suse.de>
> > > 
> > > Index: linux-2.6/include/linux/page-flags.h
> > > ===================================================================
> > > --- linux-2.6.orig/include/linux/page-flags.h
> > > +++ linux-2.6/include/linux/page-flags.h
> > > @@ -115,6 +115,8 @@
> > >  		test_and_set_bit(PG_locked, &(page)->flags)
> > >  #define ClearPageLocked(page)		\
> > >  		clear_bit(PG_locked, &(page)->flags)
> > > +#define __ClearPageLocked(page)		\
> > > +		__clear_bit(PG_locked, &(page)->flags)
> > >  #define TestClearPageLocked(page)	\
> > >  		test_and_clear_bit(PG_locked, &(page)->flags)
> > >  
> > > Index: linux-2.6/mm/vmscan.c
> > > ===================================================================
> > > --- linux-2.6.orig/mm/vmscan.c
> > > +++ linux-2.6/mm/vmscan.c
> > > @@ -576,7 +576,7 @@ static unsigned long shrink_page_list(st
> > >  			goto keep_locked;
> > >  
> > >  free_it:
> > > -		unlock_page(page);
> > > +		__ClearPageLocked(page);
> > >  		nr_reclaimed++;
> > >  		if (!pagevec_add(&freed_pvec, page))
> > >  			__pagevec_release_nonlru(&freed_pvec);
> > 
> > mutter.
> > 
> > So why does __pagevec_release_nonlru() check the page refcount?
> 
> It doesn't

yes it does

> although it will have to return the count to zero of course.
> 
> I don't want to submit that because the lockless pagecache always needs
> the refcount to be checked :) And which I am actually going to submit to
> you after you chuck out a few patches.
> 
> But unlock_page is really murderous on my powerpc (with all the
> unlock-speeup patches, dd if=/dev/zero of=/dev/null of a huge sparse file
> goes up by 10% throughput on the G5!!).

well this change won't help that much.

mutter.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch] mm: unlockless reclaim

[Nick Piggin]

On Thu, Jul 12, 2007 at 01:00:07AM -0700, Andrew Morton wrote:
> On Thu, 12 Jul 2007 09:55:32 +0200 Nick Piggin <npiggin@suse.de> wrote:
> 
> > > 
> > > mutter.
> > > 
> > > So why does __pagevec_release_nonlru() check the page refcount?
> > 
> > It doesn't
> 
> yes it does

That was in answer to your question: I mean: it doesn't need to.


> > although it will have to return the count to zero of course.
> > 
> > I don't want to submit that because the lockless pagecache always needs
> > the refcount to be checked :) And which I am actually going to submit to
> > you after you chuck out a few patches.
> > 
> > But unlock_page is really murderous on my powerpc (with all the
> > unlock-speeup patches, dd if=/dev/zero of=/dev/null of a huge sparse file
> > goes up by 10% throughput on the G5!!).
> 
> well this change won't help that much.

Oh, well the dd includes reclaim and so it ends up doing 2 locks for
each page (1 to reading, 1 to reclaim). So this alone supposedly should
help by 5% :)


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch] mm: unlockless reclaim

[Andrew Morton]

On Thu, 12 Jul 2007 06:11:15 +0200
Nick Piggin <npiggin@suse.de> wrote:

> unlock_page is pretty expensive. Even after my patches to optimise the
> memory order and away the waitqueue hit for uncontended pages, it is
> still a locked operation, which may be anywhere up to hundreds of cycles
> on some CPUs.
> 
> When we reclaim a page, we don't need to "unlock" it as such, because
> we know there will be no contention (if there was, it would be a bug
> because the page is just about to get freed).
> 
> Signed-off-by: Nick Piggin <npiggin@suse.de>
> 
> Index: linux-2.6/include/linux/page-flags.h
> ===================================================================
> --- linux-2.6.orig/include/linux/page-flags.h
> +++ linux-2.6/include/linux/page-flags.h
> @@ -115,6 +115,8 @@
>  		test_and_set_bit(PG_locked, &(page)->flags)
>  #define ClearPageLocked(page)		\
>  		clear_bit(PG_locked, &(page)->flags)
> +#define __ClearPageLocked(page)		\
> +		__clear_bit(PG_locked, &(page)->flags)
>  #define TestClearPageLocked(page)	\
>  		test_and_clear_bit(PG_locked, &(page)->flags)
>  
> Index: linux-2.6/mm/vmscan.c
> ===================================================================
> --- linux-2.6.orig/mm/vmscan.c
> +++ linux-2.6/mm/vmscan.c
> @@ -576,7 +576,7 @@ static unsigned long shrink_page_list(st
>  			goto keep_locked;
>  
>  free_it:
> -		unlock_page(page);
> +		__ClearPageLocked(page);
>  		nr_reclaimed++;
>  		if (!pagevec_add(&freed_pvec, page))
>  			__pagevec_release_nonlru(&freed_pvec);

I really hate this patch :(  For the usual reasons.

I'd have thought that such a terrifying point-cannon-at-someone-else's-foot
hack would at least merit a comment explaining (fully) to the reader why it
is a safe thing to do at this site.

And explaining to them why __pagevec_release_nonlru() immediately
contradicts the assumption which this code is making.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch] mm: unlockless reclaim

[Nick Piggin]

On Thu, Jul 12, 2007 at 12:20:39PM -0700, Andrew Morton wrote:
> On Thu, 12 Jul 2007 06:11:15 +0200
> Nick Piggin <npiggin@suse.de> wrote:
> 
> > unlock_page is pretty expensive. Even after my patches to optimise the
> > memory order and away the waitqueue hit for uncontended pages, it is
> > still a locked operation, which may be anywhere up to hundreds of cycles
> > on some CPUs.
> > 
> > When we reclaim a page, we don't need to "unlock" it as such, because
> > we know there will be no contention (if there was, it would be a bug
> > because the page is just about to get freed).
> > 
> > Signed-off-by: Nick Piggin <npiggin@suse.de>
> > 
> > Index: linux-2.6/include/linux/page-flags.h
> > ===================================================================
> > --- linux-2.6.orig/include/linux/page-flags.h
> > +++ linux-2.6/include/linux/page-flags.h
> > @@ -115,6 +115,8 @@
> >  		test_and_set_bit(PG_locked, &(page)->flags)
> >  #define ClearPageLocked(page)		\
> >  		clear_bit(PG_locked, &(page)->flags)
> > +#define __ClearPageLocked(page)		\
> > +		__clear_bit(PG_locked, &(page)->flags)
> >  #define TestClearPageLocked(page)	\
> >  		test_and_clear_bit(PG_locked, &(page)->flags)
> >  
> > Index: linux-2.6/mm/vmscan.c
> > ===================================================================
> > --- linux-2.6.orig/mm/vmscan.c
> > +++ linux-2.6/mm/vmscan.c
> > @@ -576,7 +576,7 @@ static unsigned long shrink_page_list(st
> >  			goto keep_locked;
> >  
> >  free_it:
> > -		unlock_page(page);
> > +		__ClearPageLocked(page);
> >  		nr_reclaimed++;
> >  		if (!pagevec_add(&freed_pvec, page))
> >  			__pagevec_release_nonlru(&freed_pvec);
> 
> I really hate this patch :(  For the usual reasons.
> 
> I'd have thought that such a terrifying point-cannon-at-someone-else's-foot
> hack would at least merit a comment explaining (fully) to the reader why it
> is a safe thing to do at this site.

OK, comments are fair enough although we already do similar things
to clear other flags here and in the page allocator path itself so
I thought it was already deemed kosher. I'll add some in a next
iteration.

 
> And explaining to them why __pagevec_release_nonlru() immediately
> contradicts the assumption which this code is making.

Again fair enough. I'll send it sometime when you've got less mm
stuff in your tree I guess.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch] mm: unlockless reclaim

[KOSAKI Motohiro]

> On Tue, Aug 19, 2008 at 02:09:07PM +0900, KOSAKI Motohiro wrote:
> > > -		unlock_page(page);
> > > +		/*
> > > +		 * At this point, we have no other references and there is
> > > +		 * no way to pick any more up (removed from LRU, removed
> > > +		 * from pagecache). Can use non-atomic bitops now (and
> > > +		 * we obviously don't have to worry about waking up a process
> > > +		 * waiting on the page lock, because there are no references.
> > > +		 */
> > > +		__clear_page_locked(page);
> > >  free_it:
> > >  		nr_reclaimed++;
> > >  		if (!pagevec_add(&freed_pvec, page)) {
> > > 
> > 
> > To insert VM_BUG_ON(page_count(page) != 1) is better?
> > Otherthing, looks good to me.
> 
> That is a very good idea, except that now with lockless pagecache, we're
> not so well placed to do this type of check. Actually at this point,
> the refcount will be 0 because of page_freeze_refs, and then if anybody
> else tries to do a get_page, they should hit the VM_BUG_ON in get_page.

Ah, you are right.
Sorry, I often forgot it ;)



--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch] mm: unlockless reclaim

[Nick Piggin]

On Tue, Aug 19, 2008 at 02:09:07PM +0900, KOSAKI Motohiro wrote:
> > -		unlock_page(page);
> > +		/*
> > +		 * At this point, we have no other references and there is
> > +		 * no way to pick any more up (removed from LRU, removed
> > +		 * from pagecache). Can use non-atomic bitops now (and
> > +		 * we obviously don't have to worry about waking up a process
> > +		 * waiting on the page lock, because there are no references.
> > +		 */
> > +		__clear_page_locked(page);
> >  free_it:
> >  		nr_reclaimed++;
> >  		if (!pagevec_add(&freed_pvec, page)) {
> > 
> 
> To insert VM_BUG_ON(page_count(page) != 1) is better?
> Otherthing, looks good to me.

That is a very good idea, except that now with lockless pagecache, we're
not so well placed to do this type of check. Actually at this point,
the refcount will be 0 because of page_freeze_refs, and then if anybody
else tries to do a get_page, they should hit the VM_BUG_ON in get_page.

 
> Reviewed-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>

Thanks for looking at it.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [patch] mm: unlockless reclaim

[KOSAKI Motohiro]

> -		unlock_page(page);
> +		/*
> +		 * At this point, we have no other references and there is
> +		 * no way to pick any more up (removed from LRU, removed
> +		 * from pagecache). Can use non-atomic bitops now (and
> +		 * we obviously don't have to worry about waking up a process
> +		 * waiting on the page lock, because there are no references.
> +		 */
> +		__clear_page_locked(page);
>  free_it:
>  		nr_reclaimed++;
>  		if (!pagevec_add(&freed_pvec, page)) {
> 

To insert VM_BUG_ON(page_count(page) != 1) is better?
Otherthing, looks good to me.

Reviewed-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>



--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[patch] mm: unlockless reclaim

[Nick Piggin]

unlock_page is fairly expensive. It can be avoided in page reclaim success
path. By definition if we have any other references to the page it would
be a bug anyway.

Signed-off-by: Nick Piggin <npiggin@suse.de>
---
 mm/vmscan.c |    9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

Index: linux-2.6/mm/vmscan.c
===================================================================
--- linux-2.6.orig/mm/vmscan.c
+++ linux-2.6/mm/vmscan.c
@@ -637,7 +637,14 @@ static unsigned long shrink_page_list(st
 		if (!mapping || !__remove_mapping(mapping, page))
 			goto keep_locked;
 
-		unlock_page(page);
+		/*
+		 * At this point, we have no other references and there is
+		 * no way to pick any more up (removed from LRU, removed
+		 * from pagecache). Can use non-atomic bitops now (and
+		 * we obviously don't have to worry about waking up a process
+		 * waiting on the page lock, because there are no references.
+		 */
+		__clear_page_locked(page);
 free_it:
 		nr_reclaimed++;
 		if (!pagevec_add(&freed_pvec, page)) {

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[patch] mm: unlockless reclaim

[Nick Piggin]

Combined with the previous and subsequent patches, throughput of pages through
the pagecache on an Opteron system here goes up by anywhere from 50% to 500%,
depending on the number of files and threads involved.
--

unlock_page is fairly expensive. It can be avoided in page reclaim.

Signed-off-by: Nick Piggin <npiggin@suse.de>
---
Index: linux-2.6/mm/vmscan.c
===================================================================
--- linux-2.6.orig/mm/vmscan.c
+++ linux-2.6/mm/vmscan.c
@@ -589,7 +589,14 @@ static unsigned long shrink_page_list(st
 			goto keep_locked;
 
 free_it:
-		unlock_page(page);
+		/*
+		 * At this point, we have no other references and there is
+		 * no way to pick any more up (removed from LRU, removed
+		 * from pagecache). Can use non-atomic bitops now (and
+		 * we obviously don't have to worry about waking up a process
+		 * waiting on the page lock, because there are no references.
+		 */
+		__clear_page_locked(page);
 		nr_reclaimed++;
 		if (!pagevec_add(&freed_pvec, page))
 			__pagevec_release_nonlru(&freed_pvec);

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>