[patch] shrink_mmap() 2.3.99-pre6-3 (take 3)

[patch] shrink_mmap() 2.3.99-pre6-3 (take 3)

[Rik van Riel]

Hi,

here's a 3rd version of my patch to shrink_mmap(). This
version fixes most of the performance problems with
shrink_mmap() .. to make performance better big changes
will be needed. It also doesn't eliminate a possible race
condition (afaik Ben is working on that one) in shrink_mmap().

The patch does the following:
- remove possible race condition from truncate_inode_pages()
- don't age mapped pages, save the referenced bit for later
- put pages in the right place on the LRU queue
   (thanks for the .prev hint, Andrea ;))

It has been tested under a fairly wide variety of (over)loaded
situations and performance has been good.

regards,

Rik
--
The Internet is not a network of computers. It is a network
of people. That is its real strength.

Wanna talk about the kernel?  irc.openprojects.net / #kernelnewbies
http://www.conectiva.com/		http://www.surriel.com/



--- linux-2.3.99-pre6-3/mm/filemap.c.orig	Mon Apr 17 12:21:46 2000
+++ linux-2.3.99-pre6-3/mm/filemap.c	Wed Apr 19 14:17:27 2000
@@ -149,11 +149,16 @@
 
 		/* page wholly truncated - free it */
 		if (offset >= start) {
+			if (TryLockPage(page)) {
+				spin_unlock(&pagecache_lock);
+				get_page(page);
+				wait_on_page(page);
+				put_page(page);
+				goto repeat;
+			}
 			get_page(page);
 			spin_unlock(&pagecache_lock);
 
-			lock_page(page);
-
 			if (!page->buffers || block_flushpage(page, 0))
 				lru_cache_del(page);
 
@@ -191,11 +196,13 @@
 			continue;
 
 		/* partial truncate, clear end of page */
+		if (TryLockPage(page)) {
+			spin_unlock(&pagecache_lock);
+			goto repeat;
+		}
 		get_page(page);
 		spin_unlock(&pagecache_lock);
 
-		lock_page(page);
-
 		memclear_highpage_flush(page, partial, PAGE_CACHE_SIZE-partial);
 		if (page->buffers)
 			block_flushpage(page, partial);
@@ -208,6 +215,9 @@
 		 */
 		UnlockPage(page);
 		page_cache_release(page);
+		get_page(page);
+		wait_on_page(page);
+		put_page(page);
 		goto repeat;
 	}
 	spin_unlock(&pagecache_lock);
@@ -233,7 +243,17 @@
 		page = list_entry(page_lru, struct page, lru);
 		list_del(page_lru);
 
-		dispose = &zone->lru_cache;
+		/* What?! A page is in the LRU queue of another zone?! */
+		if (!memclass(page->zone, zone))
+			BUG();
+
+		dispose = &young;
+		/* the page is in use, we can't free it now */
+		if (!page->buffers && page_count(page) > 1)
+			goto dispose_continue;
+
+		count--;
+
 		if (test_and_clear_bit(PG_referenced, &page->flags))
 			/* Roll the page at the top of the lru list,
 			 * we could also be more aggressive putting
@@ -243,17 +263,6 @@
 			goto dispose_continue;
 
 		dispose = &old;
-		/* don't account passes over not DMA pages */
-		if (zone && (!memclass(page->zone, zone)))
-			goto dispose_continue;
-
-		count--;
-
-		dispose = &young;
-
-		/* avoid unscalable SMP locking */
-		if (!page->buffers && page_count(page) > 1)
-			goto dispose_continue;
 
 		if (TryLockPage(page))
 			goto dispose_continue;

--
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.eu.org/Linux-MM/

Re: [patch] shrink_mmap() 2.3.99-pre6-3 (take 3)

[Andrea Arcangeli]

On Wed, 19 Apr 2000, Rik van Riel wrote:

>will be needed. It also doesn't eliminate a possible race
>condition (afaik Ben is working on that one) in shrink_mmap().

Which shrink_mmap race condition?

>The patch does the following:
>- remove possible race condition from truncate_inode_pages()

Which truncate_inode_pages race condition? Please provide a stack trace,
it shouldn't take too time for you if you have the race condition in mind.

Thanks.

Andrea

--
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.eu.org/Linux-MM/

Re: [patch] shrink_mmap() 2.3.99-pre6-3 (take 3)

[Rik van Riel]

On Thu, 20 Apr 2000, Andrea Arcangeli wrote:
> On Wed, 19 Apr 2000, Rik van Riel wrote:
> 
> >will be needed. It also doesn't eliminate a possible race
> >condition (afaik Ben is working on that one) in shrink_mmap().
> 
> Which shrink_mmap race condition?

Oh, you're right. I was worried about what would happen if we
looped back in the while() loop without the lock held, but now
I see that we grab the pagemap_lru_lock in unlock_continue...

I guess shrink_mmap() needs some comments. ;)

> >The patch does the following:
> >- remove possible race condition from truncate_inode_pages()
> 
> Which truncate_inode_pages race condition? Please provide a
> stack trace, it shouldn't take too time for you if you have the
> race condition in mind.

Stephen has already answered this question a number of
emails ago.

regards,

Rik
--
The Internet is not a network of computers. It is a network
of people. That is its real strength.

Wanna talk about the kernel?  irc.openprojects.net / #kernelnewbies
http://www.conectiva.com/		http://www.surriel.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.eu.org/Linux-MM/

Re: [patch] shrink_mmap() 2.3.99-pre6-3 (take 3)

[Andrea Arcangeli]

On Wed, 19 Apr 2000, Rik van Riel wrote:

>I guess shrink_mmap() needs some comments. ;)

I tried to add commentary for everything that I considered subtle, but of
course I won't reject further commentary.

>> Which truncate_inode_pages race condition? Please provide a
>> stack trace, it shouldn't take too time for you if you have the
>> race condition in mind.
>
>Stephen has already answered this question a number of
>emails ago.

Maybe I missed the email (if it's not a pain for you please forward it to
me). thanks!

To me it seems the VFS avoids by design truncate_inode_pages() to be
called in parallel on the same inode (that was the problem you mentioned 
in earlier email).

Andrea

--
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.eu.org/Linux-MM/