From: Jeff Layton <jlayton@redhat.com>
To: smfrench@gmail.com
Cc: smfrench@austin.rr.com, linux-cifs-client@lists.samba.org,
linux-fsdevel@vger.kernel.org
Subject: [PATCH] cifs: fix O_APPEND on directio mounts
Date: Wed, 27 Aug 2008 20:24:52 -0400 [thread overview]
Message-ID: <1219883092-6923-1-git-send-email-jlayton@redhat.com> (raw)
The direct I/O write codepath for CIFS is done through
cifs_user_write(). That function does not currently call
generic_write_checks() so the file position isn't being properly set
when the file is opened with O_APPEND. It's also not doing the other
"normal" checks that should be done for a write call.
The problem is currently that when you open a file with O_APPEND on a
mount with the directio mount option, the file position is set to the
beginning of the file. This makes any subsequent writes clobber the data
in the file starting at the beginning.
This seems to fix the problem in cursory testing. It is, however
important to note that NFS disallows the combination of
(O_DIRECT|O_APPEND). If my understanding is correct, the concern is
races with multiple clients appending to a file clobbering each others'
data. Since the write model for CIFS and NFS is pretty similar in this
regard, CIFS is probably subject to the same sort of races. What's
unclear to me is why this is a particular problem with O_DIRECT and not
with buffered writes...
Regardless, disallowing O_APPEND on an entire mount is probably not
reasonable, so we'll probably just have to deal with it and reevaluate
this flag combination when we get proper support for O_DIRECT. In the
meantime this patch at least fixes the existing problem.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
fs/cifs/file.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index ff14d14..cbefe1f 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -833,6 +833,10 @@ ssize_t cifs_user_write(struct file *file, const char __user *write_data,
return -EBADF;
open_file = (struct cifsFileInfo *) file->private_data;
+ rc = generic_write_checks(file, poffset, &write_size, 0);
+ if (rc)
+ return rc;
+
xid = GetXid();
if (*poffset > file->f_path.dentry->d_inode->i_size)
--
1.5.5.1
next reply other threads:[~2008-08-28 0:27 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-08-28 0:24 Jeff Layton [this message]
2008-08-28 2:24 ` [PATCH] cifs: fix O_APPEND on directio mounts Steve French
2008-08-28 11:06 ` Jeff Layton
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=1219883092-6923-1-git-send-email-jlayton@redhat.com \
--to=jlayton@redhat.com \
--cc=linux-cifs-client@lists.samba.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=smfrench@austin.rr.com \
--cc=smfrench@gmail.com \
/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).