[PATCH] -ac21 don't set referenced bit

[PATCH] -ac21 don't set referenced bit

[Rik van Riel]

Hi,

the patch below, against -ac21, does two things:

1) do not set the referenced bit when we add a page to
   one of the caches ... this allows us to distinguish
   between pages which are used again and pages which
   aren't   [keeps performance nice in the presence of
   streaming IO?]

2) in do_try_to_free_pages, if we make progress on a
   priority level, do not decrease priority when we loop
   again ... this should allow us to  a) free more pages
   since we can keep looping back as long as progress is
   being made and  b) maybe keep a better balance between
   shrink_mmap and swap_out since we have a better chance
   to never reach the "gimme memory now or I'll rape your
   wife and kill your children" level
   [and we don't want to get there since arriving at that
   priority level means we'll be doing worse page aging
   and forcing the issue ... potentially making things worse
   for the next time]

With this patch I'm seeing higher application performance and
lower kswapd cpu usage...

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/



--- mm/filemap.c.orig	Mon Jun 19 11:32:32 2000
+++ mm/filemap.c	Mon Jun 19 11:33:21 2000
@@ -564,7 +564,7 @@
 
 /*
  * This adds a page to the page cache, starting out as locked,
- * owned by us, referenced, but not uptodate and with no errors.
+ * owned by us, but not uptodate and with no errors.
  */
 static inline void __add_to_page_cache(struct page * page,
 	struct address_space *mapping, unsigned long offset,
@@ -576,8 +576,8 @@
 	if (PageLocked(page))
 		BUG();
 
-	flags = page->flags & ~((1 << PG_uptodate) | (1 << PG_error) | (1 << PG_dirty));
-	page->flags = flags | (1 << PG_locked) | (1 << PG_referenced);
+	flags = page->flags & ~((1 << PG_uptodate) | (1 << PG_error));
+	page->flags = flags | (1 << PG_locked);
 	page_cache_get(page);
 	page->index = offset;
 	add_page_to_inode_queue(mapping, page);
--- mm/swap_state.c.orig	Mon Jun 19 11:32:32 2000
+++ mm/swap_state.c	Mon Jun 19 11:33:59 2000
@@ -58,8 +58,8 @@
 		BUG();
 	if (page->mapping)
 		BUG();
-	flags = page->flags & ~((1 << PG_error) | (1 << PG_dirty));
-	page->flags = flags | (1 << PG_referenced) | (1 << PG_uptodate);
+	flags = page->flags & ~(1 << PG_error);
+	page->flags = flags | (1 << PG_uptodate);
 	add_to_page_cache_locked(page, &swapper_space, entry.val);
 }
 
--- mm/vmscan.c.orig	Mon Jun 19 11:32:32 2000
+++ mm/vmscan.c	Mon Jun 19 11:47:10 2000
@@ -444,6 +444,7 @@
 	int priority;
 	int count = FREE_COUNT;
 	int swap_count = 0;
+	int made_progress = 0;
 	int ret = 0;
 
 	/* Always trim SLAB caches when memory gets low. */
@@ -452,7 +453,7 @@
 	priority = 64;
 	do {
 		while (shrink_mmap(priority, gfp_mask)) {
-			ret = 1;
+			made_progress = 1;
 			if (!--count)
 				goto done;
 		}
@@ -468,11 +469,11 @@
 			count -= shrink_dcache_memory(priority, gfp_mask);
 			count -= shrink_icache_memory(priority, gfp_mask);
 			if (count <= 0) {
-				ret = 1;
+				made_progress = 1;
 				goto done;
 			}
 			while (shm_swap(priority, gfp_mask)) {
-				ret = 1;
+				made_progress = 1;
 				if (!--count)
 					goto done;
 			}
@@ -493,11 +494,25 @@
 		 */
 		swap_count += count;
 		while (swap_out(priority, gfp_mask)) {
+			made_progress = 1;
 			if (--swap_count < 0)
 				break;
 		}
 
-	} while (--priority >= 0);
+		/*
+		 * If we made progress at the current priority, the next
+		 * loop will also be done at this priority level. There's
+		 * absolutely no reason to drop to a lower priority and
+		 * potentially upset the balance between shrink_mmap and
+		 * swap_out.
+		 */
+		if (made_progress) {
+			made_progress = 0;
+			ret = 1;
+		} else {
+			priority--;
+		}
+	} while (priority >= 0);
 
 	/* Always end on a shrink_mmap.. */
 	while (shrink_mmap(0, gfp_mask)) {


--
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] -ac21 don't set referenced bit

[Andrea Arcangeli]

On Mon, 19 Jun 2000, Rik van Riel wrote:

>the patch below, against -ac21, does two things:
>
>1) do not set the referenced bit when we add a page to
>   one of the caches ... this allows us to distinguish

Glad to see you agreed with that. You forgot the buffer cache, hint from:

	ftp://ftp.*.kernel.org/pub/linux/kernel/people/andrea/patches/v2.4/2.4.0-test1-ac21/classzone-32.gz

@@ -2338,7 +2362,8 @@
        spin_unlock(&free_list[isize].lock);
 
        page->buffers = bh;
-       lru_cache_add(page);
+       page->flags &= ~(1 << PG_referenced);
+       lru_cache_add(page, LRU_NORMAL_CACHE);
        atomic_inc(&buffermem_pages);
        return 1;
 

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] -ac21 don't set referenced bit

[Rik van Riel]

On Mon, 19 Jun 2000, Andrea Arcangeli wrote:
> On Mon, 19 Jun 2000, Rik van Riel wrote:
> 
> >the patch below, against -ac21, does two things:
> >
> >1) do not set the referenced bit when we add a page to
> >   one of the caches ... this allows us to distinguish
> 
> Glad to see you agreed with that. You forgot the buffer cache, hint from:

No I didn't ... ;)

> @@ -2338,7 +2362,8 @@
>         spin_unlock(&free_list[isize].lock);
>  
>         page->buffers = bh;
> -       lru_cache_add(page);
> +       page->flags &= ~(1 << PG_referenced);
> +       lru_cache_add(page, LRU_NORMAL_CACHE);
>         atomic_inc(&buffermem_pages);
>         return 1;

>From include/linux/swap.h:

#define lru_cache_add(page)                     \
do {                                            \
        spin_lock(&pagemap_lru_lock);           \
        list_add(&(page)->lru, &lru_cache);     \
        nr_lru_pages++;                         \
        page->age = PG_AGE_START;               \
        ClearPageReferenced(page);              \
        SetPageActive(page);                    \
        spin_unlock(&pagemap_lru_lock);         \
} while (0)

We've had this for a number of kernel versions now...

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] -ac21 don't set referenced bit

[Andrea Arcangeli]

On Mon, 19 Jun 2000, Rik van Riel wrote:

>#define lru_cache_add(page)                     \
>do {                                            \
>        spin_lock(&pagemap_lru_lock);           \
>        list_add(&(page)->lru, &lru_cache);     \
>        nr_lru_pages++;                         \
>        page->age = PG_AGE_START;               \
>        ClearPageReferenced(page);              \
>        SetPageActive(page);                    \
>        spin_unlock(&pagemap_lru_lock);         \
>} while (0)
>
>We've had this for a number of kernel versions now...

Woops, sorry I missed that (I rewrote all such functions and I had in mind
the old ones). However clearing there cause some place to clear two
times. Just think what we do when we insert a page in the page
cache. That's why I'm not embedding the reference bit clear into the
lru_cache_add.

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] -ac21 don't set referenced bit

[Rik van Riel]

On Mon, 19 Jun 2000, Andrea Arcangeli wrote:
> On Mon, 19 Jun 2000, Rik van Riel wrote:
> 
> >#define lru_cache_add(page)                     \
> >do {                                            \
> >        spin_lock(&pagemap_lru_lock);           \
> >        list_add(&(page)->lru, &lru_cache);     \
> >        nr_lru_pages++;                         \
> >        page->age = PG_AGE_START;               \
> >        ClearPageReferenced(page);              \
> >        SetPageActive(page);                    \
> >        spin_unlock(&pagemap_lru_lock);         \
> >} while (0)
> >
> >We've had this for a number of kernel versions now...
> 
> Woops, sorry I missed that (I rewrote all such functions and I
> had in mind the old ones). However clearing there cause some
> place to clear two times.

My approach is a bit simpler. Since we *always* want to clear
the bit when we put the page in the LRU list, we can simply
remove that piece of code duplication from elsewhere in the
code.

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] -ac21 don't set referenced bit

[Andrea Arcangeli]

On Mon, 19 Jun 2000, Rik van Riel wrote:

>My approach is a bit simpler. Since we *always* want to clear
>the bit when we put the page in the LRU list, we can simply
>remove that piece of code duplication from elsewhere in the
>code.

I do the clear of the bit in __add_to_page_cache at zero cost. You do it
with a cost inside lru_cache_add. That's the only difference between the
two approchs.

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/