Re: [PATCH] mm: fix page_mkclean_one

[Linus Torvalds <torvalds@osdl.org>]

On Tue, 26 Dec 2006, David Miller wrote:
> 
> I've seen it on sparc64, UP kernel, no preempt.

Btw, having tried to debug the writeback code, there's one very special 
case that just makes me go "hmm".

If we have a buffer that is "busy" when we try to write back a page, we 
have this magic "wbc->sync_mode == WB_SYNC_NONE && wbc->nonblocking" mode, 
where we won't wait for it, but instead we'll redirty the page and redo 
the whole thing.

Looking at the code, that should all work, but at the same time, it 
triggers some of my debug messages about having a dirty page during 
writeback, and one way to trigger that debug message is to try to run 
rtorrent on the machine.. 

I dunno. Witht he writeback being suspicious, and the normal 
"block_write_full_page()" path being implicated in at least ext2, I just 
wonder. This is one of those "let's see if behaviour changes" patches, 
that I'm just throwing out there..

		Linus

---
diff --git a/fs/buffer.c b/fs/buffer.c
index 263f88e..4652ef1 100644
--- a/fs/buffer.c
+++ b/fs/buffer.c
@@ -1653,19 +1653,7 @@ static int __block_write_full_page(struct inode *inode, struct page *page,
 	do {
 		if (!buffer_mapped(bh))
 			continue;
-		/*
-		 * If it's a fully non-blocking write attempt and we cannot
-		 * lock the buffer then redirty the page.  Note that this can
-		 * potentially cause a busy-wait loop from pdflush and kswapd
-		 * activity, but those code paths have their own higher-level
-		 * throttling.
-		 */
-		if (wbc->sync_mode != WB_SYNC_NONE || !wbc->nonblocking) {
-			lock_buffer(bh);
-		} else if (test_set_buffer_locked(bh)) {
-			redirty_page_for_writepage(wbc, page);
-			continue;
-		}
+		lock_buffer(bh);
 		if (test_clear_buffer_dirty(bh)) {
 			mark_buffer_async_write(bh);
 		} else {