From: Nick Piggin <npiggin@suse.de>
To: Andrew Morton <akpm@linux-foundation.org>,
linux-fsdevel@vger.kernel.org, mhalcrow@us.ibm.com,
phillip@hellewell.homeip.net, sfrench@samba.org,
dhowells@redhat.com
Subject: [rfc][patch 2/5] cifs: new aops
Date: Mon, 12 Nov 2007 08:14:23 +0100 [thread overview]
Message-ID: <20071112071423.GD22953@wotan.suse.de> (raw)
In-Reply-To: <20071112071245.GB22953@wotan.suse.de>
Convert cifs to new aops.
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.
Signed-off-by: Nick Piggin <npiggin@suse.de>
---
Index: linux-2.6/fs/cifs/file.c
===================================================================
--- linux-2.6.orig/fs/cifs/file.c
+++ linux-2.6/fs/cifs/file.c
@@ -103,7 +103,7 @@ static inline int cifs_open_inode_helper
/* want handles we can use to read with first
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) {
list_add_tail(&pCifsFile->flist,
&pCifsInode->openFileList);
@@ -1411,49 +1411,53 @@ static int cifs_writepage(struct page *p
return rc;
}
-static int cifs_commit_write(struct file *file, struct page *page,
- unsigned offset, unsigned to)
+static int cifs_write_end(struct file *file, struct address_space *mapping,
+ loff_t pos, unsigned len, unsigned copied,
+ struct page *page, void *fsdata)
{
- int xid;
- int rc = 0;
- struct inode *inode = page->mapping->host;
- loff_t position = ((loff_t)page->index << PAGE_CACHE_SHIFT) + to;
- char *page_data;
+ int rc;
+ struct inode *inode = mapping->host;
+ loff_t position;
+
+ cFYI(1, ("commit write for page %p from position %lld with %d bytes",
+ page, pos, copied));
+
+ if (!PageUptodate(page) && copied == PAGE_CACHE_SIZE)
+ SetPageUptodate(page);
- xid = GetXid();
- cFYI(1, ("commit write for page %p up to position %lld for %d",
- page, position, to));
- spin_lock(&inode->i_lock);
- if (position > inode->i_size) {
- i_size_write(inode, position);
- }
- spin_unlock(&inode->i_lock);
if (!PageUptodate(page)) {
- position = ((loff_t)page->index << PAGE_CACHE_SHIFT) + offset;
- /* can not rely on (or let) writepage write this data */
- if (to < offset) {
- cFYI(1, ("Illegal offsets, can not copy from %d to %d",
- offset, to));
- FreeXid(xid);
- return rc;
- }
+ char *page_data;
+ unsigned offset = pos & (PAGE_CACHE_SIZE - 1);
+ int xid;
+
+ xid = GetXid();
/* this is probably better than directly calling
partialpage_write since in this function the file handle is
known which we might as well leverage */
/* BB check if anything else missing out of ppw
such as updating last write time */
page_data = kmap(page);
- rc = cifs_write(file, page_data + offset, to-offset,
- &position);
- if (rc > 0)
- rc = 0;
- /* else if (rc < 0) should we set writebehind rc? */
+ rc = cifs_write(file, page_data + offset, copied, &position);
+ /* if (rc < 0) should we set writebehind rc? */
kunmap(page);
+
+ FreeXid(xid);
} else {
+ rc = copied;
set_page_dirty(page);
}
- FreeXid(xid);
+ if (rc > 0) {
+ position = pos + rc;
+ spin_lock(&inode->i_lock);
+ if (position > inode->i_size)
+ i_size_write(inode, position);
+ spin_unlock(&inode->i_lock);
+ }
+
+ unlock_page(page);
+ page_cache_release(page);
+
return rc;
}
@@ -1999,49 +2003,45 @@ int is_size_safe_to_change(struct cifsIn
return 1;
}
-static int cifs_prepare_write(struct file *file, struct page *page,
- unsigned from, unsigned to)
+static int cifs_write_begin(struct file *file, struct address_space *mapping,
+ loff_t pos, unsigned len, unsigned flags,
+ struct page **pagep, void **fsdata)
{
- int rc = 0;
- loff_t i_size;
- loff_t offset;
+ pgoff_t index = pos >> PAGE_CACHE_SHIFT;
+ loff_t offset = pos & (PAGE_CACHE_SIZE - 1);
+ struct page *page;
+
+ cFYI(1, ("write begin from %lld len %d", (long long)pos, len));
+
+ page = __grab_cache_page(mapping, index);
+ if (!page)
+ return -ENOMEM;
- cFYI(1, ("prepare write for page %p from %d to %d", page, from, to));
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);
+ if (len == PAGE_CACHE_SIZE && flags & AOP_FLAG_UNINTERRUPTIBLE)
return 0;
- }
- offset = (loff_t)page->index << PAGE_CACHE_SHIFT;
- i_size = i_size_read(page->mapping->host);
+ if ((file->f_flags & O_ACCMODE) != O_WRONLY) {
+ 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 */
rc = cifs_readpage_worker(file, page, &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 {
/* 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 */
+ 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;
}
@@ -2050,8 +2050,8 @@ const struct address_space_operations ci
.readpages = cifs_readpages,
.writepage = cifs_writepage,
.writepages = cifs_writepages,
- .prepare_write = cifs_prepare_write,
- .commit_write = cifs_commit_write,
+ .write_begin = cifs_write_begin,
+ .write_end = cifs_write_end,
.set_page_dirty = __set_page_dirty_nobuffers,
/* .sync_page = cifs_sync_page, */
/* .direct_IO = */
@@ -2066,8 +2066,8 @@ const struct address_space_operations ci
.readpage = cifs_readpage,
.writepage = cifs_writepage,
.writepages = cifs_writepages,
- .prepare_write = cifs_prepare_write,
- .commit_write = cifs_commit_write,
+ .write_begin = cifs_write_begin,
+ .write_end = cifs_write_end,
.set_page_dirty = __set_page_dirty_nobuffers,
/* .sync_page = cifs_sync_page, */
/* .direct_IO = */
next prev parent reply other threads:[~2007-11-12 7:14 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-11-12 7:12 [rfc][patches] remove ->prepare_write Nick Piggin
2007-11-12 7:13 ` [rfc][patch 1/5] ecryptfs new aops Nick Piggin
2007-11-12 7:14 ` Nick Piggin [this message]
2007-11-12 7:14 ` [rfc][patch 3/5] afs: " Nick Piggin
2007-11-12 7:20 ` [rfc][patch 4/5] rd: rewrite rd Nick Piggin
2007-11-12 7:23 ` [rfc][patch 5/5] remove prepare_write Nick Piggin
2007-11-12 15:29 ` [rfc][patch 3/5] afs: new aops David Howells
2007-11-13 0:15 ` Nick Piggin
2007-11-13 0:30 ` David Howells
2007-11-13 0:44 ` Nick Piggin
2007-11-13 10:56 ` David Howells
2007-11-14 4:24 ` Nick Piggin
2007-11-14 12:18 ` David Howells
2007-11-14 15:18 ` Nick Piggin
2007-11-14 15:57 ` David Howells
2007-11-14 21:32 ` Nick Piggin
2007-11-15 12:15 ` David Howells
2007-11-15 21:37 ` Nick Piggin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20071112071423.GD22953@wotan.suse.de \
--to=npiggin@suse.de \
--cc=akpm@linux-foundation.org \
--cc=dhowells@redhat.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=mhalcrow@us.ibm.com \
--cc=phillip@hellewell.homeip.net \
--cc=sfrench@samba.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).