Commit d9414774 authored by Nick Piggin's avatar Nick Piggin Committed by Steve French

cifs: Convert cifs to new aops.

cifs: Convert cifs to new aops.

This patch is based on the one originally posted by Nick Piggin. His
patch was very close, but had a couple of small bugs. Nick's original
comments follow:

This is another relatively naive conversion. Always do the read upfront
when the page is not uptodate (unless we're in the writethrough path).

Fix an uninitialized data exposure where SetPageUptodate was called
before the page was uptodate.

SetPageUptodate and switch to writeback mode in the case that the full
page was dirtied.
Acked-by: default avatarShaggy <shaggy@austin.ibm.com>
Acked-by: default avatarBadari Pulavarty <pbadari@us.ibm.com>
Signed-off-by: default avatarNick Piggin <npiggin@suse.de>
Signed-off-by: default avatarJeff Layton <jlayton@redhat.com>
Signed-off-by: default avatarSteve French <sfrench@us.ibm.com>
parent d388908e
...@@ -107,7 +107,7 @@ static inline int cifs_open_inode_helper(struct inode *inode, struct file *file, ...@@ -107,7 +107,7 @@ static inline int cifs_open_inode_helper(struct inode *inode, struct file *file,
/* want handles we can use to read with first /* want handles we can use to read with first
in the list so we do not have to walk the in the list so we do not have to walk the
list to search for one in prepare_write */ list to search for one in write_begin */
if ((file->f_flags & O_ACCMODE) == O_WRONLY) { if ((file->f_flags & O_ACCMODE) == O_WRONLY) {
list_add_tail(&pCifsFile->flist, list_add_tail(&pCifsFile->flist,
&pCifsInode->openFileList); &pCifsInode->openFileList);
...@@ -1455,49 +1455,52 @@ static int cifs_writepage(struct page *page, struct writeback_control *wbc) ...@@ -1455,49 +1455,52 @@ static int cifs_writepage(struct page *page, struct writeback_control *wbc)
return rc; return rc;
} }
static int cifs_commit_write(struct file *file, struct page *page, static int cifs_write_end(struct file *file, struct address_space *mapping,
unsigned offset, unsigned to) loff_t pos, unsigned len, unsigned copied,
struct page *page, void *fsdata)
{ {
int xid; int rc;
int rc = 0; struct inode *inode = mapping->host;
struct inode *inode = page->mapping->host;
loff_t position = ((loff_t)page->index << PAGE_CACHE_SHIFT) + to;
char *page_data;
xid = GetXid(); cFYI(1, ("write_end for page %p from pos %lld with %d bytes",
cFYI(1, ("commit write for page %p up to position %lld for %d", page, pos, copied));
page, position, to));
spin_lock(&inode->i_lock); if (!PageUptodate(page) && copied == PAGE_CACHE_SIZE)
if (position > inode->i_size) SetPageUptodate(page);
i_size_write(inode, position);
spin_unlock(&inode->i_lock);
if (!PageUptodate(page)) { if (!PageUptodate(page)) {
position = ((loff_t)page->index << PAGE_CACHE_SHIFT) + offset; char *page_data;
/* can not rely on (or let) writepage write this data */ unsigned offset = pos & (PAGE_CACHE_SIZE - 1);
if (to < offset) { int xid;
cFYI(1, ("Illegal offsets, can not copy from %d to %d",
offset, to)); xid = GetXid();
FreeXid(xid);
return rc;
}
/* this is probably better than directly calling /* this is probably better than directly calling
partialpage_write since in this function the file handle is partialpage_write since in this function the file handle is
known which we might as well leverage */ known which we might as well leverage */
/* BB check if anything else missing out of ppw /* BB check if anything else missing out of ppw
such as updating last write time */ such as updating last write time */
page_data = kmap(page); page_data = kmap(page);
rc = cifs_write(file, page_data + offset, to-offset, rc = cifs_write(file, page_data + offset, copied, &pos);
&position); /* if (rc < 0) should we set writebehind rc? */
if (rc > 0)
rc = 0;
/* else if (rc < 0) should we set writebehind rc? */
kunmap(page); kunmap(page);
FreeXid(xid);
} else { } else {
rc = copied;
pos += copied;
set_page_dirty(page); set_page_dirty(page);
} }
FreeXid(xid); if (rc > 0) {
spin_lock(&inode->i_lock);
if (pos > inode->i_size)
i_size_write(inode, pos);
spin_unlock(&inode->i_lock);
}
unlock_page(page);
page_cache_release(page);
return rc; return rc;
} }
...@@ -2043,49 +2046,44 @@ bool is_size_safe_to_change(struct cifsInodeInfo *cifsInode, __u64 end_of_file) ...@@ -2043,49 +2046,44 @@ bool is_size_safe_to_change(struct cifsInodeInfo *cifsInode, __u64 end_of_file)
return true; return true;
} }
static int cifs_prepare_write(struct file *file, struct page *page, static int cifs_write_begin(struct file *file, struct address_space *mapping,
unsigned from, unsigned to) loff_t pos, unsigned len, unsigned flags,
struct page **pagep, void **fsdata)
{ {
int rc = 0; pgoff_t index = pos >> PAGE_CACHE_SHIFT;
loff_t i_size; loff_t offset = pos & (PAGE_CACHE_SIZE - 1);
loff_t offset;
cFYI(1, ("prepare write for page %p from %d to %d", page, from, to)); cFYI(1, ("write_begin from %lld len %d", (long long)pos, len));
if (PageUptodate(page))
*pagep = __grab_cache_page(mapping, index);
if (!*pagep)
return -ENOMEM;
if (PageUptodate(*pagep))
return 0; return 0;
/* If we are writing a full page it will be up to date, /* If we are writing a full page it will be up to date,
no need to read from the server */ no need to read from the server */
if ((to == PAGE_CACHE_SIZE) && (from == 0)) { if (len == PAGE_CACHE_SIZE && flags & AOP_FLAG_UNINTERRUPTIBLE)
SetPageUptodate(page);
return 0; return 0;
}
offset = (loff_t)page->index << PAGE_CACHE_SHIFT; if ((file->f_flags & O_ACCMODE) != O_WRONLY) {
i_size = i_size_read(page->mapping->host); int rc;
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
*/
simple_prepare_write(file, page, from, to);
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 */
rc = cifs_readpage_worker(file, page, &offset); rc = cifs_readpage_worker(file, *pagep, &offset);
/* we do not need to pass errors back
e.g. if we do not have read access to the file
because cifs_write_end will attempt synchronous writes
-- shaggy */
} else { } else {
/* we could try using another file handle if there is one - /* we could try using another file handle if there is one -
but how would we lock it to prevent close of that handle but how would we lock it to prevent close of that handle
racing with this read? In any case racing with this read? In any case
this will be written out by commit_write so is fine */ this will be written out by write_end so is fine */
} }
/* 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;
} }
...@@ -2094,8 +2092,8 @@ const struct address_space_operations cifs_addr_ops = { ...@@ -2094,8 +2092,8 @@ const struct address_space_operations cifs_addr_ops = {
.readpages = cifs_readpages, .readpages = cifs_readpages,
.writepage = cifs_writepage, .writepage = cifs_writepage,
.writepages = cifs_writepages, .writepages = cifs_writepages,
.prepare_write = cifs_prepare_write, .write_begin = cifs_write_begin,
.commit_write = cifs_commit_write, .write_end = cifs_write_end,
.set_page_dirty = __set_page_dirty_nobuffers, .set_page_dirty = __set_page_dirty_nobuffers,
/* .sync_page = cifs_sync_page, */ /* .sync_page = cifs_sync_page, */
/* .direct_IO = */ /* .direct_IO = */
...@@ -2110,8 +2108,8 @@ const struct address_space_operations cifs_addr_ops_smallbuf = { ...@@ -2110,8 +2108,8 @@ const struct address_space_operations cifs_addr_ops_smallbuf = {
.readpage = cifs_readpage, .readpage = cifs_readpage,
.writepage = cifs_writepage, .writepage = cifs_writepage,
.writepages = cifs_writepages, .writepages = cifs_writepages,
.prepare_write = cifs_prepare_write, .write_begin = cifs_write_begin,
.commit_write = cifs_commit_write, .write_end = cifs_write_end,
.set_page_dirty = __set_page_dirty_nobuffers, .set_page_dirty = __set_page_dirty_nobuffers,
/* .sync_page = cifs_sync_page, */ /* .sync_page = cifs_sync_page, */
/* .direct_IO = */ /* .direct_IO = */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment