1. 16 Jul, 2014 2 commits
  2. 14 Jul, 2014 1 commit
  3. 04 Jul, 2014 10 commits
    • Jiri Slaby's avatar
      Linux 3.12.24 · 8097be3b
      Jiri Slaby authored
      8097be3b
    • Jie Liu's avatar
      xfs: don't perform discard if the given range length is less than block size · 0af3f136
      Jie Liu authored
      commit f9fd0135 upstream.
      
      For discard operation, we should return EINVAL if the given range length
      is less than a block size, otherwise it will go through the file system
      to discard data blocks as the end range might be evaluated to -1, e.g,
      /xfs7: 9811378176 bytes were trimmed
      
      This issue can be triggered via xfstests/generic/288.
      
      Also, it seems to get the request queue pointer via bdev_get_queue()
      instead of the hard code pointer dereference is not a bad thing.
      Signed-off-by: default avatarJie Liu <jeff.liu@oracle.com>
      Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      Signed-off-by: default avatarJiri Slaby <jslaby@suse.cz>
      0af3f136
    • Dave Chinner's avatar
      xfs: xfs_remove deadlocks due to inverted AGF vs AGI lock ordering · f7009499
      Dave Chinner authored
      commit 27320369 upstream.
      
      Removing an inode from the namespace involves removing the directory
      entry and dropping the link count on the inode. Removing the
      directory entry can result in locking an AGF (directory blocks were
      freed) and removing a link count can result in placing the inode on
      an unlinked list which results in locking an AGI.
      
      The big problem here is that we have an ordering constraint on AGF
      and AGI locking - inode allocation locks the AGI, then can allocate
      a new extent for new inodes, locking the AGF after the AGI.
      Similarly, freeing the inode removes the inode from the unlinked
      list, requiring that we lock the AGI first, and then freeing the
      inode can result in an inode chunk being freed and hence freeing
      disk space requiring that we lock an AGF.
      
      Hence the ordering that is imposed by other parts of the code is AGI
      before AGF. This means we cannot remove the directory entry before
      we drop the inode reference count and put it on the unlinked list as
      this results in a lock order of AGF then AGI, and this can deadlock
      against inode allocation and freeing. Therefore we must drop the
      link counts before we remove the directory entry.
      
      This is still safe from a transactional point of view - it is not
      until we get to xfs_bmap_finish() that we have the possibility of
      multiple transactions in this operation. Hence as long as we remove
      the directory entry and drop the link count in the first transaction
      of the remove operation, there are no transactional constraints on
      the ordering here.
      
      Change the ordering of the operations in the xfs_remove() function
      to align the ordering of AGI and AGF locking to match that of the
      rest of the code.
      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>
      Signed-off-by: default avatarJiri Slaby <jslaby@suse.cz>
      f7009499
    • Jie Liu's avatar
      xfs: fix the extent count when allocating an new indirection array entry · 67820ad0
      Jie Liu authored
      commit bb86d21c upstream.
      
      At xfs_iext_add(), if extent(s) are being appended to the last page in
      the indirection array and the new extent(s) don't fit in the page, the
      number of extents(erp->er_extcount) in a new allocated entry should be
      the minimum value between count and XFS_LINEAR_EXTS, instead of count.
      
      For now, there is no existing test case can demonstrates a problem with
      the er_extcount being set incorrectly here, but it obviously like a bug.
      Signed-off-by: default avatarJie Liu <jeff.liu@oracle.com>
      Reviewed-by: default avatarBen Myers <bpm@sgi.com>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      Signed-off-by: default avatarJiri Slaby <jslaby@suse.cz>
      67820ad0
    • Geyslan G. Bem's avatar
      xfs: fix possible NULL dereference in xlog_verify_iclog · 9af76725
      Geyslan G. Bem authored
      commit 643f7c4e upstream.
      
      In xlog_verify_iclog a debug check of the incore log buffers prints an
      error if icptr is null and then goes on to dereference the pointer
      regardless.  Convert this to an assert so that the intention is clear.
      This was reported by Coverty.
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      Reviewed-by: default avatarEric Sandeen <sandeen@redhat.com>
      Signed-off-by: default avatarJiri Slaby <jslaby@suse.cz>
      9af76725
    • Dave Chinner's avatar
      xfs: prevent stack overflows from page cache allocation · 71024660
      Dave Chinner authored
      commit ad22c7a0 upstream.
      
      Page cache allocation doesn't always go through ->begin_write and
      hence we don't always get the opportunity to set the allocation
      context to GFP_NOFS. Failing to do this means we open up the direct
      relcaim stack to recurse into the filesystem and consume a
      significant amount of stack.
      
      On RHEL6.4 kernels we are seeing ra_submit() and
      generic_file_splice_read() from an nfsd context recursing into the
      filesystem via the inode cache shrinker and evicting inodes. This is
      causing truncation to be run (e.g EOF block freeing) and causing
      bmap btree block merges and free space btree block splits to occur.
      These btree manipulations are occurring with the call chain already
      30 functions deep and hence there is not enough stack space to
      complete such operations.
      
      To avoid these specific overruns, we need to prevent the page cache
      allocation from recursing via direct reclaim. We can do that because
      the allocation functions take the allocation context from that which
      is stored in the mapping for the inode. We don't set that right now,
      so the default is GFP_HIGHUSER_MOVABLE, which is effectively a
      GFP_KERNEL context. We need it to be the equivalent of GFP_NOFS, so
      when we initialise an inode, set the mapping gfp mask appropriately.
      
      This makes the use of AOP_FLAG_NOFS redundant from other parts of
      the XFS IO path, so get rid of it.
      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>
      Signed-off-by: default avatarJiri Slaby <jslaby@suse.cz>
      71024660
    • Eric Sandeen's avatar
      xfs: don't break from growfs ag update loop on error · e465c957
      Eric Sandeen authored
      commit 59e5a0e8 upstream.
      
      When xfs_growfs_data_private() is updating backup superblocks,
      it bails out on the first error encountered, whether reading or
      writing:
      
      * If we get an error writing out the alternate superblocks,
      * just issue a warning and continue.  The real work is
      * already done and committed.
      
      This can cause a problem later during repair, because repair
      looks at all superblocks, and picks the most prevalent one
      as correct.  If we bail out early in the backup superblock
      loop, we can end up with more "bad" matching superblocks than
      good, and a post-growfs repair may revert the filesystem to
      the old geometry.
      
      With the combination of superblock verifiers and old bugs,
      we're more likely to encounter read errors due to verification.
      
      And perhaps even worse, we don't even properly write any of the
      newly-added superblocks in the new AGs.
      
      Even with this change, growfs will still say:
      
        xfs_growfs: XFS_IOC_FSGROWFSDATA xfsctl failed: Structure needs cleaning
        data blocks changed from 319815680 to 335216640
      
      which might be confusing to the user, but it at least communicates
      that something has gone wrong, and dmesg will probably highlight
      the need for an xfs_repair.
      
      And this is still best-effort; if verifiers fail on more than
      half the backup supers, they may still "win" - but that's probably
      best left to repair to more gracefully handle by doing its own
      strict verification as part of the backup super "voting."
      Signed-off-by: default avatarEric Sandeen <sandeen@redhat.com>
      Acked-by: default avatarDave Chinner <david@fromorbit.com>
      Reviewed-by: default avatarMark Tinguely <tinguely@sgi.com>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      Signed-off-by: default avatarJiri Slaby <jslaby@suse.cz>
      e465c957
    • Eric Sandeen's avatar
      xfs: don't emit corruption noise on fs probes · 22a16208
      Eric Sandeen authored
      commit 31625f28 upstream.
      
      If we get EWRONGFS due to probing of non-xfs filesystems,
      there's no need to issue the scary corruption error and backtrace.
      Signed-off-by: default avatarEric Sandeen <sandeen@redhat.com>
      Reviewed-by: default avatarMark Tinguely <tinguely@sgi.com>
      Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      Signed-off-by: default avatarJiri Slaby <jslaby@suse.cz>
      22a16208
    • Dave Chinner's avatar
      xfs: prevent deadlock trying to cover an active log · 192fedc5
      Dave Chinner authored
      commit 2c6e24ce upstream.
      
      Recent analysis of a deadlocked XFS filesystem from a kernel
      crash dump indicated that the filesystem was stuck waiting for log
      space. The short story of the hang on the RHEL6 kernel is this:
      
      	- the tail of the log is pinned by an inode
      	- the inode has been pushed by the xfsaild
      	- the inode has been flushed to it's backing buffer and is
      	  currently flush locked and hence waiting for backing
      	  buffer IO to complete and remove it from the AIL
      	- the backing buffer is marked for write - it is on the
      	  delayed write queue
      	- the inode buffer has been modified directly and logged
      	  recently due to unlinked inode list modification
      	- the backing buffer is pinned in memory as it is in the
      	  active CIL context.
      	- the xfsbufd won't start buffer writeback because it is
      	  pinned
      	- xfssyncd won't force the log because it sees the log as
      	  needing to be covered and hence wants to issue a dummy
      	  transaction to move the log covering state machine along.
      
      Hence there is no trigger to force the CIL to the log and hence
      unpin the inode buffer and therefore complete the inode IO, remove
      it from the AIL and hence move the tail of the log along, allowing
      transactions to start again.
      
      Mainline kernels also have the same deadlock, though the signature
      is slightly different - the inode buffer never reaches the delayed
      write lists because xfs_buf_item_push() sees that it is pinned and
      hence never adds it to the delayed write list that the xfsaild
      flushes.
      
      There are two possible solutions here. The first is to simply force
      the log before trying to cover the log and so ensure that the CIL is
      emptied before we try to reserve space for the dummy transaction in
      the xfs_log_worker(). While this might work most of the time, it is
      still racy and is no guarantee that we don't get stuck in
      xfs_trans_reserve waiting for log space to come free. Hence it's not
      the best way to solve the problem.
      
      The second solution is to modify xfs_log_need_covered() to be aware
      of the CIL. We only should be attempting to cover the log if there
      is no current activity in the log - covering the log is the process
      of ensuring that the head and tail in the log on disk are identical
      (i.e. the log is clean and at idle). Hence, by definition, if there
      are items in the CIL then the log is not at idle and so we don't
      need to attempt to cover it.
      
      When we don't need to cover the log because it is active or idle, we
      issue a log force from xfs_log_worker() - if the log is idle, then
      this does nothing.  However, if the log is active due to there being
      items in the CIL, it will force the items in the CIL to the log and
      unpin them.
      
      In the case of the above deadlock scenario, instead of
      xfs_log_worker() getting stuck in xfs_trans_reserve() attempting to
      cover the log, it will instead force the log, thereby unpinning the
      inode buffer, allowing IO to be issued and complete and hence
      removing the inode that was pinning the tail of the log from the
      AIL. At that point, everything will start moving along again. i.e.
      the xfs_log_worker turns back into a watchdog that can alleviate
      deadlocks based around pinned items that prevent the tail of the log
      from being moved...
      Signed-off-by: default avatarDave Chinner <dchinner@redhat.com>
      Reviewed-by: default avatarEric Sandeen <sandeen@redhat.com>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      Signed-off-by: default avatarJiri Slaby <jslaby@suse.cz>
      192fedc5
    • Jie Liu's avatar
      xfs: fix the wrong new_size/rnew_size at xfs_iext_realloc_direct() · 5b093812
      Jie Liu authored
      commit 17ec81c1 upstream.
      
      At xfs_iext_realloc_direct(), the new_size is changed by adding
      if_bytes if originally the extent records are stored at the inline
      extent buffer, and we have to switch from it to a direct extent
      list for those new allocated extents, this is wrong. e.g,
      
      Create a file with three extents which was showing as following,
      
      xfs_io -f -c "truncate 100m" /xfs/testme
      
      for i in $(seq 0 5 10); do
      	offset=$(($i * $((1 << 20))))
      	xfs_io -c "pwrite $offset 1m" /xfs/testme
      done
      
      Inline
      ------
      irec:	if_bytes	bytes_diff	new_size
      1st	0		16		16
      2nd	16		16		32
      
      Switching
      ---------						rnew_size
      3rd	32		16		48 + 32 = 80	roundup=128
      
      In this case, the desired value of new_size should be 48, and then
      it will be roundup to 64 and be assigned to rnew_size.
      
      However, this issue has been covered by resetting the if_bytes to
      the new_size which is calculated at the begnning of xfs_iext_add()
      before leaving out this function, and in turn make the rnew_size
      correctly again. Hence, this can not be detected via xfstestes.
      
      This patch fix above problem and revise the new_size comments at
      xfs_iext_realloc_direct() to make it more readable.  Also, fix the
      comments while switching from the inline extent buffer to a direct
      extent list to reflect this change.
      Signed-off-by: default avatarJie Liu <jeff.liu@oracle.com>
      Reviewed-by: default avatarDave Chinner <dchinner@redhat.com>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      Signed-off-by: default avatarJiri Slaby <jslaby@suse.cz>
      5b093812
  4. 02 Jul, 2014 27 commits