From: Steve French <sfrench@us.ibm.com>
Date: Tue, 6 Mar 2007 00:31:00 +0000 (+0000)
Subject: [CIFS] cifs_prepare_write was incorrectly rereading page in some cases
X-Git-Tag: v2.6.21-rc3~21^2
X-Git-Url: https://err.no/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8a236264f7d6db3f52881d37a86c5a5f704072b0;p=linux-2.6

[CIFS] cifs_prepare_write was incorrectly rereading page in some cases

Noticed by Shaggy.

Signed-off-by: Shaggy <shaggy@us.ibm.com>
Signed-off-by: Steve French <sfrench@us.ibm.com>
---

diff --git a/fs/cifs/CHANGES b/fs/cifs/CHANGES
index e08a147c09..6247628bda 100644
--- a/fs/cifs/CHANGES
+++ b/fs/cifs/CHANGES
@@ -2,8 +2,9 @@ Verison 1.48
 ------------
 Fix mtime bouncing around from local idea of last write times to remote time.
 Fix hang (in i_size_read) when simultaneous size update of same remote file
-on smp system corrupts sequence number.
- 
+on smp system corrupts sequence number. Do not reread unnecessarily partial page
+(which we are about to overwrite anyway) when writing out file opened rw.
+
 Version 1.47
 ------------
 Fix oops in list_del during mount caused by unaligned string.
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index c07ff8317a..2d3275bedb 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -1992,34 +1992,52 @@ static int cifs_prepare_write(struct file *file, struct page *page,
 	unsigned from, unsigned to)
 {
 	int rc = 0;
-        loff_t offset = (loff_t)page->index << PAGE_CACHE_SHIFT;
+	loff_t i_size;
+	loff_t offset;
+
 	cFYI(1, ("prepare write for page %p from %d to %d",page,from,to));
-	if (!PageUptodate(page)) {
-	/*	if (to - from != PAGE_CACHE_SIZE) {
-			void *kaddr = kmap_atomic(page, KM_USER0);
+	if (PageUptodate(page))
+		return 0;
+
+	/* If we are writing a full page it will be up to date,
+	   no need to read from the server */
+	if ((to == PAGE_CACHE_SIZE) && (from == 0)) {
+		SetPageUptodate(page);
+		return 0;
+	}
+
+	offset = (loff_t)page->index << PAGE_CACHE_SHIFT;
+	i_size = i_size_read(page->mapping->host);
+
+	if ((offset >= i_size) ||
+	    ((from == 0) && (offset + to) >= i_size)) {
+		/*
+		 * We don't need to read data beyond the end of the file.
+		 * zero it, and set the page uptodate
+		 */
+		void *kaddr = kmap_atomic(page, KM_USER0);
+
+		if (from)
 			memset(kaddr, 0, from);
+		if (to < PAGE_CACHE_SIZE)
 			memset(kaddr + to, 0, PAGE_CACHE_SIZE - to);
-			flush_dcache_page(page);
-			kunmap_atomic(kaddr, KM_USER0);
-		} */
-		/* If we are writing a full page it will be up to date,
-		   no need to read from the server */
-		if ((to == PAGE_CACHE_SIZE) && (from == 0))
-			SetPageUptodate(page);
-
+		flush_dcache_page(page);
+		kunmap_atomic(kaddr, KM_USER0);
+		SetPageUptodate(page);
+	} else if ((file->f_flags & O_ACCMODE) != O_WRONLY) {
 		/* might as well read a page, it is fast enough */
-		if ((file->f_flags & O_ACCMODE) != O_WRONLY) {
-			rc = cifs_readpage_worker(file, page, &offset);
-		} else {
-		/* should we try using another file handle if there is one -
-		   how would we lock it to prevent close of that handle
-		   racing with this read?
-		   In any case this will be written out by commit_write */
-		}
+		rc = cifs_readpage_worker(file, page, &offset);
+	} else {
+		/* we could try using another file handle if there is one -
+		   but how would we lock it to prevent close of that handle
+		   racing with this read? In any case
+		   this will be written out by commit_write so is fine */
 	}
 
-	/* BB should we pass any errors back? 
-	   e.g. if we do not have read access to the file */
+	/* we do not need to pass errors back 
+	   e.g. if we do not have read access to the file 
+	   because cifs_commit_write will do the right thing.  -- shaggy */
+
 	return 0;
 }
 
diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
index f80007eaeb..5f468459a1 100644
--- a/fs/cifs/transport.c
+++ b/fs/cifs/transport.c
@@ -499,7 +499,7 @@ SendReceive2(const unsigned int xid, struct cifsSesInfo *ses,
 	   due to last connection to this server being unmounted */
 	if (signal_pending(current)) {
 		/* if signal pending do not hold up user for full smb timeout
-		but we still give response a change to complete */
+		but we still give response a chance to complete */
 		timeout = 2 * HZ;
 	}   
 
@@ -587,7 +587,6 @@ SendReceive2(const unsigned int xid, struct cifsSesInfo *ses,
 	}
 
 out:
-
 	DeleteMidQEntry(midQ);
 	atomic_dec(&ses->server->inFlight); 
 	wake_up(&ses->server->request_q);
@@ -681,7 +680,7 @@ SendReceive(const unsigned int xid, struct cifsSesInfo *ses,
 	   due to last connection to this server being unmounted */
 	if (signal_pending(current)) {
 		/* if signal pending do not hold up user for full smb timeout
-		but we still give response a change to complete */
+		but we still give response a chance to complete */
 		timeout = 2 * HZ;
 	}   
 
@@ -765,7 +764,6 @@ SendReceive(const unsigned int xid, struct cifsSesInfo *ses,
 	}
 
 out:
-
 	DeleteMidQEntry(midQ);
 	atomic_dec(&ses->server->inFlight); 
 	wake_up(&ses->server->request_q);