[BUG] CIFS won't link

[Linus Torvalds]

On Sat, 23 Dec 2006, Ian McDonald wrote:
> 
> This isn't my area of expertise so I'm not providing a fix as I'm sure
> I'll get it wrong!

Well, I probably get it wrong too, but here's an email I already sent out 
to some people who could at least _test_ it (which I can't).

The more people who see this and can test it, the better..

		Linus
---
Date: Fri, 22 Dec 2006 16:25:42 -0800 (PST)
From: Linus Torvalds <torvalds at osdl.org>
To: Jean Delvare <khali at linux-fr.org>
cc: Thomas Meyer <thomas at m3y3r.de>, Steve French <sfrench at samba.org>, 
    linux-kernel at vger.kernel.org
Subject: Re: WARNING: "test_clear_page_dirty" [fs/cifs/cifs.ko] undefined!
Message-ID: <Pine.LNX.4.64.0612221615430.3671 at woody.osdl.org>

On Fri, 22 Dec 2006, Jean Delvare wrote:
> 
> The approach seems quite broken to me, the users should have been fixed
> _before_ removing the function, so as to avoid compilation failures.
> These are a pain for testers, and break git bisect too. Grmbl.

This needed to be fixed, and quite frankly, things don't get fixed nearly 
as quickly if you don't just break them first. And there really were just 
two filesystems that got broken, cifs being one of them.

I just can't test it.

> Now that it's done... Steve, can you please take a look and provide a
> patch so that cifs builds again?

CIFS _should_ be using "clear_page_dirty_for_io()" in that place, and that 
will fix the build. However, the reason I didn't just do that myself is 
that I can't test the end result, and for the life of me, I can't see 
where CIFS does the "end_page_writeback()" that it needs to do at IO 
completion time.

And the thing that confuses me about that, is that if CIFS doesn't do 
"end_page_writeback()", then it was already broken before - because when 
the VM calls "->writepage()" the clear_page_dirty_for_io() will have been 
done by the VM, and it needs that "end_page_writeback()" so that the 
system can know when the IO is done.

I _suspect_ that those "unlock_page()" calls should be accompanied by a 
"end_page_writeback()" call, and that the proper patch MAY look something 
like the appended, but I worry about having missed something really 
subtle. Maybe there's a end_page_writeback() somewhere else.

And if there isn't, I wonder if shared mappings have _ever_ worked on 
CIFS? And if so, how? That writeback bit thing isn't new per se.

So this may or may not fix it. If you can test it (_including_ with some 
dirty shared mmap-on-mmap action, please - just call me kinky), I'll 
commit it. But I need somebody who actually uses this to test it.

		Linus

---
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 0f05cab..4f0472d 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -1245,7 +1245,7 @@ retry:
 				wait_on_page_writeback(page);
 
 			if (PageWriteback(page) ||
-					!test_clear_page_dirty(page)) {
+					!clear_page_dirty_for_io(page)) {
 				unlock_page(page);
 				break;
 			}
@@ -1253,6 +1253,7 @@ retry:
 			if (page_offset(page) >= mapping->host->i_size) {
 				done = 1;
 				unlock_page(page);
+				end_page_writeback(page);
 				break;
 			}
 
@@ -1316,6 +1317,7 @@ retry:
 					SetPageError(page);
 				kunmap(page);
 				unlock_page(page);
+				end_page_writeback(page);
 				page_cache_release(page);
 			}
 			if ((wbc->nr_to_write -= n_iov) <= 0)
@@ -1356,7 +1358,8 @@ static int cifs_writepage(struct page* page, struct writeback_control *wbc)
 	rc = cifs_partialpagewrite(page, 0, PAGE_CACHE_SIZE);
 	SetPageUptodate(page); /* BB add check for error and Clearuptodate? */
 	unlock_page(page);
-	page_cache_release(page);	
+	end_page_writeback(page);
+	page_cache_release(page);
 	FreeXid(xid);
 	return rc;
 }