Just cc'ing Alan Stern here, who wrote the usb/gadget code which
this simplifies.
On Monday 22 December 2008, Christoph Hellwig wrote:
> 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.
>
> [and now actually export vfs_fsync]
>
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
>
> Index: xfs/fs/nfsd/vfs.c
> ===================================================================
> --- xfs.orig/fs/nfsd/vfs.c 2008-12-22 18:01:32.367372702 +0100
> +++ xfs/fs/nfsd/vfs.c 2008-12-22 18:13:29.210247796 +0100
> @@ -743,45 +743,16 @@ nfsd_close(struct file *filp)
> 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, 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-22 18:01:32.372372501 +0100
> +++ xfs/fs/sync.c 2008-12-22 18:14:30.534247267 +0100
> @@ -75,14 +75,39 @@ int file_fsync(struct file *filp, struct
> 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;
> + const 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 @@ long do_fsync(struct file *file, int dat
> * 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);
> @@ -104,15 +129,16 @@ long do_fsync(struct file *file, int dat
> out:
> return ret;
> }
> +EXPORT_SYMBOL(vfs_fsync);
>
> -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 +146,12 @@ static long __do_fsync(unsigned int fd,
>
> 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-22 18:01:32.391372407 +0100
> +++ xfs/include/linux/fs.h 2008-12-22 18:01:53.623372207 +0100
> @@ -1827,7 +1827,7 @@ extern int __filemap_fdatawrite_range(st
> 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-22 18:01:32.396372765 +0100
> +++ xfs/mm/msync.c 2008-12-22 18:01:53.624372893 +0100
> @@ -82,7 +82,7 @@ asmlinkage long sys_msync(unsigned long
> (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-22 18:01:32.405372585 +0100
> +++ xfs/drivers/usb/gadget/file_storage.c 2008-12-22 18:20:54.054277912 +0100
> @@ -1863,26 +1863,10 @@ static int do_write(struct fsg_dev *fsg)
> 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, 1);
> }
>
> static void fsync_all(struct fsg_dev *fsg)
> Index: xfs/fs/coda/file.c
> ===================================================================
> --- xfs.orig/fs/coda/file.c 2008-12-22 18:01:32.379372066 +0100
> +++ xfs/fs/coda/file.c 2008-12-22 18:01:53.626372520 +0100
> @@ -200,8 +200,7 @@ int coda_release(struct inode *coda_inod
> 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 @@ int coda_fsync(struct file *coda_file, s
> 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-22 18:01:32.385372900 +0100
> +++ xfs/fs/ecryptfs/file.c 2008-12-22 18:01:53.627372577 +0100
> @@ -275,18 +275,9 @@ static int ecryptfs_release(struct inode
> 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)
>
>
--
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
