Selaa lähdekoodia

[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>
Steve French 18 vuotta sitten
vanhempi
commit
8a236264f7
3 muutettua tiedostoa jossa 45 lisäystä ja 28 poistoa
  1. 3 2
      fs/cifs/CHANGES
  2. 40 22
      fs/cifs/file.c
  3. 2 4
      fs/cifs/transport.c

+ 3 - 2
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 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
 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
 Version 1.47
 ------------
 ------------
 Fix oops in list_del during mount caused by unaligned string.
 Fix oops in list_del during mount caused by unaligned string.

+ 40 - 22
fs/cifs/file.c

@@ -1992,34 +1992,52 @@ static int cifs_prepare_write(struct file *file, struct page *page,
 	unsigned from, unsigned to)
 	unsigned from, unsigned to)
 {
 {
 	int rc = 0;
 	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));
 	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);
 			memset(kaddr, 0, from);
+		if (to < PAGE_CACHE_SIZE)
 			memset(kaddr + to, 0, PAGE_CACHE_SIZE - to);
 			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 */
 		/* 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;
 	return 0;
 }
 }
 
 

+ 2 - 4
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 */
 	   due to last connection to this server being unmounted */
 	if (signal_pending(current)) {
 	if (signal_pending(current)) {
 		/* if signal pending do not hold up user for full smb timeout
 		/* 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;
 		timeout = 2 * HZ;
 	}   
 	}   
 
 
@@ -587,7 +587,6 @@ SendReceive2(const unsigned int xid, struct cifsSesInfo *ses,
 	}
 	}
 
 
 out:
 out:
-
 	DeleteMidQEntry(midQ);
 	DeleteMidQEntry(midQ);
 	atomic_dec(&ses->server->inFlight); 
 	atomic_dec(&ses->server->inFlight); 
 	wake_up(&ses->server->request_q);
 	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 */
 	   due to last connection to this server being unmounted */
 	if (signal_pending(current)) {
 	if (signal_pending(current)) {
 		/* if signal pending do not hold up user for full smb timeout
 		/* 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;
 		timeout = 2 * HZ;
 	}   
 	}   
 
 
@@ -765,7 +764,6 @@ SendReceive(const unsigned int xid, struct cifsSesInfo *ses,
 	}
 	}
 
 
 out:
 out:
-
 	DeleteMidQEntry(midQ);
 	DeleteMidQEntry(midQ);
 	atomic_dec(&ses->server->inFlight); 
 	atomic_dec(&ses->server->inFlight); 
 	wake_up(&ses->server->request_q);
 	wake_up(&ses->server->request_q);