[CIFS] fix regression in cifs_write_begin/cifs_write_end

The conversion to write_begin/write_end interfaces had a bug where we
were passing a bad parameter to cifs_readpage_worker. Rather than
passing the page offset of the start of the write, we needed to pass the
offset of the beginning of the page. This was reliably showing up as
data corruption in the fsx-linux test from LTP.

It also became evident that this code was occasionally doing unnecessary
read calls. Optimize those away by using the PG_checked flag to indicate
that the unwritten part of the page has been initialized.

CC: Nick Piggin <npiggin@suse.de>
Acked-by: Dave Kleikamp <shaggy@us.ibm.com>
Signed-off-by: Jeff Layton <jlayton@redhat.com>
Signed-off-by: Steve French <sfrench@us.ibm.com>

authored by Jeff Layton and committed by Steve French a98ee8c1 ed313489

+56 -21
+56 -21
fs/cifs/file.c
··· 1475 1475 cFYI(1, ("write_end for page %p from pos %lld with %d bytes", 1476 1476 page, pos, copied)); 1477 1477 1478 - if (!PageUptodate(page) && copied == PAGE_CACHE_SIZE) 1478 + if (PageChecked(page)) { 1479 + if (copied == len) 1480 + SetPageUptodate(page); 1481 + ClearPageChecked(page); 1482 + } else if (!PageUptodate(page) && copied == PAGE_CACHE_SIZE) 1479 1483 SetPageUptodate(page); 1480 1484 1481 1485 if (!PageUptodate(page)) { ··· 2066 2062 { 2067 2063 pgoff_t index = pos >> PAGE_CACHE_SHIFT; 2068 2064 loff_t offset = pos & (PAGE_CACHE_SIZE - 1); 2065 + loff_t page_start = pos & PAGE_MASK; 2066 + loff_t i_size; 2067 + struct page *page; 2068 + int rc = 0; 2069 2069 2070 2070 cFYI(1, ("write_begin from %lld len %d", (long long)pos, len)); 2071 2071 2072 - *pagep = __grab_cache_page(mapping, index); 2073 - if (!*pagep) 2074 - return -ENOMEM; 2072 + page = __grab_cache_page(mapping, index); 2073 + if (!page) { 2074 + rc = -ENOMEM; 2075 + goto out; 2076 + } 2075 2077 2076 - if (PageUptodate(*pagep)) 2077 - return 0; 2078 + if (PageUptodate(page)) 2079 + goto out; 2078 2080 2079 - /* If we are writing a full page it will be up to date, 2080 - no need to read from the server */ 2081 - if (len == PAGE_CACHE_SIZE && flags & AOP_FLAG_UNINTERRUPTIBLE) 2082 - return 0; 2081 + /* 2082 + * If we write a full page it will be up to date, no need to read from 2083 + * the server. If the write is short, we'll end up doing a sync write 2084 + * instead. 2085 + */ 2086 + if (len == PAGE_CACHE_SIZE) 2087 + goto out; 2088 + 2089 + /* 2090 + * optimize away the read when we have an oplock, and we're not 2091 + * expecting to use any of the data we'd be reading in. That 2092 + * is, when the page lies beyond the EOF, or straddles the EOF 2093 + * and the write will cover all of the existing data. 2094 + */ 2095 + if (CIFS_I(mapping->host)->clientCanCacheRead) { 2096 + i_size = i_size_read(mapping->host); 2097 + if (page_start >= i_size || 2098 + (offset == 0 && (pos + len) >= i_size)) { 2099 + zero_user_segments(page, 0, offset, 2100 + offset + len, 2101 + PAGE_CACHE_SIZE); 2102 + /* 2103 + * PageChecked means that the parts of the page 2104 + * to which we're not writing are considered up 2105 + * to date. Once the data is copied to the 2106 + * page, it can be set uptodate. 2107 + */ 2108 + SetPageChecked(page); 2109 + goto out; 2110 + } 2111 + } 2083 2112 2084 2113 if ((file->f_flags & O_ACCMODE) != O_WRONLY) { 2085 - int rc; 2086 - 2087 - /* might as well read a page, it is fast enough */ 2088 - rc = cifs_readpage_worker(file, *pagep, &offset); 2089 - 2090 - /* we do not need to pass errors back 2091 - e.g. if we do not have read access to the file 2092 - because cifs_write_end will attempt synchronous writes 2093 - -- shaggy */ 2114 + /* 2115 + * might as well read a page, it is fast enough. If we get 2116 + * an error, we don't need to return it. cifs_write_end will 2117 + * do a sync write instead since PG_uptodate isn't set. 2118 + */ 2119 + cifs_readpage_worker(file, page, &page_start); 2094 2120 } else { 2095 2121 /* we could try using another file handle if there is one - 2096 2122 but how would we lock it to prevent close of that handle 2097 2123 racing with this read? In any case 2098 2124 this will be written out by write_end so is fine */ 2099 2125 } 2100 - 2101 - return 0; 2126 + out: 2127 + *pagep = page; 2128 + return rc; 2102 2129 } 2103 2130 2104 2131 const struct address_space_operations cifs_addr_ops = {