Message ID | 20121120074138.24645.36708.stgit@blackbox.djwong.org |
---|---|
State | Accepted, archived |
Headers | show |
On Mon, Nov 19, 2012 at 11:41:38PM -0800, Darrick J. Wong wrote: > Hi, > > Fsyncing is tricky business, so factor out the bits of the xfs_file_fsync > function that can be used from the I/O post-processing path. > > From: Jeff Moyer <jmoyer@redhat.com> > Signed-off-by: Jeff Moyer <jmoyer@redhat.com> > --- > fs/xfs/xfs_file.c | 44 +++++++++++++++++++++++++++++--------------- > fs/xfs/xfs_inode.h | 1 + > 2 files changed, 30 insertions(+), 15 deletions(-) > > > diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c > index aa473fa..507f446 100644 > --- a/fs/xfs/xfs_file.c > +++ b/fs/xfs/xfs_file.c > @@ -152,25 +152,18 @@ xfs_dir_fsync( > return _xfs_log_force_lsn(mp, lsn, XFS_LOG_SYNC, NULL); > } > > -STATIC int > -xfs_file_fsync( > - struct file *file, > - loff_t start, > - loff_t end, > +/* > + * Returns 0 on success, -errno on failure. > + */ > +int > +do_xfs_file_fsync( > + struct xfs_inode *ip, > + struct xfs_mount *mp, > int datasync) xfs_do_file_fsync() And being an internal XFS function, it should return a positive error number. .... > +{ > + struct inode *inode = file->f_mapping->host; > + struct xfs_inode *ip = XFS_I(inode); > + struct xfs_mount *mp = ip->i_mount; > + int error = 0; > + > + trace_xfs_file_fsync(ip); > + > + error = filemap_write_and_wait_range(inode->i_mapping, start, end); > + if (error) > + return error; > + > + return do_xfs_file_fsync(ip, mp, datasync); And be negated here. > +} > + > STATIC ssize_t > xfs_file_aio_read( > struct kiocb *iocb, > diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h > index 94b32f9..d5bf105 100644 > --- a/fs/xfs/xfs_inode.h > +++ b/fs/xfs/xfs_inode.h > @@ -546,6 +546,7 @@ do { \ > iput(VFS_I(ip)); \ > } while (0) > > +int do_xfs_file_fsync(struct xfs_inode *, struct xfs_mount *, int); > #endif /* __KERNEL__ */ This should probably go in fs/xfs/xfs_vnodeops.h (like the only other non-static function (xfs_zero_eof) in fs/xfs/xfs_file.c is. Cheers, Dave.
On Mon, Nov 19, 2012 at 11:41:38PM -0800, Darrick J. Wong wrote: > Hi, > > Fsyncing is tricky business, so factor out the bits of the xfs_file_fsync > function that can be used from the I/O post-processing path. Why would we need to skip the filemap_write_and_wait_range call here? If we're doing direct I/O we should not have any pages in this regions anyway. You're also not skipping it in the generic implementation as far as I can see, so I see no point in doing it just in XFS. -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Christoph Hellwig <hch@infradead.org> writes: > On Mon, Nov 19, 2012 at 11:41:38PM -0800, Darrick J. Wong wrote: >> Hi, >> >> Fsyncing is tricky business, so factor out the bits of the xfs_file_fsync >> function that can be used from the I/O post-processing path. > > Why would we need to skip the filemap_write_and_wait_range call here? > If we're doing direct I/O we should not have any pages in this regions > anyway. You're also not skipping it in the generic implementation as > far as I can see, so I see no point in doing it just in XFS. OK, I'll fix that. Thanks! Jeff -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c index aa473fa..507f446 100644 --- a/fs/xfs/xfs_file.c +++ b/fs/xfs/xfs_file.c @@ -152,25 +152,18 @@ xfs_dir_fsync( return _xfs_log_force_lsn(mp, lsn, XFS_LOG_SYNC, NULL); } -STATIC int -xfs_file_fsync( - struct file *file, - loff_t start, - loff_t end, +/* + * Returns 0 on success, -errno on failure. + */ +int +do_xfs_file_fsync( + struct xfs_inode *ip, + struct xfs_mount *mp, int datasync) { - struct inode *inode = file->f_mapping->host; - struct xfs_inode *ip = XFS_I(inode); - struct xfs_mount *mp = ip->i_mount; - int error = 0; int log_flushed = 0; xfs_lsn_t lsn = 0; - - trace_xfs_file_fsync(ip); - - error = filemap_write_and_wait_range(inode->i_mapping, start, end); - if (error) - return error; + int error = 0; if (XFS_FORCED_SHUTDOWN(mp)) return -XFS_ERROR(EIO); @@ -222,6 +215,27 @@ xfs_file_fsync( return -error; } +STATIC int +xfs_file_fsync( + struct file *file, + loff_t start, + loff_t end, + int datasync) +{ + struct inode *inode = file->f_mapping->host; + struct xfs_inode *ip = XFS_I(inode); + struct xfs_mount *mp = ip->i_mount; + int error = 0; + + trace_xfs_file_fsync(ip); + + error = filemap_write_and_wait_range(inode->i_mapping, start, end); + if (error) + return error; + + return do_xfs_file_fsync(ip, mp, datasync); +} + STATIC ssize_t xfs_file_aio_read( struct kiocb *iocb, diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h index 94b32f9..d5bf105 100644 --- a/fs/xfs/xfs_inode.h +++ b/fs/xfs/xfs_inode.h @@ -546,6 +546,7 @@ do { \ iput(VFS_I(ip)); \ } while (0) +int do_xfs_file_fsync(struct xfs_inode *, struct xfs_mount *, int); #endif /* __KERNEL__ */ /*