On Thu, Mar 25, 2021 at 05:32:57PM -0700, Allison Henderson wrote: > Hi all, > > This set is a subset of a larger series for Dealyed Attributes. Which is a > subset of a yet larger series for parent pointers. Delayed attributes allow > attribute operations (set and remove) to be logged and committed in the same > way that other delayed operations do. This allows more complex operations (like > parent pointers) to be broken up into multiple smaller transactions. To do > this, the existing attr operations must be modified to operate as a delayed > operation. This means that they cannot roll, commit, or finish transactions. > Instead, they return -EAGAIN to allow the calling function to handle the > transaction. In this series, we focus on only the delayed attribute portion. > We will introduce parent pointers in a later set. > > In this version I have reduced the set back to the "Delay Ready Attrs" sub series to > avoid reviewer burn out, but the extended series is available to view in the inlcuded > git hub links, which extend all the way through parent pointers. Feel free to review > as much as feels reasonable. The set as a whole is a bit much to digest at once, so > working through it in progressive subsets seems like a reasonable way to manage its > dev efforts. > > Lastly, in the last revision folks asked for some stress testing on the set. On my > system, I found that in an fsstress test with all patches applied, we spend at most > %0.17 of the time in the attr routines, compared to at most %0.12 with out the set applied. > Both can fluctuate quite a bit depending on the other operations going on that seem to > occupy most of the activity. For the most part though, I do not find these results to be > particularly concerning. Though folks are certainly welcome to try it out on their own > system to see how the results might differ. > > Updates since v15: Mostly just review feed back from the previous revision. I've > tracked changes below to help reviews recall the changes discussed Hmm... so I ran fstests against this on an otherwise default V5 filesystem, and saw three new regressions: xfs/125 spat out this from the final repair run: Phase 1 - find and verify superblock... Phase 2 - using internal log - zero log... - scan filesystem freespace and inode maps... - found root inode chunk Phase 3 - for each AG... - scan (but don't clear) agi unlinked lists... - process known inodes and perform inode discovery... - agno = 0 attribute entry #32 in attr block 2, inode 134 is INCOMPLETE problem with attribute contents in inode 134 would clear attr fork bad nblocks 8 for inode 134, would reset to 0 bad anextents 4 for inode 134, would reset to 0 - agno = 1 - agno = 2 - agno = 3 - process newly discovered inodes... Phase 4 - check for duplicate blocks... - setting up duplicate extent list... - check for inodes claiming duplicate blocks... - agno = 0 - agno = 1 - agno = 2 - agno = 3 No modify flag set, skipping phase 5 Phase 6 - check inode connectivity... - traversing filesystem ... - traversal finished ... - moving disconnected inodes to lost+found ... Phase 7 - verify link counts... No modify flag set, skipping filesystem flush and exiting. xfs_repair should not fail And xfs/434 and xfs/436 both complained about memory leaks stemming from an xfs_da_state that xfs/125 didn't free correctly: [ 1247.150683] ============================================================================= [ 1247.151799] BUG xfs_da_state (Tainted: G B W ): Objects remaining in xfs_da_state on __kmem_cache_shutdown() [ 1247.153246] ----------------------------------------------------------------------------- [ 1247.153246] [ 1247.154528] INFO: Slab 0xffffea00002e9280 objects=17 used=11 fp=0xffff88800ba4b4a0 flags=0xfff80000010200 [ 1247.155764] CPU: 2 PID: 50257 Comm: modprobe Tainted: G B W 5.12.0-rc4-djwx #rc4 [ 1247.156849] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.13.0-1ubuntu1.1 04/01/2014 [ 1247.157996] Call Trace: [ 1247.158330] dump_stack+0x64/0x7c [ 1247.158767] slab_err+0xb7/0xdc [ 1247.159196] ? printk+0x58/0x6f [ 1247.159615] __kmem_cache_shutdown.cold+0x39/0x15e [ 1247.160248] kmem_cache_destroy+0x3f/0x110 [ 1247.160779] xfs_destroy_zones+0xbe/0xe2 [xfs] [ 1247.161462] exit_xfs_fs+0x5f/0x9b4 [xfs] [ 1247.162065] __do_sys_delete_module.constprop.0+0x145/0x220 [ 1247.162740] do_syscall_64+0x2d/0x40 [ 1247.163197] entry_SYSCALL_64_after_hwframe+0x44/0xae [ 1247.163810] RIP: 0033:0x7fd91cfe4bcb [ 1247.164262] Code: 73 01 c3 48 8b 0d c5 82 0c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa b8 b0 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 95 82 0c 00 f7 d8 64 89 01 48 [ 1247.166352] RSP: 002b:00007fff89097038 EFLAGS: 00000206 ORIG_RAX: 00000000000000b0 [ 1247.167217] RAX: ffffffffffffffda RBX: 0000558b8e105cc0 RCX: 00007fd91cfe4bcb [ 1247.167998] RDX: 0000000000000000 RSI: 0000000000000800 RDI: 0000558b8e105d28 [ 1247.168781] RBP: 0000558b8e105cc0 R08: 0000000000000000 R09: 0000000000000000 [ 1247.169562] R10: 00007fd91d060ac0 R11: 0000000000000206 R12: 0000558b8e105d28 [ 1247.170351] R13: 0000000000000000 R14: 0000558b8e105d28 R15: 0000558b8e105cc0 >From a quick bisect, all of thse problem originates in the last patch. --D > xfs: Reverse apply 72b97ea40d > NEW > > xfs: Add helper xfs_attr_node_remove_step > DROPPED > > xfs: Add xfs_attr_node_remove_cleanup > No change > > xfs: Hoist transaction handling in xfs_attr_node_remove_step > DROPPED > > xfs: Hoist xfs_attr_set_shortform > No change > > xfs: Add helper xfs_attr_set_fmt > Fixed helper to return error when defer_finish fails > > xfs: Separate xfs_attr_node_addname and xfs_attr_node_addname_work > Renamed xfs_attr_node_addname_work to xfs_attr_node_addname_clear_incomplete > > xfs: Add helper xfs_attr_node_addname_find_attr > Renamed goto out, to goto error > > xfs: Hoist xfs_attr_node_addname > Removed unused retval variable > Removed extra state free in xfs_attr_node_addname > > xfs: Hoist xfs_attr_leaf_addname > Fixed spelling typos > > xfs: Hoist node transaction handling > Added consistent braces to if/else statement > > xfs: Add delay ready attr remove routines > Typo fixes > Merged xfs_attr_remove_iter with xfs_attr_node_removename_iter > Added state XFS_DAS_RMTBLK > Flow chart updated > > xfs: Add delay ready attr set routines > Rebase adjustments > Typo fixes > > > Extended Series Changes > ------------------------ > xfs: Add state machine tracepoints > Rebase adjustments > xfs_attr_node_remove_rmt_return removed to match earlier refactoring changes > trace_xfs_attr_node_removename_iter_return becomes > trace_xfs_attr_remove_iter_return to match earlier refactoring changes > > xfs: Rename __xfs_attr_rmtval_remove > No change > > xfs: Handle krealloc errors in xlog_recover_add_to_cont_trans > Added kmem_alloc_large fall back > > xfs: Set up infrastructure for deferred attribute operations > Typo fixes > Rename xfs_trans_attr to xfs_trans_attr_finish_update > Added helper function xfs_attri_validate > Split patch into infrastructure and implementation patches > Added XFS_ERROR_REPORT in xlog_recover_attri_commit_pass2: > > xfs: Implement for deferred attribute operations > NEW > > xfs: Skip flip flags for delayed attrs > Did a performance analysis > > xfs: Add xfs_attr_set_deferred and xfs_attr_remove_deferred > Typo fixes > > xfs: Remove unused xfs_attr_*_args > Rebase adjustments > > xfs: Add delayed attributes error tag > Added errortag include > > xfs: Merge xfs_delattr_context into xfs_attr_item > Typo fixes > > > This series can be viewed on github here: > https://github.com/allisonhenderson/xfs_work/tree/delay_ready_attrs_v16 > > As well as the extended delayed attribute and parent pointer series: > https://github.com/allisonhenderson/xfs_work/tree/delay_ready_attrs_v16_extended > > And the test cases: > https://github.com/allisonhenderson/xfs_work/tree/pptr_xfstestsv2 > > In order to run the test cases, you will need have the corresponding xfsprogs > changes as well. Which can be found here: > https://github.com/allisonhenderson/xfs_work/tree/delay_ready_attrs_xfsprogs_v16 > https://github.com/allisonhenderson/xfs_work/tree/delay_ready_attrs_xfsprogs_v16_extended > > To run the xfs attributes tests run: > check -g attr > > To run as delayed attributes run: > export MOUNT_OPTIONS="-o delattr" > check -g attr > > To run parent pointer tests: > check -g parent > > I've also made the corresponding updates to the user space side as well, and ported anything > they need to seat correctly. > > Questions, comment and feedback appreciated! > > Thanks all! > Allison > > Allison Henderson (11): > xfs: Reverse apply 72b97ea40d > xfs: Add xfs_attr_node_remove_cleanup > xfs: Hoist xfs_attr_set_shortform > xfs: Add helper xfs_attr_set_fmt > xfs: Separate xfs_attr_node_addname and > xfs_attr_node_addname_clear_incomplete > xfs: Add helper xfs_attr_node_addname_find_attr > xfs: Hoist xfs_attr_node_addname > xfs: Hoist xfs_attr_leaf_addname > xfs: Hoist node transaction handling > xfs: Add delay ready attr remove routines > xfs: Add delay ready attr set routines > > fs/xfs/libxfs/xfs_attr.c | 903 ++++++++++++++++++++++++---------------- > fs/xfs/libxfs/xfs_attr.h | 364 ++++++++++++++++ > fs/xfs/libxfs/xfs_attr_leaf.c | 2 +- > fs/xfs/libxfs/xfs_attr_remote.c | 126 ++++-- > fs/xfs/libxfs/xfs_attr_remote.h | 7 +- > fs/xfs/xfs_attr_inactive.c | 2 +- > fs/xfs/xfs_trace.h | 1 - > 7 files changed, 998 insertions(+), 407 deletions(-) > > -- > 2.7.4 >
