- 22 May, 2022 5 commits
-
-
Darrick J. Wong authored
Initialize and destroy the xattr log item caches in the same places that we do all the other log item caches. Signed-off-by:
Darrick J. Wong <djwong@kernel.org> Reviewed-by:
Allison Henderson <allison.henderson@oracle.com> Reviewed-by:
Dave Chinner <dchinner@redhat.com> Signed-off-by:
Dave Chinner <david@fromorbit.com>
-
Darrick J. Wong authored
Nobody uses this field, so get rid of it and the unused flag definition. Rearrange the structure layout to reduce its size from 104 to 96 bytes. This gets us from 39 to 42 objects per page. Signed-off-by:
-
Darrick J. Wong authored
Create a separate slab cache for struct xfs_attr_item objects, since we can pack the (104-byte) intent items more tightly than we can with the general slab cache objects. On x86, this means 39 intents per memory page instead of 32. Signed-off-by:
-
Darrick J. Wong authored
The flags that are stored in the extended attr intent log item really should have a separate namespace from the rest of the XFS_ATTR_* flags. Give them one to make it a little more obvious that they're intent item flags. Signed-off-by:
-
Darrick J. Wong authored
The calling conventions of this function are a mess -- callers /can/ provide a pointer to a pointer to a state structure, but it's not required, and as evidenced by the last two patches, the callers that do weren't be careful enough about how to deal with an existing da state. Push the allocation and freeing responsibilty to the callers, which means that callers from the xattr node state machine steps now have the visibility to allocate or free the da state structure as they please. As a bonus, the node remove/add paths for larp-mode replaces can reset the da state structure instead of freeing and immediately reallocating it. Signed-off-by:
-
- 20 May, 2022 6 commits
-
-
Darrick J. Wong authored
Technically speaking, objects allocated out of a specific slab cache are supposed to be freed to that slab cache. The popular slab backends will take care of this for us, but SLOB famously doesn't. Fix this, even if slob + xfs are not that common of a combination. Signed-off-by:
-
Darrick J. Wong authored
When we're validating a recovered xattr log item during log recovery, we should check the name before starting to allocate resources. This isn't strictly necessary on its own, but it means that we won't bother with huge memory allocations during recovery if the attr name is garbage, which will simplify the changes in the next patch. Signed-off-by:
-
Darrick J. Wong authored
Make sure we screen the "attr flags" field of recovered xattr intent log items to reject flag bits that we don't know about. This is really the attr *filter* field from xfs_da_args, so rename the field and create a mask to make checking for invalid bits easier. Signed-off-by:
-
Darrick J. Wong authored
Make sure we screen the op flags field of recovered xattr intent log items to reject flag bits that we don't know about. Signed-off-by:
-
Darrick J. Wong authored
If a setxattr operation finds an xattr structure in leaf format, adding the attr can fail due to lack of space and hence requires an upgrade to node format. After this happens, we'll roll the transaction and re-enter the state machine, at which time we need to perform a second lookup of the attribute name to find its new location. This lookup attaches a new da state structure to the xfs_attr_item but doesn't free the old one (from the leaf lookup) and leaks it. Fix that. Signed-off-by:
-
Darrick J. Wong authored
kmemleak reported that we lost an xfs_da_state while removing xattrs in generic/020: unreferenced object 0xffff88801c0e4b40 (size 480): comm "attr", pid 30515, jiffies 4294931061 (age 5.960s) hex dump (first 32 bytes): 78 bc 65 07 00 c9 ff ff 00 30 60 1c 80 88 ff ff x.e......0`..... 02 00 00 00 00 00 00 00 80 18 83 4e 80 88 ff ff ...........N.... backtrace: [<ffffffffa023ef4a>] xfs_da_state_alloc+0x1a/0x30 [xfs] [<ffffffffa021b6f3>] xfs_attr_node_hasname+0x23/0x90 [xfs] [<ffffffffa021c6f1>] xfs_attr_set_iter+0x441/0xa30 [xfs] [<ffffffffa02b5104>] xfs_xattri_finish_update+0x44/0x80 [xfs] [<ffffffffa02b515e>] xfs_attr_finish_item+0x1e/0x40 [xfs] [<ffffffffa0244744>] xfs_defer_finish_noroll+0x184/0x740 [xfs] [<ffffffffa02a6473>] __xfs_trans_commit+0x153/0x3e0 [xfs] [<ffffffffa021d149>] xfs_attr_set+0x469/0x7e0 [xfs] [<ffffffffa02a78d9>] xfs_xattr_set+0x89/0xd0 [xfs] [<ffffffff812e6512>] __vfs_removexattr+0x52/0x70 [<ffffffff812e6a08>] __vfs_removexattr_locked+0xb8/0x150 [<ffffffff812e6af6>] vfs_removexattr+0x56/0x100 [<ffffffff812e6bf8>] removexattr+0x58/0x90 [<ffffffff812e6cce>] path_removexattr+0x9e/0xc0 [<ffffffff812e6d44>] __x64_sys_lremovexattr+0x14/0x20 [<ffffffff81786b35>] do_syscall_64+0x35/0x80 I think this is a consequence of xfs_attr_node_removename_setup attaching a new da(btree) state to xfs_attr_item and never freeing it. I /think/ it's the case that the remove paths could detach the da state earlier in the remove state machine since nothing else accesses the state. However, let's future-proof the new xattr code by adding a catch-all when we free the xfs_attr_item to make sure we never leak the da state. Signed-off-by:
-
- 12 May, 2022 16 commits
-
-
Dave Chinner authored
-
Dave Chinner authored
Because heap allocation of 64kB buffers will fail: .... XFS: fs_mark(8414) possible memory allocation deadlock size 65768 in kmem_alloc (mode:0x2d40) XFS: fs_mark(8417) possible memory allocation deadlock size 65768 in kmem_alloc (mode:0x2d40) XFS: fs_mark(8409) possible memory allocation deadlock size 65768 in kmem_alloc (mode:0x2d40) XFS: fs_mark(8428) possible memory allocation deadlock size 65768 in kmem_alloc (mode:0x2d40) XFS: fs_mark(8430) possible memory allocation deadlock size 65768 in kmem_alloc (mode:0x2d40) XFS: fs_mark(8437) possible memory allocation deadlock size 65768 in kmem_alloc (mode:0x2d40) XFS: fs_mark(8433) possible memory allocation deadlock size 65768 in kmem_alloc (mode:0x2d40) XFS: fs_mark(8406) possible memory allocation deadlock size 65768 in kmem_alloc (mode:0x2d40) XFS: fs_mark(8412) possible memory allocation deadlock size 65768 in kmem_alloc (mode:0x2d40) XFS: fs_mark(8432) possible memory allocation deadlock size 65768 in kmem_alloc (mode:0x2d40) XFS: fs_mark(8424) possible memory allocation deadlock size 65768 in kmem_alloc (mode:0x2d40) .... I'd use kvmalloc() instead, but.... - 48.19% xfs_attr_create_intent - 46.89% xfs_attri_init - kvmalloc_node - 46.04% __kmalloc_node - kmalloc_large_node - 45.99% __alloc_pages - 39.39% __alloc_pages_slowpath.constprop.0 - 38.89% __alloc_pages_direct_compact - 38.71% try_to_compact_pages - compact_zone_order - compact_zone - 21.09% isolate_migratepages_block 10.31% PageHuge 5.82% set_pfnblock_flags_mask 0.86% get_pfnblock_flags_mask - 4.48% __reset_isolation_suitable 4.44% __reset_isolation_pfn - 3.56% __pageblock_pfn_to_page 1.33% pfn_to_online_page 2.83% get_pfnblock_flags_mask - 0.87% migrate_pages 0.86% compaction_alloc 0.84% find_suitable_fallback - 6.60% get_page_from_freelist 4.99% clear_page_erms - 1.19% _raw_spin_lock_irqsave - do_raw_spin_lock __pv_queued_spin_lock_slowpath - 0.86% __vmalloc_node_range 0.65% __alloc_pages_bulk .... this is just yet another reminder of how much kvmalloc() sucks. So lift xlog_cil_kvmalloc(), rename it to xlog_kvmalloc() and use that instead.... We also clean up the attribute name and value lengths as they no longer need to be rounded out to sizes compatible with log vectors. Signed-off-by: -
Dave Chinner authored
xfs_repair flags these as a corruption error, so the verifier should catch software bugs that result in empty leaf blocks being written to disk, too. Signed-off-by:
-
Dave Chinner authored
We can't use the same algorithm for replacing an existing attribute when logging attributes. The existing algorithm is essentially: 1. create new attr w/ INCOMPLETE 2. atomically flip INCOMPLETE flags between old + new attribute 3. remove old attr which is marked w/ INCOMPLETE This algorithm guarantees that we see either the old or new attribute, and if we fail after the atomic flag flip, we don't have to recover the removal of the old attr because we never see INCOMPLETE attributes in lookups. For logged attributes, however, this does not work. The logged attribute intents do not track the work that has been done as the transaction rolls, and hence the only recovery mechanism we have is "run the replace operation from scratch". This is further exacerbated by the attempt to avoid needing the INCOMPLETE flag to create an atomic swap. This means we can create a second active attribute of the same name before we remove the original. If we fail at any point after the create but before the removal has completed, we end up with duplicate attributes in the attr btree and recovery only tries to replace one of them. There are several other failure modes where we can leave partially allocated remote attributes that expose stale data, partially free remote attributes that enable UAF based stale data exposure, etc. TO fix this, we need a different algorithm for replace operations when LARP is enabled. Luckily, it's not that complex if we take the right first step. That is, the first thing we log is the attri intent with the new name/value pair and mark the old attr as INCOMPLETE in the same transaction. From there, we then remove the old attr and keep relogging the new name/value in the intent, such that we always know that we have to create the new attr in recovery. Once the old attr is removed, we then run a normal ATTR_CREATE operation relogging the intent as we go. If the new attr is local, then it gets created in a single atomic transaction that also logs the final intent done. If the new attr is remote, the we set INCOMPLETE on the new attr while we allocate and set the remote value, and then we clear the INCOMPLETE flag at in the last transaction taht logs the final intent done. If we fail at any point in this algorithm, log recovery will always see the same state on disk: the new name/value in the intent, and either an INCOMPLETE attr or no attr in the attr btree. If we find an INCOMPLETE attr, we run the full replace starting with removing the INCOMPLETE attr. If we don't find it, then we simply create the new attr. Notably, recovery of a failed create that has an INCOMPLETE flag set is now the same - we start with the lookup of the INCOMPLETE attr, and if that exists then we do the full replace recovery process, otherwise we just create the new attr. Hence changing the way we do the replace operation when LARP is enabled allows us to use the same log recovery algorithm for both the ATTR_CREATE and ATTR_REPLACE operations. This is also the same algorithm we use for runtime ATTR_REPLACE operations (except for the step setting up the initial conditions). The result is that: - ATTR_CREATE uses the same algorithm regardless of whether LARP is enabled or not - ATTR_REPLACE with larp=0 is identical to the old algorithm - ATTR_REPLACE with larp=1 runs an unmodified attr removal algorithm from the larp=0 code and then runs the unmodified ATTR_CREATE code. - log recovery when larp=1 runs the same ATTR_REPLACE algorithm as it uses at runtime. Because the state machine is now quite clean, changing the algorithm is really just a case of changing the initial state and how the states link together for the ATTR_REPLACE case. Hence it's not a huge amount of code for what is a fairly substantial rework of the attr logging and recovery algorithm.... Signed-off-by:
-
Dave Chinner authored
We currently store the high level attr operation in args->attr_flags. This field contains what the VFS is telling us to do, but don't necessarily match what we are doing in the low level modification state machine. e.g. XATTR_REPLACE implies both XFS_DA_OP_ADDNAME and XFS_DA_OP_RENAME because it is doing both a remove and adding a new attr. However, deep in the individual state machine operations, we check errors against this high level VFS op flags, not the low level XFS_DA_OP flags. Indeed, we don't even have a low level flag for a REMOVE operation, so the only way we know we are doing a remove is the complete absence of XATTR_REPLACE, XATTR_CREATE, XFS_DA_OP_ADDNAME and XFS_DA_OP_RENAME. And because there are other flags in these fields, this is a pain to check if we need to. As the XFS_DA_OP flags are only needed once the deferred operations are set up, set these flags appropriately when we set the initial operation state. We also introduce a XFS_DA_OP_REMOVE flag to make it easy to know that we are doing a remove operation. With these, we can remove the use of XATTR_REPLACE and XATTR_CREATE in low level lookup operations, and manipulate the low level flags according to the low level context that is operating. e.g. log recovery does not have a VFS xattr operation state to copy into args->attr_flags, and the low level state machine ops we do for recovery do not match the high level VFS operations that were in progress when the system failed... Signed-off-by:
-
Dave Chinner authored
xfs_attri_remove_iter is not used anymore, so remove it and all the infrastructure it uses and is needed to drive it. THe xfs_attr_refillstate() function now throws an unused warning, so isolate the xfs_attr_fillstate()/xfs_attr_refillstate() code pair with an #if 0 and a comment explaining why we want to keep this code and restore the optimisation it provides in the near future. Signed-off-by:
-
Dave Chinner authored
Now that xfs_attri_set_iter() has initial states for removing attributes, switch the pure attribute removal code over to using it. This requires attrs being removed to always be marked as INCOMPLETE before we start the removal due to the fact we look up the attr to remove again in xfs_attr_node_remove_attr(). Note: this drops the fillstate/refillstate optimisations from the remove path that avoid having to look up the path again after setting the incomplete flag and removing remote attrs. Restoring that optimisation to this path is future Dave's problem. Signed-off-by:
-
Dave Chinner authored
We need to merge the add and remove code paths to enable safe recovery of replace operations. Hoist the initial remove states from xfs_attr_remove_iter into xfs_attr_set_iter. We will make use of them in the next patches. Signed-off-by:
-
Dave Chinner authored
Now that the full xfs_attr_set_iter() state machine always terminates with either the state being XFS_DAS_DONE on success or an error on failure, we can get rid of the need for it to return -EAGAIN whenever it needs to roll the transaction before running the next state. That is, we don't need to spray -EAGAIN return states everywhere, the caller just check the state machine state for completion to determine what action should be taken next. This greatly simplifies the code within the state machine implementation as it now only has to handle 0 for success or -errno for error and it doesn't need to tell the caller to retry. Signed-off-by:
-
Dave Chinner authored
Clean up the final leaf/node states in xfs_attr_set_iter() to further simplify the high level state machine and to set the completion state correctly. As we are adding a separate state for node format removal, we need to ensure that node formats are collapsed back to shortform or empty correctly. Signed-off-by:
-
Dave Chinner authored
We may not have a remote value for the old xattr we have to remove, so skip over the remote value removal states and go straight to the xattr name removal in the leaf/node block. Signed-off-by:
-
Dave Chinner authored
We can skip the REPLACE state when LARP is enabled, but that means the XFS_DAS_FLIP_LFLAG state is now poorly named - it indicates something that has been done rather than what the state is going to do. Rename it to "REMOVE_OLD" to indicate that we are now going to perform removal of the old attr. Signed-off-by:
-
Dave Chinner authored
When we set a new xattr, we have three exit paths: 1. nothing else to do 2. allocate and set the remote xattr value 3. perform the rest of a replace operation Currently we push both 2 and 3 into the same state, regardless of whether we just set a remote attribute or not. Once we've set the remote xattr, we have two exit states: 1. nothing else to do 2. perform the rest of a replace operation Hence we can split the remote xattr allocation and setting into their own states and factor it out of xfs_attr_set_iter() to further clean up the state machine and the implementation of the state machine. Signed-off-by:
-
Dave Chinner authored
The operations performed from XFS_DAS_FOUND_LBLK through to XFS_DAS_RM_LBLK are now identical to XFS_DAS_FOUND_NBLK through to XFS_DAS_RM_NBLK. We can collapse these down into a single set of code. To do this, define the states that leaf and node run through as separate sets of sequential states. Then as we move to the next state, we can use increments rather than specific state assignments to move through the states. This means the state progression is set by the initial state that enters the series and we don't need to duplicate the code anymore. At the exit point of the series we need to select the correct leaf or node state, but that can also be done by state increment rather than assignment. Signed-off-by:
-
Dave Chinner authored
We re-enter the XFS_DAS_FOUND_LBLK state when we have to allocate multiple extents for a remote xattr. We currently have a flag called XFS_DAC_LEAF_ADDNAME_INIT to avoid running the remote attr hole finding code more than once. However, for the node format tree, we have a separate state for this so we never reenter the state machine at XFS_DAS_FOUND_NBLK and so it does not need a special flag to skip over the remote attr hold finding code. Convert the leaf block code to use the same state machine as the node blocks and kill the XFS_DAC_LEAF_ADDNAME_INIT flag. This further points out that this "ALLOC" state is only traversed if we have remote xattrs or we are doing a rename operation. Rename both the leaf and node alloc states to _ALLOC_RMT to indicate they are iterating to do allocation of remote xattr blocks. Signed-off-by:
-
Dave Chinner authored
We current use XFS_DAS_UNINIT for several steps in the attr_set state machine. We use it for setting shortform xattrs, converting from shortform to leaf, leaf add, leaf-to-node and leaf add. All of these things are essentially known before we start the state machine iterating, so we really should separate them out: XFS_DAS_SF_ADD: - tries to do a shortform add - on success -> done - on ENOSPC converts to leaf, -> XFS_DAS_LEAF_ADD - on error, dies. XFS_DAS_LEAF_ADD: - tries to do leaf add - on success: - inline attr -> done - remote xattr || REPLACE -> XFS_DAS_FOUND_LBLK - on ENOSPC converts to node, -> XFS_DAS_NODE_ADD - on error, dies XFS_DAS_NODE_ADD: - tries to do node add - on success: - inline attr -> done - remote xattr || REPLACE -> XFS_DAS_FOUND_NBLK - on error, dies This makes it easier to understand how the state machine starts up and sets us up on the path to further state machine simplifications. This also converts the DAS state tracepoints to use strings rather than numbers, as converting between enums and numbers requires manual counting rather than just reading the name. This also introduces a XFS_DAS_DONE state so that we can trace successful operation completions easily. Signed-off-by:
-
- 11 May, 2022 13 commits
-
-
Catherine Hoang authored
Having just dropped support for quota warning limits and warning counters, the warning fields no longer have any meaning. Prevent these fields from being set by removing QC_WARNS_MASK from XFS_QC_SETINFO_MASK and XFS_QC_MASK. Signed-off-by:
Catherine Hoang <catherine.hoang@oracle.com> Reviewed-by:
-
Catherine Hoang authored
Warning counts are not used anywhere in the kernel. In addition, there are no use cases, test coverage, or documentation for this functionality. Remove the 'warnings' field from struct xfs_dquot_res and any other related code. Signed-off-by:
-
Catherine Hoang authored
Warning limits in xfs quota is an unused feature that is currently documented as unimplemented, and it is unclear what the intended behavior of these limits are. Remove the ‘warn’ field from struct xfs_quota_limits and any other related code. Signed-off-by:
-
Dave Chinner authored
Logged attribute intents only have set and remove types - there is no separate intent type for a replace operation. We should have a separate type for a replace operation, as it needs to perform operations that neither SET or REMOVE can perform. Add this type to the intent items and rearrange the deferred operation setup to reflect the different operations we are performing. Signed-off-by:
-
Dave Chinner authored
We currently set it and hold it when converting from short to leaf form, then release it only to immediately look it back up again to do the leaf insert. Do a bit of refactoring to xfs_attr_leaf_try_add() to avoid this messy handling of the newly allocated leaf buffer. Signed-off-by:
-
Dave Chinner authored
On the first allocation of a attrd item, xfs_trans_add_item() fires an assert like so: XFS (pmem0): EXPERIMENTAL logged extended attributes feature added. Use at your own risk! XFS: Assertion failed: !test_bit(XFS_LI_DIRTY, &lip->li_flags), file: fs/xfs/xfs_trans.c, line: 683 ------------[ cut here ]------------ kernel BUG at fs/xfs/xfs_message.c:102! Call Trace: <TASK> xfs_trans_add_item+0x17e/0x190 xfs_trans_get_attrd+0x67/0x90 xfs_attr_create_done+0x13/0x20 xfs_defer_finish_noroll+0x100/0x690 __xfs_trans_commit+0x144/0x330 xfs_trans_commit+0x10/0x20 xfs_attr_set+0x3e2/0x4c0 xfs_initxattrs+0xaa/0xe0 security_inode_init_security+0xb0/0x130 xfs_init_security+0x18/0x20 xfs_generic_create+0x13a/0x340 xfs_vn_create+0x17/0x20 path_openat+0xff3/0x12f0 do_filp_open+0xb2/0x150 The attrd log item is allocated via kmem_cache_alloc, and xfs_log_item_init() does not zero the entire log item structure - it assumes that the structure is already all zeros as it only initialises non-zero fields. Fix the attr items to be allocated via the *zalloc methods. Signed-off-by:
-
Dave Chinner authored
generic/642 triggered a reproducable assert failure in xlog_cil_commit() that resulted from a xfs_attr_set() committing an empty but dirty transaction. When the CIL is empty and this occurs, xlog_cil_commit() tries a background push and this triggers a "pushing an empty CIL" assert. XFS: Assertion failed: !list_empty(&cil->xc_cil), file: fs/xfs/xfs_log_cil.c, line: 1274 Call Trace: <TASK> xlog_cil_commit+0xa5a/0xad0 __xfs_trans_commit+0xb8/0x330 xfs_trans_commit+0x10/0x20 xfs_attr_set+0x3e2/0x4c0 xfs_xattr_set+0x8d/0xe0 __vfs_setxattr+0x6b/0x90 __vfs_setxattr_noperm+0x76/0x220 __vfs_setxattr_locked+0xdf/0x100 vfs_setxattr+0x94/0x170 setxattr+0x110/0x200 path_setxattr+0xbf/0xe0 __x64_sys_setxattr+0x2b/0x30 do_syscall_64+0x35/0x80 The problem is related to the breakdown of attribute addition in xfs_attr_set_iter() and how it is called from deferred operations. When we have a pure leaf xattr insert, we add the xattr to the leaf and set the next state to XFS_DAS_FOUND_LBLK and return -EAGAIN. This requeues the xattr defered work, rolls the transaction and runs xfs_attr_set_iter() again. This then checks the xattr for being remote (it's not) and whether a replace op is being done (this is a create op) and if neither are true it returns without having done anything. xfs_xattri_finish_update() then unconditionally sets the transaction dirty, and the deferops finishes and returns to __xfs_trans_commit() which sees the transaction dirty and tries to commit it by calling xlog_cil_commit(). The transaction is empty, and then the assert fires if this happens when the CIL is empty. This patch addresses the structure of xfs_attr_set_iter() that requires re-entry on leaf add even when nothing will be done. This gets rid of the trailing empty transaction and so doesn't trigger the XFS_TRANS_DIRTY assignment in xfs_xattri_finish_update() incorrectly. Addressing that is for a different patch. Signed-off-by:
-
Allison Henderson authored
Add an error tag on xfs_attr3_leaf_to_node to test log attribute recovery and replay. Signed-off-by:
Chandan Babu R <chandan.babu@oracle.com> Signed-off-by:
-
Allison Henderson authored
Add an error tag on xfs_da3_split to test log attribute recovery and replay. Signed-off-by:
-
Allison Henderson authored
Quick helper function to collapse duplicate code to initialize transactions for attributes Signed-off-by:
-
Allison Henderson authored
This patch adds a helper function xfs_attr_leaf_addname. While this does help to break down xfs_attr_set_iter, it does also hoist out some of the state management. This patch has been moved to the end of the clean up series for further discussion. Suggested-by:
Chandan Babu R <chandanrlinux@gmail.com> Signed-off-by:
-
Allison Henderson authored
This is a clean up patch that merges xfs_delattr_context into xfs_attr_item. Now that the refactoring is complete and the delayed operation infrastructure is in place, we can combine these to eliminate the extra struct Signed-off-by:
Darrick J. Wong <darrick.wong@oracle.com> Reviewed-by:
-
Allison Henderson authored
This patch adds a debug option to enable log attribute replay. Eventually this can be removed when delayed attrs becomes permanent. Signed-off-by:
-
