[CIFS] cifs_prepare_write was incorrectly rereading page in some cases
authorSteve French <sfrench@us.ibm.com>
Tue, 6 Mar 2007 00:31:00 +0000 (00:31 +0000)
committerSteve French <sfrench@us.ibm.com>
Tue, 6 Mar 2007 00:31:00 +0000 (00:31 +0000)
Noticed by Shaggy.

Signed-off-by: Shaggy <shaggy@us.ibm.com>
Signed-off-by: Steve French <sfrench@us.ibm.com>
fs/cifs/CHANGES
fs/cifs/file.c
fs/cifs/transport.c

index e08a147c09e12be99d84fd7dc17411dc7db23c88..6247628bdaed896cf09485c2a50f66edb5c40d5b 100644 (file)
@@ -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.
index c07ff8317a8b051c4d636cac9d25c49f15bdd5c2..2d3275bedb55b9a4667027404f5a811c5b0d5ef2 100644 (file)
@@ -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;
 }
 
index f80007eaebf4100c67224e4ffa524d2c6c517517..5f468459a1e294fb9f2ba30b2141bc5bc26eaab0 100644 (file)
@@ -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);