>
> 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@xxxxxxxxxx>
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@xxxxxxxxxx>
---
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
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
