- 30 Apr, 2024 1 commit
-
-
Christoph Hellwig authored
xfs_bmapi_write can return 0 without actually returning a mapping in mval in two different cases: 1) when there is absolutely no space available to do an allocation 2) when converting delalloc space, and the allocation is so small that it only covers parts of the delalloc extent before the range requested by the caller Callers at best can handle one of these cases, but in many cases can't cope with either one. Switch xfs_bmapi_write to always return a mapping or return an error code instead. For case 1) above ENOSPC is the obvious choice which is very much what the callers expect anyway. For case 2) there is no really good error code, so pick a funky one from the SysV streams portfolio. This fixes the reproducer here: https://lore.kernel.org/linux-xfs/CAEJPjCvT3Uag-pMTYuigEjWZHn1sGMZ0GCjVVCv29tNHK76Cgg@mail.gmail.com0/ which uses reserved blocks to create file systems that are gravely out of space and thus cause at least xfs_file_alloc_space to hang and trigger the lack of ENOSPC handling in xfs_dquot_disk_alloc. Note that this patch does not actually make any caller but xfs_alloc_file_space deal intelligently with case 2) above. Signed-off-by:Christoph Hellwig <hch@lst.de> Reported-by:
刘通 <lyutoon@gmail.com> Reviewed-by:
"Darrick J. Wong" <djwong@kernel.org> Signed-off-by:
Chandan Babu R <chandanbabu@kernel.org>
-
- 29 Apr, 2024 4 commits
-
-
Zhang Yi authored
Current clone operation could be non-atomic if the destination of a file is beyond EOF, user could get a file with corrupted (zeroed) data on crash. The problem is about preallocations. If you write some data into a file: [A...B) and XFS decides to preallocate some post-eof blocks, then it can create a delayed allocation reservation: [A.........D) The writeback path tries to convert delayed extents to real ones by allocating blocks. If there aren't enough contiguous free space, we can end up with two extents, the first real and the second still delalloc: [A....C)[C.D) After that, both the in-memory and the on-disk file sizes are still B. If we clone into the range [E...F) from another file: [A....C)[C.D) [E...F) then xfs_reflink_zero_posteof() calls iomap_zero_range() to zero out the range [B, E) beyond EOF and flush it. Since [C, D) is still a delalloc extent, its pagecache will be zeroed and both the in-memory and on-disk size will be updated to D after flushing but before cloning. This is wrong, because the user can see the size change and read the zeroes while the clone operation is ongoing. We need to keep the in-memory and on-disk size before the clone operation starts, so instead of writing zeroes through the page cache for delayed ranges beyond EOF, we convert these ranges to unwritten and invalidate any cached data over that range beyond EOF. Suggested-by:
Dave Chinner <david@fromorbit.com> Signed-off-by:
Zhang Yi <yi.zhang@huawei.com> Reviewed-by:
-
Zhang Yi authored
Since xfs_bmapi_convert_delalloc() only attempts to allocate the entire delalloc extent and require multiple invocations to allocate the target offset. So xfs_convert_blocks() add a loop to do this job and we call it in the write back path, but xfs_convert_blocks() isn't a common helper. Let's do it in xfs_bmapi_convert_delalloc() and drop xfs_convert_blocks(), preparing for the post EOF delalloc blocks converting in the buffered write begin path. Signed-off-by:
-
Zhang Yi authored
Allow callers to pass a NULLL seq argument if they don't care about the fork sequence number. Signed-off-by:
-
Zhang Yi authored
Commit 1aa91d9c ("xfs: Add async buffered write support") replace xfs_ilock(XFS_ILOCK_EXCL) with xfs_ilock_for_iomap() when locking the writing inode, and a new variable lockmode is used to indicate the lock mode. Although the lockmode should always be XFS_ILOCK_EXCL, it's still better to use this variable instead of useing XFS_ILOCK_EXCL directly when unlocking the inode. Fixes: 1aa91d9c ("xfs: Add async buffered write support") Signed-off-by:
-
- 26 Apr, 2024 5 commits
-
-
Christoph Hellwig authored
Add a new enum and a xfs_dir2_format helper that returns it to allow the code to switch on the format of a directory in a single operation and switch all helpers of xfs_dir2_isblock and xfs_dir2_isleaf to it. This also removes the explicit xfs_iread_extents call in a few of the call sites given that xfs_bmap_last_offset already takes care of it underneath. Signed-off-by:
-
Christoph Hellwig authored
Add a helper to switch between the different directory formats for removing a directory entry. Signed-off-by:
-
Christoph Hellwig authored
Add a helper to switch between the different directory formats for removing a directory entry. Signed-off-by:
-
Christoph Hellwig authored
Add a helper to switch between the different directory formats for creating a directory entry and to handle the XFS_DA_OP_JUSTCHECK flag based on the passed in ino number field. Signed-off-by:
-
Christoph Hellwig authored
Add a helper to switch between the different directory formats for lookup and to handle the -EEXIST return for a successful lookup. Signed-off-by:
-
- 24 Apr, 2024 10 commits
-
-
Jiapeng Chong authored
The function are defined in the dir_repair.c file, but not called elsewhere, so delete the unused function. fs/xfs/scrub/dir_repair.c:186:1: warning: unused function 'xrep_dir_self_parent'. Reported-by:
Abaci Robot <abaci@linux.alibaba.com> Closes: https://bugzilla.openanolis.cn/show_bug.cgi?id=8867Signed-off-by:
Jiapeng Chong <jiapeng.chong@linux.alibaba.com> Reviewed-by:
-
Chandan Babu R authored
Merge tag 'repair-fixes-6.10_2024-04-23' of https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfs-linux into xfs-6.10-mergeC xfs: minor fixes to online repair Here are some miscellaneous bug fixes for the online repair code. Signed-off-by:
-
Chandan Babu R authored
Merge tag 'reduce-scrub-iget-overhead-6.10_2024-04-23' of https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfs-linux into xfs-6.10-mergeC xfs: reduce iget overhead in scrub This patchset looks to reduce iget overhead in two ways: First, a previous patch conditionally set DONTCACHE on inodes during xchk_irele on the grounds that we knew better at irele time if an inode should be dropped. Unfortunately, over time that patch morphed into a call to d_mark_dontcache, which resulted in inodes being dropped even if they were referenced by the dcache. This actually caused *more* recycle overhead than if we'd simply called xfs_iget to set DONTCACHE only on misses. The second patch reduces the cost of untrusted iget for a vectored scrub call by having the scrubv code maintain a separate refcount to the inode so that the cache will always hit. Signed-off-by:
-
Chandan Babu R authored
Merge tag 'vectorized-scrub-6.10_2024-04-23' of https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfs-linux into xfs-6.10-mergeC xfs: vectorize scrub kernel calls Create a vectorized version of the metadata scrub and repair ioctl, and adapt xfs_scrub to use that. This mitigates the impact of system call overhead on xfs_scrub runtime. Signed-off-by:
-
Chandan Babu R authored
Merge tag 'scrub-directory-tree-6.10_2024-04-23' of https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfs-linux into xfs-6.10-mergeC xfs: detect and correct directory tree problems Historically, checking the tree-ness of the directory tree structure has not been complete. Cycles of subdirectories break the tree properties, as do subdirectories with multiple parents. It's easy enough for DFS to detect problems as long as one of the participants is reachable from the root, but this technique cannot find unconnected cycles. Directory parent pointers change that, because we can discover all of these problems from a simple walk from a subdirectory towards the root. For each child we start with, if the walk terminates without reaching the root, we know the path is disconnected and ought to be attached to the lost and found. If we find ourselves, we know this is a cycle and can delete an incoming edge. If we find multiple paths to the root, we know to delete an incoming edge. Even better, once we've finished walking paths, we've identified the good ones and know which other path(s) to remove. Signed-off-by:
-
Chandan Babu R authored
Merge tag 'repair-pptrs-6.10_2024-04-23' of https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfs-linux into xfs-6.10-mergeC xfs: online repair for parent pointers This series implements online repair for directory parent pointer metadata. The checking half is fairly straightforward -- for each outgoing directory link (forward or backwards), grab the inode at the other end, and confirm that there's a corresponding link. If we can't grab an inode or lock it, we'll save that link for a slower loop that cycles all the locks, confirms the continued existence of the link, and rechecks the link if it's actually still there. Repairs are a bit more involved -- for directories, we walk the entire filesystem to rebuild the dirents from parent pointer information. Parent pointer repairs do the same walk but rebuild the pptrs from the dirent information, but with the added twist that it duplicates all the xattrs so that it can use the atomic extent swapping code to commit the repairs atomically. This introduces an added twist to the xattr repair code -- we use dirent hooks to detect a colliding update to the pptr data while we're not holding the ILOCKs; if one is detected, we restart the xattr salvaging process but this time hold all the ILOCKs until the end of the scan. For offline repair, the phase6 directory connectivity scan generates an index of all the expected parent pointers in the filesystem. Then it walks each file and compares the parent pointers attached to that file against the index generated, and resyncs the results as necessary. Signed-off-by:
-
Chandan Babu R authored
Merge tag 'scrub-pptrs-6.10_2024-04-23' of https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfs-linux into xfs-6.10-mergeC xfs: scrubbing for parent pointers Teach online fsck to use parent pointers to assist in checking directories, parent pointers, extended attributes, and link counts. Signed-off-by:
-
Chandan Babu R authored
Merge tag 'pptrs-6.10_2024-04-23' of https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfs-linux into xfs-6.10-mergeC xfs: Parent Pointers This is the latest parent pointer attributes for xfs. The goal of this patch set is to add a parent pointer attribute to each inode. The attribute name containing the parent inode, generation, and directory offset, while the attribute value contains the file name. This feature will enable future optimizations for online scrub, shrink, nfs handles, verity, or any other feature that could make use of quickly deriving an inodes path from the mount point. Directory parent pointers are stored as namespaced extended attributes of a file. Because parent pointers are an indivisible tuple of (dirent_name, parent_ino, parent_gen) we cannot use the usual attr name lookup functions to find a parent pointer. This is solvable by introducing a new lookup mode that checks both the name and the value of the xattr. Therefore, introduce this new name-value lookup mode that's gated on the XFS_ATTR_PARENT namespace. This requires the introduction of new opcodes for the extended attribute update log intent items, which actually means that parent pointers (itself an INCOMPAT feature) does not depend on the LOGGED_XATTRS log incompat feature bit. To reduce collisions on the dirent names of parent pointers, introduce a new attr hash mode that is the dir2 namehash of the dirent name xor'd with the parent inode number. Signed-off-by:
-
Chandan Babu R authored
Merge tag 'improve-attr-validation-6.10_2024-04-23' of https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfs-linux into xfs-6.10-mergeC xfs: improve extended attribute validation Prior to introducing parent pointer extended attributes, let's spend some time cleaning up the attr code and strengthening the validation that it performs on attrs coming in from the disk. Signed-off-by:
-
Chandan Babu R authored
Merge tag 'shrink-dirattr-args-6.10_2024-04-23' of https://git.kernel.org/pub/scm/linux/kernel/git/djwong/xfs-linux into xfs-6.10-mergeC xfs: shrink struct xfs_da_args Let's clean out some unused flags and fields from struct xfs_da_args. Signed-off-by:
-
- 23 Apr, 2024 20 commits
-
-
Darrick J. Wong authored
Invalidate the cached dentries that point to the file that we're moving to lost+found before we actually move it. Signed-off-by:
-
Darrick J. Wong authored
The atomic file exchange-range functionality is now a permanent filesystem feature instead of a dynamic log-incompat feature. It cannot be turned on at runtime, so we no longer need the XCHK_FSGATES flags and whatnot that supported it. Remove the flag and the enable function, and move the xfs_has_exchange_range checks to the start of the repair functions. Signed-off-by:
-
Darrick J. Wong authored
If the transaction allocation in xrep_adoption_trans_alloc fails, we should drop only the locks that we took. In this case this is ILOCK_EXCL of both the orphanage and the file being repaired. Dropping any IOLOCK here is incorrect. Found by fuzzing u3.sfdir3.list[1].name = zeroes in xfs/1546. Signed-off-by:
-
Darrick J. Wong authored
If a program wants us to perform a scrub on a file handle and the fd passed to ioctl() is not the file referenced in the handle, iget the file once and pass it into the scrub code. This amortizes the untrusted iget lookup over /all/ the scrubbers mentioned in the scrubv call. When running fstests in "rebuild all metadata after each test" mode, I observed a 10% reduction in runtime on account of avoiding repeated inobt lookups. Signed-off-by:
-
Darrick J. Wong authored
Introduce a variant on XFS_SCRUB_METADATA that allows for a vectored mode. The caller specifies the principal metadata object that they want to scrub (allocation group, inode, etc.) once, followed by an array of scrub types they want called on that object. The kernel runs the scrub operations and writes the output flags and errno code to the corresponding array element. A new pseudo scrub type BARRIER is introduced to force the kernel to return to userspace if any corruptions have been found when scrubbing the previous scrub types in the array. This enables userspace to schedule, for example, the sequence: 1. data fork 2. barrier 3. directory If the data fork scrub is clean, then the kernel will perform the directory scrub. If not, the barrier in 2 will exit back to userspace. The alternative would have been an interface where userspace passes a pointer to an empty buffer, and the kernel formats that with xfs_scrub_vecs that tell userspace what it scrubbed and what the outcome was. With that the kernel would have to communicate that the buffer needed to have been at least X size, even though for our cases XFS_SCRUB_TYPE_NR + 2 would always be enough. Compared to that, this design keeps all the dependency policy and ordering logic in userspace where it already resides instead of duplicating it in the kernel. The downside of that is that it needs the barrier logic. When running fstests in "rebuild all metadata after each test" mode, I observed a 10% reduction in runtime due to fewer transitions across the system call boundary. Signed-off-by:
-
Darrick J. Wong authored
If the transaction allocation in the !orphanage_available case of xrep_nlinks_repair_inode fails, we need to drop the IOLOCK of the file being scrubbed before exiting. Found by fuzzing u3.sfdir3.list[1].name = zeroes in xfs/1546. Signed-off-by:
-
Darrick J. Wong authored
Back when I wrote commit a03297a0, I had thought that we'd be doing users a favor by only marking inodes dontcache at the end of a scrub operation, and only if there's only one reference to that inode. This was more or less true back when I_DONTCACHE was an XFS iflag and the only thing it did was change the outcome of xfs_fs_drop_inode to 1. Note: If there are dentries pointing to the inode when scrub finishes, the inode will have positive i_count and stay around in cache until dentry reclaim. But now we have d_mark_dontcache, which cause the inode *and* the dentries attached to it all to be marked I_DONTCACHE, which means that we drop the dentries ASAP, which drops the inode ASAP. This is bad if scrub found problems with the inode, because now they can be scheduled for inactivation, which can cause inodegc to trip on it and shut down the filesystem. Even if the inode isn't bad, this is still suboptimal because phases 3-7 each initiate inode scans. Dropping the inode immediately during phase 3 is silly because phase 5 will reload it and drop it immediately, etc. It's fine to mark the inodes dontcache, but if there have been accesses to the file that set up dentries, we should keep them. I validated this by setting up ftrace to capture xfs_iget_recycle* tracepoints and ran xfs/285 for 30 seconds. With current djwong-wtf I saw ~30,000 recycle events. I then dropped the d_mark_dontcache calls and set XFS_IGET_DONTCACHE, and the recycle events dropped to ~5,000 per 30 seconds. Therefore, grab the inode with XFS_IGET_DONTCACHE, which only has the effect of setting I_DONTCACHE for cache misses. Remove the d_mark_dontcache call that can happen in xchk_irele. Fixes: a03297a0 ("xfs: manage inode DONTCACHE status at irele time") Signed-off-by:
-
Darrick J. Wong authored
Repair corruptions in the directory tree itself. Cycles are broken by removing an incoming parent->child link. Multiply-owned directories are fixed by pruning the extra parent -> child links Disconnected subtrees are reconnected to the lost and found. Signed-off-by:
-
Darrick J. Wong authored
Move the scrub ioctl handler to scrub.c to keep the code together and to reduce unnecessary code when CONFIG_XFS_ONLINE_SCRUB=n. Signed-off-by:
-
Darrick J. Wong authored
Report directories that are the source of corruption in the directory tree. Signed-off-by:
-
Darrick J. Wong authored
We really don't want to call cond_resched every single time we go through a loop in scrub -- there may be billions of records, and probing into the scheduler itself has overhead. Reduce this overhead by only calling cond_resched 10x per second; and add a counter so that we only check jiffies once every 1000 records or so. Surprisingly, this reduces scrub-only fstests runtime by about 2%. I used the bmapinflate xfs_db command to produce a billion-extent file and this stupid gadget reduced the scrub runtime by about 4%. From a stupid microbenchmark of calling these things 1 billion times, I estimate that cond_resched costs about 5.5ns per call; jiffes costs about 0.3ns per read; and fatal_signal_pending costs about 0.4ns per call. Signed-off-by:
-
Darrick J. Wong authored
The runtime parent pointer update code expects that any file being moved around the directory tree already has an attr fork. However, if we had to rebuild an inode core record, there's a chance that we zeroed forkoff as part of the inode to pass the iget verifiers. Therefore, if we performed any repairs on an inode core, ensure that the inode has a nonzero forkoff before unlocking the inode. Signed-off-by:
-
Darrick J. Wong authored
Add a dirent update hook so that we can detect directory tree updates that affect any of the paths found by this scrubber and force it to rescan. Signed-off-by:
-
Darrick J. Wong authored
Since the parent pointer scrubber does not exhaustively search the filesystem for missing parent pointers, it doesn't have a good way to determine that there are pointers missing from an otherwise uncorrupt xattr structure. Instead, for nondirectories it employs a heuristic of comparing the file link count to the number of parent pointers found. However, we don't want this heuristic flagging a false corruption after a repair has actually scanned the entire filesystem to rebuild the parent pointers. Therefore, reset the file link count in this one case because we actually know the correct link count. Signed-off-by:
-
Darrick J. Wong authored
Create a new scrubber that detects corruptions within the directory tree structure itself. It can detect directories with multiple parents; loops within the directory tree; and directory loops not accessible from the root. Signed-off-by:
-
Darrick J. Wong authored
Adapt the orphanage's adoption code to update the child file's parent pointers as part of the reparenting process. Also ensure that the child has an attr fork to receive the parent pointer update, since the runtime code assumes one exists. Signed-off-by:
-
Darrick J. Wong authored
Once we've assembled all the parent pointers for a file, we need to commit the new dataset atomically to that file. Parent pointer records are embedded in the xattr structure, which means that we must write a new extended attribute structure, again, atomically. Therefore, we must copy the non-parent-pointer attributes from the file being repaired into the temporary file's extended attributes and then call the atomic extent swap mechanism to exchange the blocks. Signed-off-by:
-
Darrick J. Wong authored
Add a second callback function to xchk_xattr_walk so that we can do something in between attr leaf blocks. This will be used by the next patch to see if we should flush cached parent pointer updates to constrain memory usage. Signed-off-by:
-
Darrick J. Wong authored
Split this function into two pieces -- one to make the actual changes to the inode core to add the attr fork, and another one to deal with getting the transaction and locking the inodes. The next couple of patches will need this to be split into two. One patch implements committing new parent pointer recordsets to damaged files. If one file has an attr fork and the other does not, we have to create the missing attr fork before the atomic swap transaction, and can use the behavior encoded in the current xfs_bmap_add_attrfork. The second patch adapts /lost+found adoptions to handle parent pointers correctly. The adoption process will add a parent pointer to a child that is being moved to /lost+found, but this requires that the attr fork already exists. We don't know if we're actually going to commit the adoption until we've already reserved a transaction and taken the ILOCKs, which means that we must have a way to bypass the start of the current xfs_bmap_add_attrfork. Therefore, create xfs_attr_add_fork as the helper that creates a transaction and takes locks; and make xfs_bmap_add_attrfork the function that updates the inode core and allocates the incore attr fork. Signed-off-by:
-
Darrick J. Wong authored
Remove this assertion about the inode not having an attr fork from xfs_bmap_add_attrfork because the function handles that case just fine. Weirder still, the function actually /requires/ the caller not to hold the ILOCK, which means that its accesses are not stabilized. Signed-off-by:
-
