[linux-cifs-client] Re: fsx-linux failing with latest cifs-2.6 git tree

[Jeff Layton]

> 
> One minor thing -- you could do the !PageUptodate check first? If the
> page is already uptodate, then everything is much simpler I think? (and
> PageChecked should not be set).
> 
> if (!PageUptodate(page)) {
>     if (PageChecked(page)) {
>         if (copied == len)
>             SetPageUptodate(page);
>         ClearPageChecked(page);
>     } else if (copied == PAGE_CACHE_SIZE)
>         SetPageUptodate(page);
> }
> 
> I don't know if you think that's better or not, but I really like to
> make it clear that this is the !PageUptodate logic, and we never try
> to SetPageUptodate on an already uptodate page.
> 
> But I guess it is just a matter of style. So go with whatever you like
> best.

Sounds reasonable. It might even be more efficient to clean up all of
the logic in cifs_write_end at some point so that we only check
PageUptodate once. That'll take some re-org though, and the perf gain
would be minimal (if any).

This patch should apply cleanly on top of the patch already in Steve's
tree...

--------------[snip]---------------

From d4bd64bb2168a1833fa37a0e0eed8782afc633cc Mon Sep 17 00:00:00 2001
From: Jeff Layton <jlayton at redhat.com>
Date: Fri, 28 Nov 2008 07:08:59 -0500
Subject: [PATCH] cifs: clean up conditionals in cifs_write_end

Make it clear that the conditionals at the start of cifs_write_end are
just for the situation when the page is not uptodate.

Signed-off-by: Jeff Layton <jlayton at redhat.com>
---
 fs/cifs/file.c |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index f0a81e6..202a20f 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -1475,12 +1475,14 @@ static int cifs_write_end(struct file *file, struct address_space *mapping,
 	cFYI(1, ("write_end for page %p from pos %lld with %d bytes",
 		 page, pos, copied));
 
-	if (PageChecked(page)) {
-		if (copied == len)
+	if (!PageUptodate(page)) {
+		if (PageChecked(page)) {
+			if (copied == len)
+				SetPageUptodate(page);
+			ClearPageChecked(page);
+		} else if (copied == PAGE_CACHE_SIZE)
 			SetPageUptodate(page);
-		ClearPageChecked(page);
-	} else if (!PageUptodate(page) && copied == PAGE_CACHE_SIZE)
-		SetPageUptodate(page);
+	}
 
 	if (!PageUptodate(page)) {
 		char *page_data;
-- 
1.5.5.1