On Fri, 21 Nov 2008 14:38:18 -0600 "Steve French" <smfrench@xxxxxxxxx> wrote: > Fix attached. > > Shaggy/Jeff/Nick etc. do you want to review/ack it since it is late in the rc? > Talking with Steve on IRC, we thought it might be better to optimize away the read when possible. I think this patch should do it. We skip the read if the write starts past the current end of the file, or if the offset into the page of the beginning of the write is 0 and we're writing past the current end of the file. In those situations we just zero out the rest of the page. Combined patch inlined below. I also took the liberty of adding a page pointer to make the code look a little cleaner. Thoughts? Signed-off-by: Jeff Layton <jlayton@xxxxxxxxxx> Signed-off-by: Steve French <smfrench@xxxxxxxxxx> --- fs/cifs/file.c | 42 ++++++++++++++++++++++++++++++------------ 1 files changed, 30 insertions(+), 12 deletions(-) diff --git a/fs/cifs/file.c b/fs/cifs/file.c index b691b89..1d252b8 100644 --- a/fs/cifs/file.c +++ b/fs/cifs/file.c @@ -2062,26 +2062,43 @@ static int cifs_write_begin(struct file *file, struct address_space *mapping, { pgoff_t index = pos >> PAGE_CACHE_SHIFT; loff_t offset = pos & (PAGE_CACHE_SIZE - 1); + loff_t page_start = pos & PAGE_MASK; + loff_t i_size; + struct page *page; + char *buf; + int rc = 0; cFYI(1, ("write_begin from %lld len %d", (long long)pos, len)); - *pagep = __grab_cache_page(mapping, index); - if (!*pagep) - return -ENOMEM; + page = __grab_cache_page(mapping, index); + if (!page) { + rc = -ENOMEM; + goto out; + } - if (PageUptodate(*pagep)) - return 0; + if (PageUptodate(page)) + goto out; /* If we are writing a full page it will be up to date, no need to read from the server */ if (len == PAGE_CACHE_SIZE && flags & AOP_FLAG_UNINTERRUPTIBLE) - return 0; - - if ((file->f_flags & O_ACCMODE) != O_WRONLY) { - int rc; + goto out; + i_size = i_size_read(page->mapping->host); + if (pos >= i_size || ((offset == 0) && ((pos + len) >= i_size))) { + /* + * optimize away the read when the unwritten part of the page + * isn't expected to have any existing file data. Just zero + * out the unused parts. + */ + buf = kmap(page); + memset(buf, 0, offset); + memset(buf + offset + len, 0, PAGE_CACHE_SIZE - (offset + len)); + kunmap(page); + SetPageUptodate(page); + } else if ((file->f_flags & O_ACCMODE) != O_WRONLY) { /* might as well read a page, it is fast enough */ - rc = cifs_readpage_worker(file, *pagep, &offset); + cifs_readpage_worker(file, page, &page_start); /* we do not need to pass errors back e.g. if we do not have read access to the file @@ -2093,8 +2110,9 @@ static int cifs_write_begin(struct file *file, struct address_space *mapping, racing with this read? In any case this will be written out by write_end so is fine */ } - - return 0; +out: + *pagep = page; + return rc; } const struct address_space_operations cifs_addr_ops = { -- 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