1. 10 Sep, 2013 4 commits
    • Dave Chinner's avatar
      xfs: ensure we copy buffer type in da btree root splits · 0a4edc8f
      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: default avatarDave Chinner <dchinner@redhat.com>
      Reviewed-by: default avatarBen Myers <bpm@sgi.com>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      0a4edc8f
    • Dave Chinner's avatar
      xfs: set remote symlink buffer type for recovery · daf7b799
      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: default avatarDave Chinner <dchinner@redhat.com>
      Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      daf7b799
    • Dave Chinner's avatar
      xfs: recovery of swap extents operations for CRC filesystems · 638f4416
      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: default avatarDave Chinner <dchinner@redhat.com>
      Reviewed-by: default avatarMark Tinguely <tinguely@sgi.com>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      638f4416
    • Dave Chinner's avatar
      xfs: swap extents operations for CRC filesystems · 21b5c978
      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: default avatarDave Chinner <dchinner@redhat.com>
      Reviewed-by: default avatarMark Tinguely <tinguely@sgi.com>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      21b5c978
  2. 09 Sep, 2013 3 commits
  3. 03 Sep, 2013 2 commits
  4. 30 Aug, 2013 8 commits
    • Eric Sandeen's avatar
      Fix wrong flag ASSERT in xfs_attr_shortform_getvalue · 914ed44b
      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: default avatarEric Sandeen <sandeen@redhat.com>
      Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      914ed44b
    • Dave Chinner's avatar
      xfs: finish removing IOP_* macros. · 904c17e6
      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: default avatarDave Chinner <dchinner@redhat.com>
      Reviewed-by: default avatarGeoffrey Wehrman <gwehrman@sgi.com>
      Reviewed-by: default avatarMark Tinguely <tinguely@sgi.com>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      904c17e6
    • Dave Chinner's avatar
      xfs: inode log reservations are too small · 23956703
      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: default avatarDave Chinner <dchinner@redhat.com>
      Reviewed-by: default avatarMark Tinguely <tinguely@sgi.com>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      23956703
    • Brian Foster's avatar
      xfs: check correct status variable for xfs_inobt_get_rec() call · b121099d
      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: default avatarBrian Foster <bfoster@redhat.com>
      Reviewed-by: default avatarEric Sandeen <sandeen@redhat.com>
      Reviewed-by: default avatarDave Chinner <dchinner@redhat.com>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      b121099d
    • Dave Chinner's avatar
      xfs: inode buffers may not be valid during recovery readahead · d8914002
      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: default avatarDave Chinner <dchinner@redhat.com>
      Reviewed-by: default avatarBen Myers <bpm@sgi.com>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      d8914002
    • Dave Chinner's avatar
      xfs: check LSN ordering for v5 superblocks during recovery · 50d5c8d8
      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: default avatarDave Chinner <dchinner@redhat.com>
      Reviewed-by: default avatarMark Tinguely <tinguely@sgi.com>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      50d5c8d8
    • Dave Chinner's avatar
      xfs: btree block LSN escaping to disk uninitialised · b58fa554
      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: default avatarDave Chinner <dchinner@redhat.com>
      Reviewed-by: default avatarMark Tinguely <tinguely@sgi.com>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      b58fa554
    • Dave Chinner's avatar
      XFS: Assertion failed: first <= last && last < BBTOB(bp->b_length), file:... · 37804376
      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: default avatarDave Chinner <dchinner@redhat.com>
      Tested-by: default avatarBrian Foster <bfoster@redhat.com>
      Reviewed-by: default avatarMark Tinguely <tinguely@sgi.com>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      
      37804376
  5. 29 Aug, 2013 2 commits
    • Dave Chinner's avatar
      xfs: fix bad dquot buffer size in log recovery readahead · 0f0d3345
      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: default avatarDave Chinner <dchinner@redhat.com>
      Reviewed-by: default avatarBen Myers <bpm@sgi.com>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      0f0d3345
    • Dave Chinner's avatar
      xfs: don't account buffer cancellation during log recovery readahead · 84a5b730
      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: default avatarDave Chinner <dchinner@redhat.com>
      Reviewed-by: default avatarBen Myers <bpm@sgi.com>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      84a5b730
  6. 26 Aug, 2013 2 commits
  7. 23 Aug, 2013 2 commits
  8. 22 Aug, 2013 4 commits
    • Richard Weinberger's avatar
      xfs: Register hotcpu notifier after initialization · 46677e67
      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: default avatarRichard Weinberger <richard@nod.at>
      Reviewed-by: default avatarBen Myers <bpm@sgi.com>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      46677e67
    • Mark Tinguely's avatar
      xfs: add xfs sb v4 support for dirent filetype field · 3e3c51ce
      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: default avatarMark Tinguely <tinguely@sgi.com>
      Reviewed-by: default avatarGeoffrey Wehrman <gwehrman@sgi.com>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      3e3c51ce
    • Dave Chinner's avatar
      xfs: Add write support for dirent filetype field · 1c55cece
      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: default avatarDave Chinner <dchinner@redhat.com>
      Reviewed-by: default avatarMark Tinguely <tinguely@sgi.com>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      1c55cece
    • Dave Chinner's avatar
      xfs: Add read-only support for dirent filetype field · 0cb97766
      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: default avatarDave Chinner <dchinner@redhat.com>
      Reviewed-by: default avatarMark Tinguely <tinguely@sgi.com>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      0cb97766
  9. 21 Aug, 2013 1 commit
  10. 20 Aug, 2013 12 commits