[PATCH] count for buffer IO in page_launder()

[PATCH] count for buffer IO in page_launder()

[Marcelo Tosatti]

Hi,

page_launder() is not counting direct ll_rw_block() IO correctly in the
flushed pages counter. 

A page is only counted as flushed if it had its buffer_head's freed,
meaning that pages which have been queued but not freed are not counted.

The following patch against ac5 fixes the problem.



diff -Nur linux.orig/mm/vmscan.c linux/mm/vmscan.c
--- linux.orig/mm/vmscan.c	Sat Feb 24 23:30:20 2001
+++ linux/mm/vmscan.c	Mon Feb 26 05:45:08 2001
@@ -535,7 +535,7 @@
 		 * buffer pages
 		 */
 		if (page->buffers) {
-			int wait, clearedbuf;
+			int wait;
 			/*
 			 * Since we might be doing disk IO, we have to
 			 * drop the spinlock and take an extra reference
@@ -554,7 +554,8 @@
 				wait = 0;	/* No IO */
 
 			/* Try to free the page buffers. */
-			clearedbuf = try_to_free_buffers(page, wait);
+			if (try_to_free_buffers(page, wait))
+				flushed_pages++;
 
 			/*
 			 * Re-take the spinlock. Note that we cannot
@@ -564,10 +565,8 @@
 			spin_lock(&pagemap_lru_lock);
 
 			/* The buffers were not freed. */
-			if (!clearedbuf) {
+			if (page->buffers) {
 				add_page_to_inactive_dirty_list(page);
-				if (wait)
-					flushed_pages++;
 
 			/* The page was only in the buffer cache. */
 			} else if (!page->mapping) {
diff -Nur linux.orig/fs/buffer.c linux/fs/buffer.c
--- linux.orig/fs/buffer.c	Sat Feb 24 23:30:16 2001
+++ linux/fs/buffer.c	Mon Feb 26 04:44:54 2001
@@ -1399,7 +1399,8 @@
 	 * instead.
 	 */
 	if (!offset) {
-		if (!try_to_free_buffers(page, 0)) {
+		try_to_free_buffers(page, 0)
+		if (page->buffers) {
 			atomic_inc(&buffermem_pages);
 			return 0;
 		}
@@ -2413,7 +2414,7 @@
 	spin_unlock(&free_list[index].lock);
 	write_unlock(&hash_table_lock);
 	spin_unlock(&lru_list_lock);
-	return 1;
+	return 0;
 
 busy_buffer_page:
 	/* Uhhuh, start writeback so that we don't end up with all dirty pages */
@@ -2428,6 +2429,7 @@
 			goto cleaned_buffers_try_again;
 		}
 		wakeup_bdflush(0);
+		return 1;
 	}
 	return 0;
 }






--
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] count for buffer IO in page_launder()

[Stephen C. Tweedie]

Hi,

On Tue, Feb 27, 2001 at 04:09:09AM -0300, Marcelo Tosatti wrote:
> 
> page_launder() is not counting direct ll_rw_block() IO correctly in the
> flushed pages counter. 

Having not seen any follow to this, it's worth asking: what is the
expected consequence of _not_ including this?  Have you done any
performance testing on it?

--Stephen
--
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] count for buffer IO in page_launder()

[Marcelo Tosatti]

On Fri, 2 Mar 2001, Stephen C. Tweedie wrote:

> Hi,
> 
> On Tue, Feb 27, 2001 at 04:09:09AM -0300, Marcelo Tosatti wrote:
> > 
> > page_launder() is not counting direct ll_rw_block() IO correctly in the
> > flushed pages counter. 
> 
> Having not seen any follow to this, it's worth asking: what is the
> expected consequence of _not_ including this?  

The page launder loop avoids flushing too many pages if it already
flushed/cleaned enough pages to remove the system from low memory
condition (mm/vmscan.c::page_launder()):

                /*
                 * Disk IO is really expensive, so we make sure we
                 * don't do more work than needed.
                 * Note that clean pages from zones with enough free
                 * pages still get recycled and dirty pages from these
                 * zones can get flushed due to IO clustering.
                 */
                if (freed_pages + flushed_pages > target && !free_shortage())
                        break;


Dirty buffer pages and dirty pagecache pages with page->buffers mapping
which were being flushed (with try_to_free_buffers()) were not being
counted in the "flushed_pages" counter correctly.

So what could happen is that tasks trying to launder pages could
flush/swapout more than needed. 

> Have you done an performance testing on it?

No. The code makes sense now.

--
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] count for buffer IO in page_launder()

[Stephen C. Tweedie]

Hi,

On Sat, Mar 03, 2001 at 01:52:19AM -0300, Marcelo Tosatti wrote:
> 
> On Fri, 2 Mar 2001, Stephen C. Tweedie wrote:
> 
> > Have you done an performance testing on it?
> 
> No. The code makes sense now.

The development of the VM has been _full_ of well-intended,
well-reasoned patches which failed to work properly for subtle
reasons.  I despair of us ever getting the 2.4 VM right as long as
people think it's safe to submit VM patches without even basic
performance testing.

This isn't an experimental kernel.  It's supposed to be a stable
branch.

Cheers,
 Stephen

--
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/