- 10 Sep, 2013 7 commits
-
-
Dave Chinner authored
Saw this on generic/270 after a DQALLOC transaction overrun shutdown: XFS: Assertion failed: !(bip->bli_item.li_flags & XFS_LI_IN_AIL), file: fs/xfs/xfs_buf_item.c, line: 952 ..... xfs_buf_item_relse+0x4f/0xd0 xfs_buf_item_unlock+0x1b4/0x1e0 xfs_trans_free_items+0x7d/0xb0 xfs_trans_cancel+0x13c/0x1b0 xfs_symlink+0x37e/0xa60 .... When a transaction abort occured. If we are aborting a transaction and trigger this code path, then the item may be dirty. If the item is dirty, then it may be in the AIL. Hence if we are aborting, we need to check if the item is in the AIL and remove it before freeing it. Signed-off-by:
Dave Chinner <dchinner@redhat.com> Reviewed-by:
Mark Tinguely <tinguely@sgi.com> Signed-off-by:
Ben Myers <bpm@sgi.com>
-
Dave Chinner authored
We have quite a few places now where we do: x = kmem_zalloc(large size) if (!x) x = kmem_zalloc_large(large size) and do a similar dance when freeing the memory. kmem_free() already does the correct freeing dance, and kmem_zalloc_large() is only ever called in these constructs, so just factor it all into kmem_zalloc_large() and kmem_free(). Signed-off-by:
-
Dave Chinner authored
Ever since increasing the number of supported ACLs from 25 to as many as can fit in an xattr, there have been reports of order 4 memory allocations failing in the ACL code. Fix it in the same way we've fixed all the xattr read/write code that has the same problem. Signed-off-by:
-
Dave Chinner authored
When splitting the root of the da btree, we shuffled data between buffers and the structures that track them. At one point, we copy data and state from one buffer to another, including the ops associated with the buffer. When we do this, we also need to copy the buffer type associated with the buf log item so that the buffer is logged correctly. If we don't do that, log recovery won't recognise it and hence it won't recalculate the CRC on the buffer after recovery. This leads to a directory block that can't be read after recovery has run. Found by inspection after finding the same problem with remote symlink buffers. Signed-off-by:
-
Dave Chinner authored
The logging of a remote symlink block does not set the buffer type being logged, and hence on recovery the type of buffer is not recognised and hence CRCs are not calculated after replay. This results in log recoery throwing: XFS (vdc): Unknown buffer type 0 errors, and subsequent reads of the symlink failing CRC verification. Found via fsstress + godown. Reported by: Michael L. Semon <mlsemon35@gmail.com> Signed-off-by:
Christoph Hellwig <hch@lst.de> Signed-off-by:
-
Dave Chinner authored
This is the recovery side of the btree block owner change operation performed by swapext on CRC enabled filesystems. We detect that an owner change is needed by the flag that has been placed on the inode log format flag field. Because the inode recovery is being replayed after the buffers that make up the BMBT in the given checkpoint, we can walk all the buffers and directly modify them when we see the flag set on an inode. Because the inode can be relogged and hence present in multiple chekpoints with the "change owner" flag set, we could do multiple passes across the inode to do this change. While this isn't optimal, we can't directly ignore the flag as there may be multiple independent swap extent operations being replayed on the same inode in different checkpoints so we can't ignore them. Further, because the owner change operation uses ordered buffers, we might have buffers that are newer on disk than the current checkpoint and so already have the owner changed in them. Hence we cannot just peek at a buffer in the tree and check that it has the correct owner and assume that the change was completed. So, for the moment just brute force the owner change every time we see an inode with the flag set. Note that we have to be careful here because the owner of the buffers may point to either the old owner or the new owner. Currently the verifier can't verify the owner directly, so there is no failure case here right now. If we verify the owner exactly in future, then we'll have to take this into account. This was tested in terms of normal operation via xfstests - all of the fsr tests now pass without failure. however, we really need to modify xfs/227 to stress v3 inodes correctly to ensure we fully cover this case for v5 filesystems. In terms of recovery testing, I used a hacked version of xfs_fsr that held the temp inode open for a few seconds before exiting so that the filesystem could be shut down with an open owner change recovery flags set on at least the temp inode. fsr leaves the temp inode unlinked and in btree format, so this was necessary for the owner change to be reliably replayed. logprint confirmed the tmp inode in the log had the correct flag set: INO: cnt:3 total:3 a:0x69e9e0 len:56 a:0x69ea20 len:176 a:0x69eae0 len:88 INODE: #regs:3 ino:0x44 flags:0x209 dsize:88 ^^^^^ 0x200 is set, indicating a data fork owner change needed to be replayed on inode 0x44. A printk in the revoery code confirmed that the inode change was recovered: XFS (vdc): Mounting Filesystem XFS (vdc): Starting recovery (logdev: internal) recovering owner change ino 0x44 XFS (vdc): Version 5 superblock detected. This kernel L support enabled! Use of these features in this kernel is at your own risk! XFS (vdc): Ending recovery (logdev: internal) The script used to test this was: $ cat ./recovery-fsr.sh #!/bin/bash dev=/dev/vdc mntpt=/mnt/scratch testfile=$mntpt/testfile umount $mntpt mkfs.xfs -f -m crc=1 $dev mount $dev $mntpt chmod 777 $mntpt for i in `seq 10000 -1 0`; do xfs_io -f -d -c "pwrite $(($i * 4096)) 4096" $testfile > /dev/null 2>&1 done xfs_bmap -vp $testfile |head -20 xfs_fsr -d -v $testfile & sleep 10 /home/dave/src/xfstests-dev/src/godown -f $mntpt wait umount $mntpt xfs_logprint -t $dev |tail -20 time mount $dev $mntpt xfs_bmap -vp $testfile umount $mntpt $ Signed-off-by: -
Dave Chinner authored
For CRC enabled filesystems, we can't just swap inode forks from one inode to another when defragmenting a file - the blocks in the inode fork bmap btree contain pointers back to the owner inode. Hence if we are to swap the inode forks we have to atomically modify every block in the btree during the transaction. We are doing an entire fork swap here, so we could create a new transaction item type that indicates we are changing the owner of a certain structure from one value to another. If we combine this with ordered buffer logging to modify all the buffers in the tree, then we can change the buffers in the tree without needing log space for the operation. However, this then requires log recovery to perform the modification of the owner information of the objects/structures in question. This does introduce some interesting ordering details into recovery: we have to make sure that the owner change replay occurs after the change that moves the objects is made, not before. Hence we can't use a separate log item for this as we have no guarantee of strict ordering between multiple items in the log due to the relogging action of asynchronous transaction commits. Hence there is no "generic" method we can use for changing the ownership of arbitrary metadata structures. For inode forks, however, there is a simple method of communicating that the fork contents need the owner rewritten - we can pass a inode log format flag for the fork for the transaction that does a fork swap. This flag will then follow the inode fork through relogging actions so when the swap actually gets replayed the ownership can be changed immediately by log recovery. So that gives us a simple method of "whole fork" exchange between two inodes. This is relatively simple to implement, so it makes sense to do this as an initial implementation to support xfs_fsr on CRC enabled filesytems in the same manner as we do on existing filesystems. This commit introduces the swapext driven functionality, the recovery functionality will be in a separate patch. Signed-off-by:
-
- 09 Sep, 2013 3 commits
-
-
Dave Chinner authored
Calling xfs_dir3_leaf_hdr_from_disk() in a verifier before validating the magic numbers in the buffer results in ASSERT failures due to mismatching magic numbers when a corruption occurs. Seeing as the verifier is supposed to catch the corruption and pass it back to the caller, having the verifier assert fail on error defeats the purpose of detecting the errors in the first place. Check the magic numbers direct from the buffer before decoding the header. Signed-off-by:
-
Dave Chinner authored
A couple of simple locking annotations and 0 vs NULL warnings. Nothing that changes any code behaviour, just removes build noise. Signed-off-by:
-
Dave Chinner authored
sparse reports: fs/xfs/xfs_log_recover.c:2017:24: sparse: cast to restricted __be64 Because I used the wrong structure for the on-disk superblock cast in 50d5c8d8 ("xfs: check LSN ordering for v5 superblocks during recovery"). Fix it. Reported-by: kbuild test robot Signed-off-by:
-
- 03 Sep, 2013 2 commits
-
-
Dave Chinner authored
So move it to a header file shared with userspace. Signed-off-by:
-
Dave Chinner authored
So fix up the export in xfs_dir2.h that is needed by userspace. <sigh> Now xfs_dir3_sfe_put_ino has been made static. Revert 98f7462c ("xfs: xfs_dir3_sfe_put_ino can be static") to being non static so that the code shared with userspace is identical again. Signed-off-by:
-
- 30 Aug, 2013 8 commits
-
-
Eric Sandeen authored
This ASSERT is testing an if_flags flag value against a di_aformat enum value. di_aformat is never assigned XFS_IFINLINE. This happens to work for now, because XFS_IFINLINE has the same value as XFS_DINODE_FMT_LOCAL, and that's tested just before we call this function. However, I think the intention is to assert that we have read in the data, i.e. XFS_IFINLINE on if_flags, before we use if_data. This is done in other places through the code as well. Signed-off-by:
Eric Sandeen <sandeen@redhat.com> Reviewed-by:
-
Dave Chinner authored
In optimising the CIL operations, some of the IOP_* macros for calling log item operations were removed. Remove the rest of them as Christoph requested. Signed-off-by:
Geoffrey Wehrman <gwehrman@sgi.com> Reviewed-by:
-
Dave Chinner authored
We've been seeing occasional problems with log space leaks and transaction underruns such as this for some time: XFS (dm-0): xlog_write: reservation summary: trans type = FSYNC_TS (36) unit res = 2740 bytes current res = -4 bytes total reg = 0 bytes (o/flow = 0 bytes) ophdrs = 0 (ophdr space = 0 bytes) ophdr + reg = 0 bytes num regions = 0 Turns out that xfstests generic/311 is reliably reproducing this problem with the test it runs at sequence 16 of it execution. It is a 100% reliable reproducer with the mkfs configuration of "-b size=1024 -m crc=1" on a 10GB scratch device. The problem? Inode forks in btree format are logged in memory format, not disk format (i.e. bmbt format, not bmdr format). That means there is a btree block header being logged, when such a structure is never written to the inode fork in bmdr format. The bmdr header in the inode is only 4 bytes, while the bmbt header is 24 bytes for v4 filesystems and 72 bytes for v5 filesystems. We currently reserve the inode size plus the rounded up overhead of a logging a buffer, which is 128 bytes. That means the reservation for a 512 byte inode is 640 bytes. What we can actually log is: inode core, data and attr fork = 512 bytes inode log format + log op header = 56 + 12 = 68 bytes data fork bmbt hdr = 24/72 bytes attr fork bmbt hdr = 24/72 bytes So, for a v2 inodes we can log at least 628 bytes, but if we split that inode over the end of the log across log buffers, we need to also another log op header, which takes us to 640 bytes. If there's another reservation taken out of this that I haven't taken into account (perhaps multiple iclog splits?) or I haven't corectly calculated the bmbt format space used (entirely possible), then we will overun it. For v3 inodes the maximum is actually 724 bytes, and even a single maximally sized btree format fork can blow it (652 bytes). And that's exactly what is happening with the FSYNC_TS transaction in the above output - it's consumed 644 bytes of space after the CIL context took the space reserved for it (2100 bytes). This problem has always been present in the XFS code - the btree format inode forks have always been logged in this manner. Hence there has always been the possibility of an overrun with such a transaction. The CRC code has just exposed it frequently enough to be able to debug and understand the root cause.... So, let's fix all the inode log space reservations. [ I'm so glad we spent the effort to clean up the transaction reservation code. This is an easy fix now. ] Signed-off-by:
-
Brian Foster authored
The call to xfs_inobt_get_rec() in xfs_dialloc_ag() passes 'j' as the output status variable. The immediately following XFS_WANT_CORRUPTED_GOTO() checks the value of 'i,' which is from the previous lookup call and has already been checked. Fix the corruption check to use 'j.' Signed-off-by:
Brian Foster <bfoster@redhat.com> Reviewed-by:
-
Dave Chinner authored
CRC enabled filesystems fail log recovery with 100% reliability on xfstests xfs/085 with the following failure: XFS (vdb): Mounting Filesystem XFS (vdb): Starting recovery (logdev: internal) XFS (vdb): Corruption detected. Unmount and run xfs_repair XFS (vdb): bad inode magic/vsn daddr 144 #0 (magic=0) XFS: Assertion failed: 0, file: fs/xfs/xfs_inode_buf.c, line: 95 The problem is that the inode buffer has not been recovered before the readahead on the inode buffer is issued. The checkpoint being recovered actually allocates the inode chunk we are doing readahead from, so what comes from disk during readahead is essentially random and the verifier barfs on it. This inode buffer readahead problem affects non-crc filesystems, too, but xfstests does not trigger it at all on such configurations.... Signed-off-by:
-
Dave Chinner authored
Log recovery has some strict ordering requirements which unordered or reordered metadata writeback can defeat. This can occur when an item is logged in a transaction, written back to disk, and then logged in a new transaction before the tail of the log is moved past the original modification. The result of this is that when we read an object off disk for recovery purposes, the buffer that we read may not contain the object type that recovery is expecting and hence at the end of the checkpoint being recovered we have an invalid object in memory. This isn't usually a problem, as recovery will then replay all the other checkpoints and that brings the object back to a valid and correct state, but the issue is that while the object is in the invalid state it can be flushed to disk. This results in the object verifier failing and triggering a corruption shutdown of log recover. This is correct behaviour for the verifiers - the problem is that we are not detecting that the object we've read off disk is newer than the transaction we are replaying. All metadata in v5 filesystems has the LSN of it's last modification stamped in it. This enabled log recover to read that field and determine the age of the object on disk correctly. If the LSN of the object on disk is older than the transaction being replayed, then we replay the modification. If the LSN of the object matches or is more recent than the transaction's LSN, then we should avoid overwriting the object as that is what leads to the transient corrupt state. Signed-off-by:
-
Dave Chinner authored
When testing LSN ordering code for v5 superblocks, it was discovered that the the LSN embedded in the generic btree blocks was occasionally uninitialised. These values didn't get written to disk by metadata writeback - they got written by previous transactions in log recovery. The issue is here that the when the block is first allocated and initialised, the LSN field was not initialised - it gets overwritten before IO is issued on the buffer - but the value that is logged by transactions that modify the header before it is written to disk (and initialised) contain garbage. Hence the first recovery of the buffer will stamp garbage into the LSN field, and that can cause subsequent transactions to not replay correctly. The fix is simply to initialise the bb_lsn field to zero when we initialise the block for the first time. Signed-off-by:
-
Dave Chinner authored
XFS: Assertion failed: first <= last && last < BBTOB(bp->b_length), file: fs/xfs/xfs_trans_buf.c, line: 568 The calculation doesn't take into account the size of the dir v3 header, so overestimates the hash entries in a node. This causes directory buffer overruns when splitting and merging nodes. Signed-off-by:
-
- 29 Aug, 2013 2 commits
-
-
Dave Chinner authored
xfstests xfs/087 fails 100% reliably with this assert: XFS (vdb): Mounting Filesystem XFS (vdb): Starting recovery (logdev: internal) XFS: Assertion failed: bp->b_flags & XBF_STALE, file: fs/xfs/xfs_buf.c, line: 548 while trying to read a dquot buffer in xlog_recover_dquot_ra_pass2(). The issue is that the buffer length to read that is passed to xfs_buf_readahead is in units of filesystem blocks, not disk blocks. (i.e. FSB, not daddr). Fix it but putting the correct conversion in place. Signed-off-by:
-
Dave Chinner authored
When doing readhaead in log recovery, we check to see if buffers are cancelled before doing readahead. If we find a cancelled buffer, however, we always decrement the reference count we have on it, and that means that readahead is causing a double decrement of the cancelled buffer reference count. This results in log recovery *replaying cancelled buffers* as the actual recovery pass does not find the cancelled buffer entry in the commit phase of the second pass across a transaction. On debug kernels, this results in an ASSERT failure like so: XFS: Assertion failed: !(flags & XFS_BLF_CANCEL), file: fs/xfs/xfs_log_recover.c, line: 1815 xfstests generic/311 reproduces this ASSERT failure with 100% reproducability. Fix it by making readahead only peek at the buffer cancelled state rather than the full accounting that xlog_check_buffer_cancelled() does. Signed-off-by:
-
- 26 Aug, 2013 2 commits
-
-
Dan Carpenter authored
The "di_size" variable comes from the disk and it's a signed 64 bit. We check the upper limit but we should check for negative numbers as well. Signed-off-by:
Dan Carpenter <dan.carpenter@oracle.com> Reviewed-by:
-
Fengguang Wu authored
TO: Dave Chinner <david@fromorbit.com> CC: Ben Myers <bpm@sgi.com> CC: linux-kernel@vger.kernel.org Signed-off-by:
Fengguang Wu <fengguang.wu@intel.com> Reviewed-by:
-
- 23 Aug, 2013 2 commits
-
-
Zhi Yong Wu authored
It can take a long time to run log recovery operation because it is single threaded and is bound by read latency. We can find that it took most of the time to wait for the read IO to occur, so if one object readahead is introduced to log recovery, it will obviously reduce the log recovery time. Log recovery time stat: w/o this patch w/ this patch real: 0m15.023s 0m7.802s user: 0m0.001s 0m0.001s sys: 0m0.246s 0m0.107s Signed-off-by:Zhi Yong Wu <wuzhy@linux.vnet.ibm.com> Reviewed-by:
-
Jie Liu authored
At xfs_ail_min(), we do check if the AIL list is empty or not before returning the first item in it with list_empty() and list_first_entry(). This can be simplified a bit with a new list operation routine that is the list_first_entry_or_null() which has been introduced by: commit 6d7581e6 list: introduce list_first_entry_or_null v2: make xfs_ail_min() as a static inline function and move it to xfs_trans_priv.h as per Dave Chinner's comments. Signed-off-by:
Jie Liu <jeff.liu@oracle.com> Reviewed-by:
-
- 22 Aug, 2013 4 commits
-
-
Richard Weinberger authored
Currently the code initializizes mp->m_icsb_mutex and other things _after_ register_hotcpu_notifier(). As the notifier takes mp->m_icsb_mutex it can happen that it takes the lock before it's initialization. Signed-off-by:
Richard Weinberger <richard@nod.at> Reviewed-by:
-
Mark Tinguely authored
Add XFS superblock v4 support for the file type field in the directory entry feature. This support adds a feature bit for version 4 superblocks and leaves the original superblock 5 incompatibility bit. Signed-off-by:
-
Dave Chinner authored
Add support to propagate and add filetype values into the on-disk directs. This involves passing the filetype into the xfs_da_args structure along with the name and namelength for direct operations, and encoding it into the dirent at the same time we write the inode number into the dirent. With write support, add the feature flag to the XFS_SB_FEAT_INCOMPAT_ALL mask so we can now mount filesystems with this feature set. Performance of directory recursion is now much improved. Parallel walk of ~50 million directory entries across hundreds of directories improves significantly. Unpatched, no CRCs: Walking via ls -R real 3m19.886s user 6m36.960s sys 28m19.087s THis is doing roughly 500 getdents() calls per second, and 250,000 inode lookups per second to determine the inode type at roughly 17,000 read IOPS. CPU usage is 90% kernel space. With dtype support patched in and the fileset recreated with CRCs enabled: Walking via ls -R real 0m31.316s user 6m32.975s sys 0m21.111s This is doing roughly 3500 getdents() calls per second at 16,000 IOPS. There are no inode lookups at all. CPU usages is almost 100% userspace. This is a big win for recursive directory walks that only need to find file names and file types. Signed-off-by:
-
Dave Chinner authored
Add support for the file type field in directory entries so that readdir can return the type of the inode the dirent points to to userspace without first having to read the inode off disk. The encoding of the type field is a single byte that is added to the end of the directory entry name length. For all intents and purposes, it appends a "hidden" byte to the name field which contains the type information. As the directory entry is already of dynamic size, helpers are already required to access and decode the direct entry structures. Hence the relevent extraction and iteration helpers are updated to understand the hidden byte. Helpers for reading and writing the filetype field from the directory entries are also added. Only the read helpers are used by this patch. It also adds all the code necessary to read the type information out of the dirents on disk. Further we add the superblock feature bit and helpers to indicate that we understand the on-disk format change. This is not a compatible change - existing kernels cannot read the new format successfully - so an incompatible feature flag is added. We don't yet allow filesystems to mount with this flag yet - that will be added once write support is added. Finally, the code to take the type from the VFS, convert it to an XFS on-disk type and put it into the xfs_name structures passed around is added, but the directory code does not use this field yet. That will be in the next patch. Signed-off-by:
-
- 21 Aug, 2013 1 commit
-
-
Dwight Engen authored
Acked-by:
Jeremy Kerr <jk@ozlabs.org> Tested-by:
Dwight Engen <dwight.engen@oracle.com> Reviewed-by:
Arnd Bergmann <arnd@arndb.de> Signed-off-by:
-
- 20 Aug, 2013 9 commits
-
-
Chandra Seetharaman authored
For XFS, add support for Q_XGETQSTATV quotactl command. Signed-off-by:
Chandra Seetharaman <sekharan@us.ibm.com> Reviewed-by:
Rich Johnston <rjohnston@sgi.com> Signed-off-by:
-
Chandra Seetharaman authored
XFS now supports three types of quotas (user, group and project). Current version of Q_XGETSTAT has support for only two types of quotas. In order to support three types of quotas, the interface, specifically struct fs_quota_stat, need to be expanded. Current version of fs_quota_stat does not allow expansion without breaking backward compatibility. So, a quotactl command and new fs_quota_stat structure need to be added. This patch adds a new command Q_XGETQSTATV to quotactl() which takes a new data structure fs_quota_statv. This new data structure provides support for future expansion and backward compatibility. Callers of the new quotactl command have to set the version of the data structure being passed, and kernel will fill as much data as requested. If the kernel does not support the user-space provided version, EINVAL will be returned. User-space can reduce the version number and call the same quotactl again. Signed-off-by:
Jan Kara <jack@suse.cz> Reviewed-by:
-
Zhi Yong Wu authored
Signed-off-by:
-
Zhi Yong Wu authored
Signed-off-by:
-
Zhi Yong Wu authored
Signed-off-by:
-
Zhi Yong Wu authored
Signed-off-by:
-
Zhi Yong Wu authored
Signed-off-by:
-
Zhi Yong Wu authored
Signed-off-by:
-
Zhi Yong Wu authored
Signed-off-by:
-
