Message ID | x49d37tj2ji.fsf@segfault.boston.devel.redhat.com |
---|---|
State | Not Applicable, archived |
Headers | show |
Jeff Moyer <jmoyer@redhat.com> writes: > Jeff Moyer <jmoyer@redhat.com> writes: > >> Hi, Dave, >> >> Thanks for the review! >> >>> or better still, factor xfs_file_fsync() so that it calls a helper >>> that doesn't wait for data IO completion, and call that helper here >>> too. The semantics of fsync/fdatasync are too complex to have to >>> implement and maintain in multiple locations.... >> >> I definitely agree with consolidating things. However, there are four >> blocking calls in xfs_file_fsync (filemap_write_and_wait_range, >> xfs_blkdev_issue_flush, _xfs_log_force_lsn, and another call to >> xfs_blkdev_issue_flush). How would you propose to make that >> non-blocking given that those steps have to happen in sequence? > > OK, so re-reading your mail, I think you meant to just factor out > everything except the filemap_write_and_wait_range. Here are a couple > of patches which do that. Also, since we're not worried about blocking > in the endio processing, just making things synchronous makes the code a > lot simpler. Let me know what you think of the attached two patches > (which I've already run through xfstests). Dave, ping? Did you have a chance to take a look at these patches? Cheers, 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 54a67dd..e63030f 100644 --- a/fs/xfs/xfs_file.c +++ b/fs/xfs/xfs_file.c @@ -153,25 +153,18 @@ xfs_dir_fsync( return _xfs_log_force_lsn(mp, lsn, XFS_LOG_SYNC, NULL); } +/* + * Returns 0 on success, -errno on failure. + */ STATIC int -xfs_file_fsync( - struct file *file, - loff_t start, - loff_t end, +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); @@ -223,6 +216,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,