Commit 2931cdcb authored by Darrick J. Wong's avatar Darrick J. Wong Committed by Linus Torvalds

ocfs2: improve fsync efficiency and fix deadlock between aio_write and sync_file

Currently, ocfs2_sync_file grabs i_mutex and forces the current journal
transaction to complete.  This isn't terribly efficient, since sync_file
really only needs to wait for the last transaction involving that inode
to complete, and this doesn't require i_mutex.

Therefore, implement the necessary bits to track the newest tid
associated with an inode, and teach sync_file to wait for that instead
of waiting for everything in the journal to commit.  Furthermore, only
issue the flush request to the drive if jbd2 hasn't already done so.

This also eliminates the deadlock between ocfs2_file_aio_write() and
ocfs2_sync_file().  aio_write takes i_mutex then calls
ocfs2_aiodio_wait() to wait for unaligned dio writes to finish.
However, if that dio completion involves calling fsync, then we can get
into trouble when some ocfs2_sync_file tries to take i_mutex.
Signed-off-by: default avatarDarrick J. Wong <darrick.wong@oracle.com>
Reviewed-by: default avatarMark Fasheh <mfasheh@suse.de>
Cc: Joel Becker <jlbec@evilplan.org>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent a75fe48c
...@@ -6932,6 +6932,7 @@ int ocfs2_convert_inline_data_to_extents(struct inode *inode, ...@@ -6932,6 +6932,7 @@ int ocfs2_convert_inline_data_to_extents(struct inode *inode,
di->i_dyn_features = cpu_to_le16(oi->ip_dyn_features); di->i_dyn_features = cpu_to_le16(oi->ip_dyn_features);
spin_unlock(&oi->ip_lock); spin_unlock(&oi->ip_lock);
ocfs2_update_inode_fsync_trans(handle, inode, 1);
ocfs2_dinode_new_extent_list(inode, di); ocfs2_dinode_new_extent_list(inode, di);
ocfs2_journal_dirty(handle, di_bh); ocfs2_journal_dirty(handle, di_bh);
......
...@@ -2039,6 +2039,7 @@ int ocfs2_write_end_nolock(struct address_space *mapping, ...@@ -2039,6 +2039,7 @@ int ocfs2_write_end_nolock(struct address_space *mapping,
inode->i_mtime = inode->i_ctime = CURRENT_TIME; inode->i_mtime = inode->i_ctime = CURRENT_TIME;
di->i_mtime = di->i_ctime = cpu_to_le64(inode->i_mtime.tv_sec); di->i_mtime = di->i_ctime = cpu_to_le64(inode->i_mtime.tv_sec);
di->i_mtime_nsec = di->i_ctime_nsec = cpu_to_le32(inode->i_mtime.tv_nsec); di->i_mtime_nsec = di->i_ctime_nsec = cpu_to_le32(inode->i_mtime.tv_nsec);
ocfs2_update_inode_fsync_trans(handle, inode, 1);
ocfs2_journal_dirty(handle, wc->w_di_bh); ocfs2_journal_dirty(handle, wc->w_di_bh);
ocfs2_commit_trans(osb, handle); ocfs2_commit_trans(osb, handle);
......
...@@ -2957,6 +2957,7 @@ static int ocfs2_expand_inline_dir(struct inode *dir, struct buffer_head *di_bh, ...@@ -2957,6 +2957,7 @@ static int ocfs2_expand_inline_dir(struct inode *dir, struct buffer_head *di_bh,
ocfs2_init_dir_trailer(dir, dirdata_bh, i); ocfs2_init_dir_trailer(dir, dirdata_bh, i);
} }
ocfs2_update_inode_fsync_trans(handle, dir, 1);
ocfs2_journal_dirty(handle, dirdata_bh); ocfs2_journal_dirty(handle, dirdata_bh);
if (ocfs2_supports_indexed_dirs(osb) && !dx_inline) { if (ocfs2_supports_indexed_dirs(osb) && !dx_inline) {
...@@ -3338,6 +3339,7 @@ static int ocfs2_extend_dir(struct ocfs2_super *osb, ...@@ -3338,6 +3339,7 @@ static int ocfs2_extend_dir(struct ocfs2_super *osb,
} else { } else {
de->rec_len = cpu_to_le16(sb->s_blocksize); de->rec_len = cpu_to_le16(sb->s_blocksize);
} }
ocfs2_update_inode_fsync_trans(handle, dir, 1);
ocfs2_journal_dirty(handle, new_bh); ocfs2_journal_dirty(handle, new_bh);
dir_i_size += dir->i_sb->s_blocksize; dir_i_size += dir->i_sb->s_blocksize;
...@@ -3896,6 +3898,7 @@ static int ocfs2_dx_dir_rebalance(struct ocfs2_super *osb, struct inode *dir, ...@@ -3896,6 +3898,7 @@ static int ocfs2_dx_dir_rebalance(struct ocfs2_super *osb, struct inode *dir,
dquot_free_space_nodirty(dir, dquot_free_space_nodirty(dir,
ocfs2_clusters_to_bytes(dir->i_sb, 1)); ocfs2_clusters_to_bytes(dir->i_sb, 1));
ocfs2_update_inode_fsync_trans(handle, dir, 1);
ocfs2_commit_trans(osb, handle); ocfs2_commit_trans(osb, handle);
out: out:
...@@ -4134,6 +4137,7 @@ static int ocfs2_expand_inline_dx_root(struct inode *dir, ...@@ -4134,6 +4137,7 @@ static int ocfs2_expand_inline_dx_root(struct inode *dir,
mlog_errno(ret); mlog_errno(ret);
did_quota = 0; did_quota = 0;
ocfs2_update_inode_fsync_trans(handle, dir, 1);
ocfs2_journal_dirty(handle, dx_root_bh); ocfs2_journal_dirty(handle, dx_root_bh);
out_commit: out_commit:
......
...@@ -175,9 +175,13 @@ static int ocfs2_sync_file(struct file *file, loff_t start, loff_t end, ...@@ -175,9 +175,13 @@ static int ocfs2_sync_file(struct file *file, loff_t start, loff_t end,
int datasync) int datasync)
{ {
int err = 0; int err = 0;
journal_t *journal;
struct inode *inode = file->f_mapping->host; struct inode *inode = file->f_mapping->host;
struct ocfs2_super *osb = OCFS2_SB(inode->i_sb); struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
struct ocfs2_inode_info *oi = OCFS2_I(inode);
journal_t *journal = osb->journal->j_journal;
int ret;
tid_t commit_tid;
bool needs_barrier = false;
trace_ocfs2_sync_file(inode, file, file->f_path.dentry, trace_ocfs2_sync_file(inode, file, file->f_path.dentry,
OCFS2_I(inode)->ip_blkno, OCFS2_I(inode)->ip_blkno,
...@@ -192,29 +196,19 @@ static int ocfs2_sync_file(struct file *file, loff_t start, loff_t end, ...@@ -192,29 +196,19 @@ static int ocfs2_sync_file(struct file *file, loff_t start, loff_t end,
if (err) if (err)
return err; return err;
/* commit_tid = datasync ? oi->i_datasync_tid : oi->i_sync_tid;
* Probably don't need the i_mutex at all in here, just putting it here if (journal->j_flags & JBD2_BARRIER &&
* to be consistent with how fsync used to be called, someone more !jbd2_trans_will_send_data_barrier(journal, commit_tid))
* familiar with the fs could possibly remove it. needs_barrier = true;
*/ err = jbd2_complete_transaction(journal, commit_tid);
mutex_lock(&inode->i_mutex); if (needs_barrier) {
if (datasync && !(inode->i_state & I_DIRTY_DATASYNC)) { ret = blkdev_issue_flush(inode->i_sb->s_bdev, GFP_KERNEL, NULL);
/* if (!err)
* We still have to flush drive's caches to get data to the err = ret;
* platter
*/
if (osb->s_mount_opt & OCFS2_MOUNT_BARRIER)
blkdev_issue_flush(inode->i_sb->s_bdev, GFP_KERNEL, NULL);
goto bail;
} }
journal = osb->journal->j_journal;
err = jbd2_journal_force_commit(journal);
bail:
if (err) if (err)
mlog_errno(err); mlog_errno(err);
mutex_unlock(&inode->i_mutex);
return (err < 0) ? -EIO : 0; return (err < 0) ? -EIO : 0;
} }
...@@ -650,7 +644,7 @@ static int __ocfs2_extend_allocation(struct inode *inode, u32 logical_start, ...@@ -650,7 +644,7 @@ static int __ocfs2_extend_allocation(struct inode *inode, u32 logical_start,
mlog_errno(status); mlog_errno(status);
goto leave; goto leave;
} }
ocfs2_update_inode_fsync_trans(handle, inode, 1);
ocfs2_journal_dirty(handle, bh); ocfs2_journal_dirty(handle, bh);
spin_lock(&OCFS2_I(inode)->ip_lock); spin_lock(&OCFS2_I(inode)->ip_lock);
......
...@@ -130,6 +130,7 @@ struct inode *ocfs2_iget(struct ocfs2_super *osb, u64 blkno, unsigned flags, ...@@ -130,6 +130,7 @@ struct inode *ocfs2_iget(struct ocfs2_super *osb, u64 blkno, unsigned flags,
struct inode *inode = NULL; struct inode *inode = NULL;
struct super_block *sb = osb->sb; struct super_block *sb = osb->sb;
struct ocfs2_find_inode_args args; struct ocfs2_find_inode_args args;
journal_t *journal = OCFS2_SB(sb)->journal->j_journal;
trace_ocfs2_iget_begin((unsigned long long)blkno, flags, trace_ocfs2_iget_begin((unsigned long long)blkno, flags,
sysfile_type); sysfile_type);
...@@ -169,6 +170,32 @@ struct inode *ocfs2_iget(struct ocfs2_super *osb, u64 blkno, unsigned flags, ...@@ -169,6 +170,32 @@ struct inode *ocfs2_iget(struct ocfs2_super *osb, u64 blkno, unsigned flags,
goto bail; goto bail;
} }
/*
* Set transaction id's of transactions that have to be committed
* to finish f[data]sync. We set them to currently running transaction
* as we cannot be sure that the inode or some of its metadata isn't
* part of the transaction - the inode could have been reclaimed and
* now it is reread from disk.
*/
if (journal) {
transaction_t *transaction;
tid_t tid;
struct ocfs2_inode_info *oi = OCFS2_I(inode);
read_lock(&journal->j_state_lock);
if (journal->j_running_transaction)
transaction = journal->j_running_transaction;
else
transaction = journal->j_committing_transaction;
if (transaction)
tid = transaction->t_tid;
else
tid = journal->j_commit_sequence;
read_unlock(&journal->j_state_lock);
oi->i_sync_tid = tid;
oi->i_datasync_tid = tid;
}
bail: bail:
if (!IS_ERR(inode)) { if (!IS_ERR(inode)) {
trace_ocfs2_iget_end(inode, trace_ocfs2_iget_end(inode,
...@@ -1260,6 +1287,7 @@ int ocfs2_mark_inode_dirty(handle_t *handle, ...@@ -1260,6 +1287,7 @@ int ocfs2_mark_inode_dirty(handle_t *handle,
fe->i_mtime_nsec = cpu_to_le32(inode->i_mtime.tv_nsec); fe->i_mtime_nsec = cpu_to_le32(inode->i_mtime.tv_nsec);
ocfs2_journal_dirty(handle, bh); ocfs2_journal_dirty(handle, bh);
ocfs2_update_inode_fsync_trans(handle, inode, 1);
leave: leave:
return status; return status;
} }
......
...@@ -73,6 +73,13 @@ struct ocfs2_inode_info ...@@ -73,6 +73,13 @@ struct ocfs2_inode_info
u32 ip_dir_lock_gen; u32 ip_dir_lock_gen;
struct ocfs2_alloc_reservation ip_la_data_resv; struct ocfs2_alloc_reservation ip_la_data_resv;
/*
* Transactions that contain inode's metadata needed to complete
* fsync and fdatasync, respectively.
*/
tid_t i_sync_tid;
tid_t i_datasync_tid;
}; };
/* /*
......
...@@ -626,4 +626,15 @@ static inline int ocfs2_begin_ordered_truncate(struct inode *inode, ...@@ -626,4 +626,15 @@ static inline int ocfs2_begin_ordered_truncate(struct inode *inode,
new_size); new_size);
} }
static inline void ocfs2_update_inode_fsync_trans(handle_t *handle,
struct inode *inode,
int datasync)
{
struct ocfs2_inode_info *oi = OCFS2_I(inode);
oi->i_sync_tid = handle->h_transaction->t_tid;
if (datasync)
oi->i_datasync_tid = handle->h_transaction->t_tid;
}
#endif /* OCFS2_JOURNAL_H */ #endif /* OCFS2_JOURNAL_H */
...@@ -495,6 +495,7 @@ static int __ocfs2_mknod_locked(struct inode *dir, ...@@ -495,6 +495,7 @@ static int __ocfs2_mknod_locked(struct inode *dir,
struct ocfs2_dinode *fe = NULL; struct ocfs2_dinode *fe = NULL;
struct ocfs2_extent_list *fel; struct ocfs2_extent_list *fel;
u16 feat; u16 feat;
struct ocfs2_inode_info *oi = OCFS2_I(inode);
*new_fe_bh = NULL; *new_fe_bh = NULL;
...@@ -576,6 +577,9 @@ static int __ocfs2_mknod_locked(struct inode *dir, ...@@ -576,6 +577,9 @@ static int __ocfs2_mknod_locked(struct inode *dir,
mlog_errno(status); mlog_errno(status);
} }
oi->i_sync_tid = handle->h_transaction->t_tid;
oi->i_datasync_tid = handle->h_transaction->t_tid;
status = 0; /* error in ocfs2_create_new_inode_locks is not status = 0; /* error in ocfs2_create_new_inode_locks is not
* critical */ * critical */
......
...@@ -561,6 +561,9 @@ static struct inode *ocfs2_alloc_inode(struct super_block *sb) ...@@ -561,6 +561,9 @@ static struct inode *ocfs2_alloc_inode(struct super_block *sb)
if (!oi) if (!oi)
return NULL; return NULL;
oi->i_sync_tid = 0;
oi->i_datasync_tid = 0;
jbd2_journal_init_jbd_inode(&oi->ip_jinode, &oi->vfs_inode); jbd2_journal_init_jbd_inode(&oi->ip_jinode, &oi->vfs_inode);
return &oi->vfs_inode; return &oi->vfs_inode;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment