On 3/29/21 2:59 PM, Darrick J. Wong wrote:
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.
Alrighty, I will see if I can recreate these bugs and get that figured
out. Thanks!
Allison
--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://urldefense.com/v3/__https://github.com/allisonhenderson/xfs_work/tree/delay_ready_attrs_v16__;!!GqivPVa7Brio!Ol0JaXyUI3K5MEGrBo_eMHzcTqcuIL9p25-XSZftWgn4bmbxeX_AJf7Hl-kP6ecxUskY$
As well as the extended delayed attribute and parent pointer series:
https://urldefense.com/v3/__https://github.com/allisonhenderson/xfs_work/tree/delay_ready_attrs_v16_extended__;!!GqivPVa7Brio!Ol0JaXyUI3K5MEGrBo_eMHzcTqcuIL9p25-XSZftWgn4bmbxeX_AJf7Hl-kP6QsZXJc5$
And the test cases:
https://urldefense.com/v3/__https://github.com/allisonhenderson/xfs_work/tree/pptr_xfstestsv2__;!!GqivPVa7Brio!Ol0JaXyUI3K5MEGrBo_eMHzcTqcuIL9p25-XSZftWgn4bmbxeX_AJf7Hl-kP6fAPHdk4$
In order to run the test cases, you will need have the corresponding xfsprogs
changes as well. Which can be found here:
https://urldefense.com/v3/__https://github.com/allisonhenderson/xfs_work/tree/delay_ready_attrs_xfsprogs_v16__;!!GqivPVa7Brio!Ol0JaXyUI3K5MEGrBo_eMHzcTqcuIL9p25-XSZftWgn4bmbxeX_AJf7Hl-kP6ToIBbr7$
https://urldefense.com/v3/__https://github.com/allisonhenderson/xfs_work/tree/delay_ready_attrs_xfsprogs_v16_extended__;!!GqivPVa7Brio!Ol0JaXyUI3K5MEGrBo_eMHzcTqcuIL9p25-XSZftWgn4bmbxeX_AJf7Hl-kP6cfTeHmh$
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
