mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
synced 2026-01-11 17:10:13 +00:00
c91d38b57f
Jan Kara reported that the shared ILOCK held across the journal flush during fdatasync operations slows down O_DSYNC DIO on unwritten extents significantly. The underlying issue is that unwritten extent conversion needs the ILOCK exclusive, whilst the datasync operation after the extent conversion holds it shared. Hence we cannot be flushing the journal for one IO completion whilst at the same time doing unwritten extent conversion on another IO completion on the same inode. This means that IO completions lock-step, and IO performance is dependent on the journal flush latency. Jan demonstrated that reducing the ifdatasync lock hold time can improve O_DSYNC DIO to unwritten extents performance by 2.5x. Discussion on that patch found issues with the method, and we came to the conclusion that separately tracking datasync flush sequences was the best approach to solving the problem. The fsync code uses the ILOCK to serialise against concurrent modifications in the transaction commit phase. In a transaction commit, there are several disjoint updates to inode log item state that need to be considered atomically by the fsync code. These operations are all done under ILOCK_EXCL context: 1. ili_fsync_flags is updated in ->iop_precommit 2. i_pincount is updated in ->iop_pin before it is added to the CIL 3. ili_commit_seq is updated in ->iop_committing, after it has been added to the CIL In fsync, we need to: 1. check that the inode is dirty in the journal (ipincount) 2. check that ili_fsync_flags is set 3. grab the ili_commit_seq if a journal flush is needed 4. clear the ili_fsync_flags to ensure that new modifications that require fsync are tracked in ->iop_precommit correctly The serialisation of ipincount/ili_commit_seq is needed to ensure that we don't try to unnecessarily flush the journal. The serialisation of ili_fsync_flags being set in ->iop_precommit and cleared in fsync post journal flush is required for correctness. Hence holding the ILOCK_SHARED in xfs_file_fsync() performs all this serialisation for us. Ideally, we want to remove the need to hold the ILOCK_SHARED in xfs_file_fsync() for best performance. We start with the observation that fsync/fdatasync() only need to wait for operations that have been completed. Hence operations that are still being committed have not completed and datasync operations do not need to wait for them. This means we can use a single point in time in the commit process to signal "this modification is complete". This is what ->iop_committing is supposed to provide - it is the point at which the object is unlocked after the modification has been recorded in the CIL. Hence we could use ili_commit_seq to determine if we should flush the journal. In theory, we can already do this. However, in practice this will expose an internal global CIL lock to the IO path. The ipincount() checks optimise away the need to take this lock - if the inode is not pinned, then it is not in the CIL and we don't need to check if a journal flush at ili_commit_seq needs to be performed. The reason this is needed is that the ili_commit_seq is never cleared. Once it is set, it remains set even once the journal has been committed and the object has been unpinned. Hence we have to look that journal internal commit sequence state to determine if ili_commit_seq needs to be acted on or not. We can solve this by clearing ili_commit_seq when the inode is unpinned. If we clear it atomically with the last unpin going away, then we are guaranteed that new modifications will order correctly as they add a new pin counts and we won't clear a sequence number for an active modification in the CIL. Further, we can then allow the per-transaction flag state to propagate into ->iop_committing (instead of clearing it in ->iop_precommit) and that will allow us to determine if the modification needs a full fsync or just a datasync, and so we can record a separate datasync sequence number (Jan's idea!) and then use that in the fdatasync path instead of the full fsync sequence number. With this infrastructure in place, we no longer need the ILOCK_SHARED in the fsync path. All serialisation is done against the commit sequence numbers - if the sequence number is set, then we have to flush the journal. If it is not set, then we have nothing to do. This greatly simplifies the fsync implementation.... Signed-off-by: Dave Chinner <dchinner@redhat.com> Tested-by: Jan Kara <jack@suse.cz> Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Carlos Maiolino <cem@kernel.org>
63 lines
2.2 KiB
C
63 lines
2.2 KiB
C
// SPDX-License-Identifier: GPL-2.0
|
|
/*
|
|
* Copyright (c) 2000,2005 Silicon Graphics, Inc.
|
|
* All Rights Reserved.
|
|
*/
|
|
#ifndef __XFS_INODE_ITEM_H__
|
|
#define __XFS_INODE_ITEM_H__
|
|
|
|
/* kernel only definitions */
|
|
|
|
struct xfs_buf;
|
|
struct xfs_bmbt_rec;
|
|
struct xfs_inode;
|
|
struct xfs_mount;
|
|
|
|
struct xfs_inode_log_item {
|
|
struct xfs_log_item ili_item; /* common portion */
|
|
struct xfs_inode *ili_inode; /* inode ptr */
|
|
unsigned short ili_lock_flags; /* inode lock flags */
|
|
unsigned int ili_dirty_flags; /* dirty in current tx */
|
|
/*
|
|
* The ili_lock protects the interactions between the dirty state and
|
|
* the flush state of the inode log item. This allows us to do atomic
|
|
* modifications of multiple state fields without having to hold a
|
|
* specific inode lock to serialise them.
|
|
*
|
|
* We need atomic changes between inode dirtying, inode flushing and
|
|
* inode completion, but these all hold different combinations of
|
|
* ILOCK and IFLUSHING and hence we need some other method of
|
|
* serialising updates to the flush state.
|
|
*/
|
|
spinlock_t ili_lock; /* flush state lock */
|
|
unsigned int ili_last_fields; /* fields when flushed */
|
|
unsigned int ili_fields; /* fields to be logged */
|
|
xfs_lsn_t ili_flush_lsn; /* lsn at last flush */
|
|
|
|
/*
|
|
* We record the sequence number for every inode modification, as
|
|
* well as those that only require fdatasync operations for data
|
|
* integrity. This allows optimisation of the O_DSYNC/fdatasync path
|
|
* without needing to track what modifications the journal is currently
|
|
* carrying for the inode. These are protected by the above ili_lock.
|
|
*/
|
|
xfs_csn_t ili_commit_seq; /* last transaction commit */
|
|
xfs_csn_t ili_datasync_seq; /* for datasync optimisation */
|
|
};
|
|
|
|
static inline int xfs_inode_clean(struct xfs_inode *ip)
|
|
{
|
|
return !ip->i_itemp || !(ip->i_itemp->ili_fields & XFS_ILOG_ALL);
|
|
}
|
|
|
|
extern void xfs_inode_item_init(struct xfs_inode *, struct xfs_mount *);
|
|
extern void xfs_inode_item_destroy(struct xfs_inode *);
|
|
extern void xfs_iflush_abort(struct xfs_inode *);
|
|
extern void xfs_iflush_shutdown_abort(struct xfs_inode *);
|
|
int xfs_inode_item_format_convert(struct kvec *buf,
|
|
struct xfs_inode_log_format *in_f);
|
|
|
|
extern struct kmem_cache *xfs_ili_cache;
|
|
|
|
#endif /* __XFS_INODE_ITEM_H__ */
|