Skip to content

Commit 4613b17

Browse files
author
Darrick J. Wong
committed
Merge tag 'xfs-iunlink-item-5.20' of git://git.kernel.org/pub/scm/linux/kernel/git/dgc/linux-xfs into xfs-5.20-mergeB
xfs: introduce in-memory inode unlink log items To facilitate future improvements in inode logging and improving inode cluster buffer locking order consistency, we need a new mechanism for defering inode cluster buffer modifications during unlinked list modifications. The unlinked inode list buffer locking is complex. The unlinked list is unordered - we add to the tail, remove from where-ever the inode is in the list. Hence we might need to lock two inode buffers here (previous inode in list and the one being removed). While we can order the locking of these buffers correctly within the confines of the unlinked list, there may be other inodes that need buffer locking in the same transaction. e.g. O_TMPFILE being linked into a directory also modifies the directory inode. Hence we need a mechanism for defering unlinked inode list updates until a point where we know that all modifications have been made and all that remains is to lock and modify the cluster buffers. We can do this by first observing that we serialise unlinked list modifications by holding the AGI buffer lock. IOWs, the AGI is going to be locked until the transaction commits any time we modify the unlinked list. Hence it doesn't matter when in the unlink transactions that we actually load, lock and modify the inode cluster buffer. We add an in-memory unlinked inode log item to defer the inode cluster buffer update to transaction commit time where it can be ordered with all the other inode cluster operations that need to be done. Essentially all we need to do is record the inodes that need to have their unlinked list pointer updated in a new log item that we attached to the transaction. This log item exists purely for the purpose of delaying the update of the unlinked list pointer until the inode cluster buffer can be locked in the correct order around the other inode cluster buffers. It plays no part in the actual commit, and there's no change to anything that is written to the log. i.e. the inode cluster buffers still have to be fully logged here (not just ordered) as log recovery depedends on this to replay mods to the unlinked inode list. Hence if we add a "precommit" hook into xfs_trans_commit() to run a "precommit" operation on these iunlink log items, we can delay the locking, modification and logging of the inode cluster buffer until after all other modifications have been made. The precommit hook reuires us to sort the items that are going to be run so that we can lock precommit items in the correct order as we perform the modifications they describe. To make this unlinked inode list processing simpler and easier to implement as a log item, we need to change the way we track the unlinked list in memory. Starting from the observation that an inode on the unlinked list is pinned in memory by the VFS, we can use the xfs_inode itself to track the unlinked list. To do this efficiently, we want the unlinked list to be a double linked list. The problem here is that we need a list per AGI unlinked list, and there are 64 of these per AGI. The approach taken in this patchset is to shadow the AGI unlinked list heads in the perag, and link inodes by agino, hence requiring only 8 extra bytes per inode to track this state. We can then use the agino pointers for lockless inode cache lookups to retreive the inode. The aginos in the inode are modified only under the AGI lock, just like the cluster buffer pointers, so we don't need any extra locking here. The i_next_unlinked field tracks the on-disk value of the unlinked list, and the i_prev_unlinked is a purely in-memory pointer that enables us to efficiently remove inodes from the middle of the list. This results in moving a lot of the unlink modification work into the precommit operations on the unlink log item. Tracking all the unlinked inodes in the inodes themselves also gets rid of the unlinked list reference hash table that is used to track this back pointer relationship. This greatly simplifies the the unlinked list modification code, and removes memory allocations in this hot path to track back pointers. This, overall, slightly reduces the CPU overhead of the unlink path. The result of this log item means that we move all the actual manipulation of objects to be logged out of the iunlink path and into the iunlink item. This allows for future optimisation of this mechanism without needing changes to high level unlink path, as well as making the unlink lock ordering predictable and synchronised with other operations that may require inode cluster locking. Signed-off-by: Dave Chinner <[email protected]> Signed-off-by: Darrick J. Wong <[email protected]> * tag 'xfs-iunlink-item-5.20' of git://git.kernel.org/pub/scm/linux/kernel/git/dgc/linux-xfs: xfs: add in-memory iunlink log item xfs: add log item precommit operation xfs: combine iunlink inode update functions xfs: clean up xfs_iunlink_update_inode() xfs: double link the unlinked inode list xfs: introduce xfs_iunlink_lookup xfs: refactor xlog_recover_process_iunlinks() xfs: track the iunlink list pointer in the xfs_inode xfs: factor the xfs_iunlink functions xfs: flush inode gc workqueue before clearing agi bucket
2 parents 0f38063 + 784eb7d commit 4613b17

14 files changed

+539
-545
lines changed

fs/xfs/Makefile

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ xfs-y += xfs_log.o \
106106
xfs_icreate_item.o \
107107
xfs_inode_item.o \
108108
xfs_inode_item_recover.o \
109+
xfs_iunlink_item.o \
109110
xfs_refcount_item.o \
110111
xfs_rmap_item.o \
111112
xfs_log_recover.o \

fs/xfs/libxfs/xfs_ag.c

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,6 @@ xfs_free_perag(
194194
XFS_IS_CORRUPT(pag->pag_mount, atomic_read(&pag->pag_ref) != 0);
195195

196196
cancel_delayed_work_sync(&pag->pag_blockgc_work);
197-
xfs_iunlink_destroy(pag);
198197
xfs_buf_hash_destroy(pag);
199198

200199
call_rcu(&pag->rcu_head, __xfs_free_perag);
@@ -323,10 +322,6 @@ xfs_initialize_perag(
323322
if (error)
324323
goto out_remove_pag;
325324

326-
error = xfs_iunlink_init(pag);
327-
if (error)
328-
goto out_hash_destroy;
329-
330325
/* first new pag is fully initialized */
331326
if (first_initialised == NULLAGNUMBER)
332327
first_initialised = index;
@@ -349,8 +344,6 @@ xfs_initialize_perag(
349344
mp->m_ag_prealloc_blocks = xfs_prealloc_blocks(mp);
350345
return 0;
351346

352-
out_hash_destroy:
353-
xfs_buf_hash_destroy(pag);
354347
out_remove_pag:
355348
radix_tree_delete(&mp->m_perag_tree, index);
356349
out_free_pag:
@@ -362,7 +355,6 @@ xfs_initialize_perag(
362355
if (!pag)
363356
break;
364357
xfs_buf_hash_destroy(pag);
365-
xfs_iunlink_destroy(pag);
366358
kmem_free(pag);
367359
}
368360
return error;

fs/xfs/libxfs/xfs_ag.h

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -103,12 +103,6 @@ struct xfs_perag {
103103
/* background prealloc block trimming */
104104
struct delayed_work pag_blockgc_work;
105105

106-
/*
107-
* Unlinked inode information. This incore information reflects
108-
* data stored in the AGI, so callers must hold the AGI buffer lock
109-
* or have some other means to control concurrency.
110-
*/
111-
struct rhashtable pagi_unlinked_hash;
112106
#endif /* __KERNEL__ */
113107
};
114108

fs/xfs/libxfs/xfs_inode_buf.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,8 @@ xfs_inode_from_disk(
229229
ip->i_nblocks = be64_to_cpu(from->di_nblocks);
230230
ip->i_extsize = be32_to_cpu(from->di_extsize);
231231
ip->i_forkoff = from->di_forkoff;
232-
ip->i_diflags = be16_to_cpu(from->di_flags);
232+
ip->i_diflags = be16_to_cpu(from->di_flags);
233+
ip->i_next_unlinked = be32_to_cpu(from->di_next_unlinked);
233234

234235
if (from->di_dmevmask || from->di_dmstate)
235236
xfs_iflags_set(ip, XFS_IPRESERVE_DM_FIELDS);

fs/xfs/xfs_icache.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,8 @@ xfs_inode_alloc(
111111
INIT_WORK(&ip->i_ioend_work, xfs_end_io);
112112
INIT_LIST_HEAD(&ip->i_ioend_list);
113113
spin_lock_init(&ip->i_ioend_lock);
114+
ip->i_next_unlinked = NULLAGINO;
115+
ip->i_prev_unlinked = NULLAGINO;
114116

115117
return ip;
116118
}
@@ -912,6 +914,7 @@ xfs_reclaim_inode(
912914
ip->i_checked = 0;
913915
spin_unlock(&ip->i_flags_lock);
914916

917+
ASSERT(!ip->i_itemp || ip->i_itemp->ili_item.li_buf == NULL);
915918
xfs_iunlock(ip, XFS_ILOCK_EXCL);
916919

917920
XFS_STATS_INC(ip->i_mount, xs_ig_reclaims);

0 commit comments

Comments
 (0)