From: Christoph Hellwig <hch@lst.de>
To: viro@zeniv.linux.org.uk
Cc: npiggin@suse.de, dbrownell@users.sourceforge.net, agl@us.ibm.com,
ebiederm@xmission.com, linux-fsdevel@vger.kernel.org,
ecryptfs-devel@lists.launchpad.net, jaharkes@cs.cmu.edu
Subject: [PATCH, RFC] add a vfs_fsync helper
Date: Mon, 22 Dec 2008 10:03:57 +0100 [thread overview]
Message-ID: <20081222090357.GA27399@lst.de> (raw)
Fsync currently has a fdatawrite/fdatawait pair around the method call,
and a mutex_lock/unlock of the inode mutex. All callers of fsync have
to duplicate this, but we have a few and most of them don't quite get
it right. This patch adds a new vfs_fsync that takes care of this.
It's a little more complicated as usual as ->fsync might get a NULL file
pointer and just a dentry from nfsd, but otherwise gets afile and we
want to take the mapping and file operations from it when it is there.
Notes on the fsync callers:
- ecryptfs wasn't calling filemap_fdatawrite / filemap_fdatawait on the
lower file
- coda wasn't calling filemap_fdatawrite / filemap_fdatawait on the host
file, and returning 0 when ->fsync was missing
- shm wasn't calling either filemap_fdatawrite / filemap_fdatawait nor
taking i_mutex. Now given that shared memory doesn't have disk
backing not doing anything in fsync seems fine and I left it out of
the vfs_fsync conversion for now, but in that case we might just
not pass it through to the lower file at all but just call the no-op
simple_sync_file directly.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Index: xfs/fs/nfsd/vfs.c
===================================================================
--- xfs.orig/fs/nfsd/vfs.c 2008-12-19 15:02:53.816810089 +0100
+++ xfs/fs/nfsd/vfs.c 2008-12-21 12:22:17.178896598 +0100
@@ -743,45 +743,16 @@
fput(filp);
}
-/*
- * Sync a file
- * As this calls fsync (not fdatasync) there is no need for a write_inode
- * after it.
- */
-static inline int nfsd_dosync(struct file *filp, struct dentry *dp,
- const struct file_operations *fop)
-{
- struct inode *inode = dp->d_inode;
- int (*fsync) (struct file *, struct dentry *, int);
- int err;
-
- err = filemap_fdatawrite(inode->i_mapping);
- if (err == 0 && fop && (fsync = fop->fsync))
- err = fsync(filp, dp, 0);
- if (err == 0)
- err = filemap_fdatawait(inode->i_mapping);
-
- return err;
-}
-
-
static int
nfsd_sync(struct file *filp)
{
- int err;
- struct inode *inode = filp->f_path.dentry->d_inode;
- dprintk("nfsd: sync file %s\n", filp->f_path.dentry->d_name.name);
- mutex_lock(&inode->i_mutex);
- err=nfsd_dosync(filp, filp->f_path.dentry, filp->f_op);
- mutex_unlock(&inode->i_mutex);
-
- return err;
+ return vfs_fsync(filp, filp->f_path.dentry->d_inode, 0);
}
int
-nfsd_sync_dir(struct dentry *dp)
+nfsd_sync_dir(struct dentry *dentry)
{
- return nfsd_dosync(NULL, dp, dp->d_inode->i_fop);
+ return vfs_fsync(NULL, dentry, 0);
}
/*
Index: xfs/fs/sync.c
===================================================================
--- xfs.orig/fs/sync.c 2008-12-19 15:02:53.942907780 +0100
+++ xfs/fs/sync.c 2008-12-21 12:22:17.180896085 +0100
@@ -75,14 +75,39 @@
return ret;
}
-long do_fsync(struct file *file, int datasync)
+/**
+ * vfs_fsync - perform a fsync or fdatasync on a file
+ * @file: file to sync
+ * @dentry: dentry of @file
+ * @data: only perform a fdatasync operation
+ *
+ * Write back data and metadata for @file to disk. If @datasync is
+ * set only metadata needed to access modified file data is written.
+ *
+ * In case this function is called from nfsd @file may be %NULL and
+ * only @dentry is set. This can only happen when the filesystem
+ * implements the export_operations API.
+ */
+int vfs_fsync(struct file *file, struct dentry *dentry, int datasync)
{
- int ret;
- int err;
- struct address_space *mapping = file->f_mapping;
+ struct file_operations *fop;
+ struct address_space *mapping;
+ int err, ret;
+
+ /*
+ * Get mapping and operations from the file in case we have
+ * as file, or get the default values for them in case we
+ * don't have a struct file available. Damn nfsd..
+ */
+ if (file) {
+ mapping = file->f_mapping;
+ fop = file->f_op;
+ } else {
+ mapping = dentry->d_inode->i_mapping;
+ fop = dentry->d_inode->i_fop;
+ }
- if (!file->f_op || !file->f_op->fsync) {
- /* Why? We can still call filemap_fdatawrite */
+ if (!fop || !fop->fsync) {
ret = -EINVAL;
goto out;
}
@@ -94,7 +119,7 @@
* livelocks in fsync_buffers_list().
*/
mutex_lock(&mapping->host->i_mutex);
- err = file->f_op->fsync(file, file->f_path.dentry, datasync);
+ err = fop->fsync(file, dentry, datasync);
if (!ret)
ret = err;
mutex_unlock(&mapping->host->i_mutex);
@@ -105,14 +130,14 @@
return ret;
}
-static long __do_fsync(unsigned int fd, int datasync)
+static int do_fsync(unsigned int fd, int datasync)
{
struct file *file;
int ret = -EBADF;
file = fget(fd);
if (file) {
- ret = do_fsync(file, datasync);
+ ret = vfs_fsync(file, file->f_path.dentry, datasync);
fput(file);
}
return ret;
@@ -120,12 +145,12 @@
asmlinkage long sys_fsync(unsigned int fd)
{
- return __do_fsync(fd, 0);
+ return do_fsync(fd, 0);
}
asmlinkage long sys_fdatasync(unsigned int fd)
{
- return __do_fsync(fd, 1);
+ return do_fsync(fd, 1);
}
/*
Index: xfs/include/linux/fs.h
===================================================================
--- xfs.orig/include/linux/fs.h 2008-12-19 15:02:54.427785087 +0100
+++ xfs/include/linux/fs.h 2008-12-21 12:22:17.187054621 +0100
@@ -1826,7 +1826,7 @@
extern int filemap_fdatawrite_range(struct address_space *mapping,
loff_t start, loff_t end);
-extern long do_fsync(struct file *file, int datasync);
+extern int vfs_fsync(struct file *file, struct dentry *dentry, int datasync);
extern void sync_supers(void);
extern void sync_filesystems(int wait);
extern void __fsync_super(struct super_block *sb);
Index: xfs/mm/msync.c
===================================================================
--- xfs.orig/mm/msync.c 2008-12-19 15:02:56.646810993 +0100
+++ xfs/mm/msync.c 2008-12-21 12:22:17.188896406 +0100
@@ -82,7 +82,7 @@
(vma->vm_flags & VM_SHARED)) {
get_file(file);
up_read(&mm->mmap_sem);
- error = do_fsync(file, 0);
+ error = vfs_fsync(file, file->f_path.dentry, 0);
fput(file);
if (error || start >= end)
goto out;
Index: xfs/drivers/usb/gadget/file_storage.c
===================================================================
--- xfs.orig/drivers/usb/gadget/file_storage.c 2008-12-21 12:26:53.254894542 +0100
+++ xfs/drivers/usb/gadget/file_storage.c 2008-12-21 12:28:08.914896191 +0100
@@ -1863,26 +1863,10 @@
static int fsync_sub(struct lun *curlun)
{
struct file *filp = curlun->filp;
- struct inode *inode;
- int rc, err;
if (curlun->ro || !filp)
return 0;
- if (!filp->f_op->fsync)
- return -EINVAL;
-
- inode = filp->f_path.dentry->d_inode;
- mutex_lock(&inode->i_mutex);
- rc = filemap_fdatawrite(inode->i_mapping);
- err = filp->f_op->fsync(filp, filp->f_path.dentry, 1);
- if (!rc)
- rc = err;
- err = filemap_fdatawait(inode->i_mapping);
- if (!rc)
- rc = err;
- mutex_unlock(&inode->i_mutex);
- VLDBG(curlun, "fdatasync -> %d\n", rc);
- return rc;
+ return vfs_fsync(filp, filp->f_path.dentry->d_inode, 1);
}
static void fsync_all(struct fsg_dev *fsg)
Index: xfs/fs/coda/file.c
===================================================================
--- xfs.orig/fs/coda/file.c 2008-12-21 12:28:26.575894132 +0100
+++ xfs/fs/coda/file.c 2008-12-21 12:31:32.556928596 +0100
@@ -200,8 +200,7 @@
int coda_fsync(struct file *coda_file, struct dentry *coda_dentry, int datasync)
{
struct file *host_file;
- struct dentry *host_dentry;
- struct inode *host_inode, *coda_inode = coda_dentry->d_inode;
+ struct inode *coda_inode = coda_dentry->d_inode;
struct coda_file_info *cfi;
int err = 0;
@@ -213,14 +212,7 @@
BUG_ON(!cfi || cfi->cfi_magic != CODA_MAGIC);
host_file = cfi->cfi_container;
- if (host_file->f_op && host_file->f_op->fsync) {
- host_dentry = host_file->f_path.dentry;
- host_inode = host_dentry->d_inode;
- mutex_lock(&host_inode->i_mutex);
- err = host_file->f_op->fsync(host_file, host_dentry, datasync);
- mutex_unlock(&host_inode->i_mutex);
- }
-
+ err = vfs_fsync(host_file, host_file->f_path.dentry, datasync);
if ( !err && !datasync ) {
lock_kernel();
err = venus_fsync(coda_inode->i_sb, coda_i2f(coda_inode));
Index: xfs/fs/ecryptfs/file.c
===================================================================
--- xfs.orig/fs/ecryptfs/file.c 2008-12-21 12:31:51.618018979 +0100
+++ xfs/fs/ecryptfs/file.c 2008-12-21 12:34:31.203894170 +0100
@@ -275,18 +275,9 @@
static int
ecryptfs_fsync(struct file *file, struct dentry *dentry, int datasync)
{
- struct file *lower_file = ecryptfs_file_to_lower(file);
- struct dentry *lower_dentry = ecryptfs_dentry_to_lower(dentry);
- struct inode *lower_inode = lower_dentry->d_inode;
- int rc = -EINVAL;
-
- if (lower_inode->i_fop->fsync) {
- mutex_lock(&lower_inode->i_mutex);
- rc = lower_inode->i_fop->fsync(lower_file, lower_dentry,
- datasync);
- mutex_unlock(&lower_inode->i_mutex);
- }
- return rc;
+ return vfs_fsync(ecryptfs_file_to_lower(file),
+ ecryptfs_dentry_to_lower(dentry),
+ datasync);
}
static int ecryptfs_fasync(int fd, struct file *file, int flag)
next reply other threads:[~2008-12-22 9:04 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-12-22 9:03 Christoph Hellwig [this message]
2008-12-22 12:35 ` [PATCH, RFC] add a vfs_fsync helper Nick Piggin
2008-12-22 12:56 ` Christoph Hellwig
2008-12-22 13:00 ` Nick Piggin
2008-12-22 13:25 ` Al Viro
2008-12-22 20:11 ` [PATCH] " Christoph Hellwig
2008-12-22 20:42 ` David Brownell
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=20081222090357.GA27399@lst.de \
--to=hch@lst.de \
--cc=agl@us.ibm.com \
--cc=dbrownell@users.sourceforge.net \
--cc=ebiederm@xmission.com \
--cc=ecryptfs-devel@lists.launchpad.net \
--cc=jaharkes@cs.cmu.edu \
--cc=linux-fsdevel@vger.kernel.org \
--cc=npiggin@suse.de \
--cc=viro@zeniv.linux.org.uk \
/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).