[PATCH 2.6.3-mm4] writeback trylock patch

[PATCH 2.6.3-mm4] writeback trylock patch

[Daniel McNeil]

[-- Attachment #1: Type: text/plain, Size: 379 bytes --]

Andrew,

Here is update to the wb_rwsema patch that adds back the trylock.
It avoids the hang hugh was seeing by setting encountered_congestion
if the trylock fails and checking it in sync_sb_inodes().  Hugh
tested this and did not see the hang.

This prevents non-sync writebacks to from blocking behind sync
writebacks.

This patch applies to 2.6.4-rc1-mm2.

Thoguhts?

Daniel

[-- Attachment #2: 263-mm4.writeback.trylock.patch --]
[-- Type: text/x-patch, Size: 1871 bytes --]

diff -rup linux-2.6.3-mm4.orig/fs/fs-writeback.c linux-2.6.3-mm4/fs/fs-writeback.c
--- linux-2.6.3-mm4.orig/fs/fs-writeback.c	2004-02-27 16:45:44.956839646 -0800
+++ linux-2.6.3-mm4/fs/fs-writeback.c	2004-02-27 16:57:38.746524736 -0800
@@ -158,10 +158,14 @@ __sync_single_inode(struct inode *inode,
 		 * for all i/o without worrying about racing WB_SYNC_NONE
 		 * writers.
 		 */
-		if (wait)
+		if (wait) {
 			down_write(&mapping->wb_rwsema);
-		else
-			down_read(&mapping->wb_rwsema);
+		} else {
+			if (!down_read_trylock(&mapping->wb_rwsema)) {
+				wbc->encountered_congestion = 1;
+				goto skip_writeback;
+			}
+		}
 	}
 
 	/*
@@ -184,6 +188,7 @@ __sync_single_inode(struct inode *inode,
 			up_read(&mapping->wb_rwsema);
 	}
 
+skip_writeback:
 	/* Don't write the inode if only I_DIRTY_PAGES was set */
 	if (dirty & (I_DIRTY_SYNC | I_DIRTY_DATASYNC))
 		write_inode(inode, wait);
@@ -310,7 +315,12 @@ sync_sb_inodes(struct super_block *sb, s
 			break;
 		}
 
-		if (wbc->nonblocking && bdi_write_congested(bdi)) {
+		/*
+		 * wbc->encountered_congestion is set if we cannot get
+		 * the wb_rwsema.
+		 */
+		if (wbc->nonblocking &&
+		    (bdi_write_congested(bdi) || wbc->encountered_congestion)) {
 			wbc->encountered_congestion = 1;
 			if (sb != blockdev_superblock)
 				break;		/* Skip a congested fs */
diff -rup linux-2.6.3-mm4.orig/mm/filemap.c linux-2.6.3-mm4/mm/filemap.c
--- linux-2.6.3-mm4.orig/mm/filemap.c	2004-02-27 16:47:56.351858126 -0800
+++ linux-2.6.3-mm4/mm/filemap.c	2004-02-27 16:49:29.044938317 -0800
@@ -152,9 +152,10 @@ static int __filemap_fdatawrite(struct a
 		return 0;
 
 	if (!blkdev) {
-		if (sync_mode == WB_SYNC_NONE)
-			down_read(&mapping->wb_rwsema);
-		else
+		if (sync_mode == WB_SYNC_NONE) {
+			if (!down_read_trylock(&mapping->wb_rwsema))
+				return 0;
+		} else 
 			down_write(&mapping->wb_rwsema);
 	}
 

Re: [PATCH 2.6.3-mm4] writeback trylock patch

[Andrew Morton]

Daniel McNeil <daniel@osdl.org> wrote:
>
> Here is update to the wb_rwsema patch that adds back the trylock.
> It avoids the hang hugh was seeing by setting encountered_congestion
> if the trylock fails and checking it in sync_sb_inodes().  Hugh
> tested this and did not see the hang.
> 
> This prevents non-sync writebacks to from blocking behind sync
> writebacks.
> 
> This patch applies to 2.6.4-rc1-mm2.
> 
> Thoguhts?

spose so.  One concern is with writer throttling: if the caller of write(2)
is simply skipping a large file then it should throttle in
balance_dirty_pages() or block on i_sem I guess.  This code is getting
pretty sticky.

Why are you setting wbc->encountered_congestion in sync_sb_inodes()?

And why are you doing a trylock in __filemap_fdatawrite()?  That only
affects fadvise(), and probably we want to block in there anyway.  It
doesn't matter really but less code is better code.