1. 18 Nov, 2013 2 commits
    • Dave Chinner's avatar
      xfs: increase inode cluster size for v5 filesystems · 8f80587b
      Dave Chinner authored
      v5 filesystems use 512 byte inodes as a minimum, so read inodes in
      clusters that are effectively half the size of a v4 filesystem with
      256 byte inodes. For v5 fielsystems, scale the inode cluster size
      with the size of the inode so that we keep a constant 32 inodes per
      cluster ratio for all inode IO.
      
      This only works if mkfs.xfs sets the inode alignment appropriately
      for larger inode clusters, so this functionality is made conditional
      on mkfs doing the right thing. xfs_repair needs to know about
      the inode alignment changes, too.
      
      Wall time:
      	create	bulkstat	find+stat	ls -R	unlink
      v4	237s	161s		173s		201s	299s
      v5	235s	163s		205s		 31s	356s
      patched	234s	160s		182s		 29s	317s
      
      System time:
      	create	bulkstat	find+stat	ls -R	unlink
      v4	2601s	2490s		1653s		1656s	2960s
      v5	2637s	2497s		1681s		  20s	3216s
      patched	2613s	2451s		1658s		  20s	3007s
      
      So, wall time same or down across the board, system time same or
      down across the board, and cache hit rates all improve except for
      the ls -R case which is a pure cold cache directory read workload
      on v5 filesystems...
      
      So, this patch removes most of the performance and CPU usage
      differential between v4 and v5 filesystems on traversal related
      workloads.
      
      Note: while this patch is currently for v5 filesystems only, there
      is no reason it can't be ported back to v4 filesystems.  This hasn't
      been done here because bringing the code back to v4 requires
      forwards and backwards kernel compatibility testing.  i.e. to
      deterine if older kernels(*) do the right thing with larger inode
      alignments but still only using 8k inode cluster sizes. None of this
      testing and validation on v4 filesystems has been done, so for the
      moment larger inode clusters is limited to v5 superblocks.
      
      (*) a current default config v4 filesystem should mount just fine on
      2.6.23 (when lazy-count support was introduced), and so if we change
      the alignment emitted by mkfs without a feature bit then we have to
      make sure it works properly on all kernels since 2.6.23. And if we
      allow it to be changed when the lazy-count bit is not set, then it's
      all kernels since v2 logs were introduced that need to be tested for
      compatibility...
      Signed-off-by: default avatarDave Chinner <dchinner@redhat.com>
      Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
      Reviewed-by: default avatarEric Sandeen <sandeen@redhat.com>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      8f80587b
    • Mark Tinguely's avatar
      xfs: fix unlock in xfs_bmap_add_attrfork · 9e3908e3
      Mark Tinguely authored
      xfs_trans_ijoin() activates the inode in a transaction and
      also can specify which lock to free when the transaction is
      committed or canceled.
      
      xfs_bmap_add_attrfork call locks and adds the lock to the
      transaction but also manually removes the lock. Change the
      routine to not add the lock to the transaction and manually
      remove lock on completion.
      
      While here, clean up the xfs_trans_cancel flags and goto names.
      Signed-off-by: default avatarMark Tinguely <tinguely@sgi.com>
      Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      9e3908e3
  2. 15 Nov, 2013 1 commit
  3. 06 Nov, 2013 3 commits
  4. 04 Nov, 2013 1 commit
    • Dave Chinner's avatar
      xfs: xfs_remove deadlocks due to inverted AGF vs AGI lock ordering · 27320369
      Dave Chinner authored
      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>
      27320369
  5. 31 Oct, 2013 1 commit
  6. 30 Oct, 2013 16 commits
    • Eric Sandeen's avatar
      xfs: be more forgiving of a v4 secondary sb w/ junk in v5 fields · 10e6e65d
      Eric Sandeen authored
      Today, if xfs_sb_read_verify encounters a v4 superblock
      with junk past v4 fields which includes data in sb_crc,
      it will be treated as a failing checksum and a significant
      corruption.
      
      There are known prior bugs which leave junk at the end
      of the V4 superblock; we don't need to actually fail the
      verification in this case if other checks pan out ok.
      
      So if this is a secondary superblock, and the primary
      superblock doesn't indicate that this is a V5 filesystem,
      don't treat this as an actual checksum failure.
      
      We should probably check the garbage condition as
      we do in xfs_repair, and possibly warn about it
      or self-heal, but that's a different scope of work.
      
      Stable folks: This can go back to v3.10, which is what
      introduced the sb CRC checking that is tripped up by old,
      stale, incorrect V4 superblocks w/ unzeroed bits.
      
      Cc: stable@vger.kernel.org
      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>
      10e6e65d
    • Geyslan G. Bem's avatar
      xfs: fix possible NULL dereference in xlog_verify_iclog · 643f7c4e
      Geyslan G. Bem authored
      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>
      643f7c4e
    • Denis Efremov's avatar
      xfs:xfs_dir2_node.c: pointer use before check for null · 5bf1f439
      Denis Efremov authored
      ASSERT on args takes place after args dereference.
      This assertion is redundant since we are going to panic anyway.
      
      Found by Linux Driver Verification project (linuxtesting.org) -
      PVS-Studio analyzer.
      Signed-off-by: default avatarDenis Efremov <yefremov.denis@gmail.com>
      Reviewed-by: default avatarBen Myers <bpm@sgi.com>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      5bf1f439
    • Dave Chinner's avatar
      xfs: prevent stack overflows from page cache allocation · ad22c7a0
      Dave Chinner authored
      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>
      ad22c7a0
    • Dave Chinner's avatar
      xfs: fix static and extern sparse warnings · 632b89e8
      Dave Chinner authored
      The kbuild test robot indicated that there were some new sparse
      warnings in fs/xfs/xfs_dquot_buf.c. Actually, there were a lot more
      that is wasn't warning about, so fix them all up.
      
      Reported-by: kbuild test robot
      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>
      632b89e8
    • Dave Chinner's avatar
      xfs: validity check the directory block leaf entry count · a6293621
      Dave Chinner authored
      The directory block format verifier fails to check that the leaf
      entry count is in a valid range, and so if it is corrupted then it
      can lead to derefencing a pointer outside the block buffer. While we
      can't exactly validate the count without first walking the directory
      block, we can ensure the count lands in the valid area within the
      directory block and hence avoid out-of-block references.
      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>
      a6293621
    • Dave Chinner's avatar
      xfs: make dir2 ftype offset pointers explicit · b01ef655
      Dave Chinner authored
      Rather than hiding the ftype field size accounting inside the dirent
      padding for the ".." and first entry offset functions for v2
      directory formats, add explicit functions that calculate it
      correctly.
      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>
      b01ef655
    • Dave Chinner's avatar
      xfs: convert directory vector functions to constants · 1c9a5b2e
      Dave Chinner authored
      Many of the vectorised function calls now take no parameters and
      return a constant value. There is no reason for these to be vectored
      functions, so convert them to constants
      
      Binary sizes:
      
         text    data     bss     dec     hex filename
       794490   96802    1096  892388   d9de4 fs/xfs/xfs.o.orig
       792986   96802    1096  890884   d9804 fs/xfs/xfs.o.p1
       792350   96802    1096  890248   d9588 fs/xfs/xfs.o.p2
       789293   96802    1096  887191   d8997 fs/xfs/xfs.o.p3
       789005   96802    1096  886903   d8997 fs/xfs/xfs.o.p4
       789061   96802    1096  886959   d88af fs/xfs/xfs.o.p5
       789733   96802    1096  887631   d8b4f fs/xfs/xfs.o.p6
       791421   96802    1096  889319   d91e7 fs/xfs/xfs.o.p7
       791701   96802    1096  889599   d92ff fs/xfs/xfs.o.p8
       791205   96802    1096  889103   d91cf fs/xfs/xfs.o.p9
      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>
      1c9a5b2e
    • Dave Chinner's avatar
      xfs: convert directory vector functions to constants · 24dd0f54
      Dave Chinner authored
      Next step in the vectorisation process is the directory free block
      encode/decode operations. There are relatively few of these, though
      there are quite a number of calls to them.
      
      Binary sizes:
      
         text    data     bss     dec     hex filename
       794490   96802    1096  892388   d9de4 fs/xfs/xfs.o.orig
       792986   96802    1096  890884   d9804 fs/xfs/xfs.o.p1
       792350   96802    1096  890248   d9588 fs/xfs/xfs.o.p2
       789293   96802    1096  887191   d8997 fs/xfs/xfs.o.p3
       789005   96802    1096  886903   d8997 fs/xfs/xfs.o.p4
       789061   96802    1096  886959   d88af fs/xfs/xfs.o.p5
       789733   96802    1096  887631   d8b4f fs/xfs/xfs.o.p6
       791421   96802    1096  889319   d91e7 fs/xfs/xfs.o.p7
       791701   96802    1096  889599   d92ff fs/xfs/xfs.o.p8
      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>
      24dd0f54
    • Dave Chinner's avatar
      xfs: vectorise encoding/decoding directory headers · 01ba43b8
      Dave Chinner authored
      Conversion from on-disk structures to in-core header structures
      currently relies on magic number checks. If the magic number is
      wrong, but one of the supported values, we do the wrong thing with
      the encode/decode operation. Split these functions so that there are
      discrete operations for the specific directory format we are
      handling.
      
      In doing this, move all the header encode/decode functions to
      xfs_da_format.c as they are directly manipulating the on-disk
      format. It should be noted that all the growth in binary size is
      from xfs_da_format.c - the rest of the code actaully shrinks.
      
         text    data     bss     dec     hex filename
       794490   96802    1096  892388   d9de4 fs/xfs/xfs.o.orig
       792986   96802    1096  890884   d9804 fs/xfs/xfs.o.p1
       792350   96802    1096  890248   d9588 fs/xfs/xfs.o.p2
       789293   96802    1096  887191   d8997 fs/xfs/xfs.o.p3
       789005   96802    1096  886903   d8997 fs/xfs/xfs.o.p4
       789061   96802    1096  886959   d88af fs/xfs/xfs.o.p5
       789733   96802    1096  887631   d8b4f fs/xfs/xfs.o.p6
       791421   96802    1096  889319   d91e7 fs/xfs/xfs.o.p7
      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>
      01ba43b8
    • Dave Chinner's avatar
      xfs: vectorise DA btree operations · 4bceb18f
      Dave Chinner authored
      The remaining non-vectorised code for the directory structure is the
      node format blocks. This is shared with the attribute tree, and so
      is slightly more complex to vectorise.
      
      Introduce a "non-directory" directory ops structure that is attached
      to all non-directory inodes so that attribute operations can be
      vectorised for all inodes.
      
      Once we do this, we can vectorise all the da btree operations.
      Because this patch adds more infrastructure than it removes the
      binary size does not decrease:
      
         text    data     bss     dec     hex filename
       794490   96802    1096  892388   d9de4 fs/xfs/xfs.o.orig
       792986   96802    1096  890884   d9804 fs/xfs/xfs.o.p1
       792350   96802    1096  890248   d9588 fs/xfs/xfs.o.p2
       789293   96802    1096  887191   d8997 fs/xfs/xfs.o.p3
       789005   96802    1096  886903   d8997 fs/xfs/xfs.o.p4
       789061   96802    1096  886959   d88af fs/xfs/xfs.o.p5
       789733   96802    1096  887631   d8b4f fs/xfs/xfs.o.p6
      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>
      4bceb18f
    • Dave Chinner's avatar
      xfs: vectorise directory leaf operations · 4141956a
      Dave Chinner authored
      Next step in the vectorisation process is the leaf block
      encode/decode operations. Most of the operations on leaves are
      handled by the data block vectors, so there are relatively few of
      them here.
      
      Because of all the shuffling of code and having to pass more state
      to some functions, this patch doesn't directly reduce the size of
      the binary. It does open up many more opportunities for factoring
      and optimisation, however.
      
         text    data     bss     dec     hex filename
       794490   96802    1096  892388   d9de4 fs/xfs/xfs.o.orig
       792986   96802    1096  890884   d9804 fs/xfs/xfs.o.p1
       792350   96802    1096  890248   d9588 fs/xfs/xfs.o.p2
       789293   96802    1096  887191   d8997 fs/xfs/xfs.o.p3
       789005   96802    1096  886903   d8997 fs/xfs/xfs.o.p4
       789061   96802    1096  886959   d88af fs/xfs/xfs.o.p5
      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>
      4141956a
    • Dave Chinner's avatar
      xfs: vectorise directory data operations part 2 · 2ca98774
      Dave Chinner authored
      Convert the rest of the directory data block encode/decode
      operations to vector format.
      
      This further reduces the size of the built binary:
      
         text    data     bss     dec     hex filename
       794490   96802    1096  892388   d9de4 fs/xfs/xfs.o.orig
       792986   96802    1096  890884   d9804 fs/xfs/xfs.o.p1
       792350   96802    1096  890248   d9588 fs/xfs/xfs.o.p2
       789293   96802    1096  887191   d8997 fs/xfs/xfs.o.p3
       789005   96802    1096  886903   d8997 fs/xfs/xfs.o.p4
      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>
      2ca98774
    • Dave Chinner's avatar
      xfs: vectorise directory data operations · 9d23fc85
      Dave Chinner authored
      Following from the initial patches to vectorise the shortform
      directory encode/decode operations, convert half the data block
      operations to use the vector. The rest will be done in a second
      patch.
      
      This further reduces the size of the built binary:
      
         text    data     bss     dec     hex filename
       794490   96802    1096  892388   d9de4 fs/xfs/xfs.o.orig
       792986   96802    1096  890884   d9804 fs/xfs/xfs.o.p1
       792350   96802    1096  890248   d9588 fs/xfs/xfs.o.p2
       789293   96802    1096  887191   d8997 fs/xfs/xfs.o.p3
      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>
      9d23fc85
    • Dave Chinner's avatar
      xfs: vectorise remaining shortform dir2 ops · 4740175e
      Dave Chinner authored
      Following from the initial patch to introduce the directory
      operations vector, convert the rest of the shortform directory
      operations to use vectored ops rather than superblock feature
      checks. This further reduces the size of the built binary:
      
         text    data     bss     dec     hex filename
       794490   96802    1096  892388   d9de4 fs/xfs/xfs.o.orig
       792986   96802    1096  890884   d9804 fs/xfs/xfs.o.p1
       792350   96802    1096  890248   d9588 fs/xfs/xfs.o.p2
      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>
      4740175e
    • Dave Chinner's avatar
      xfs: abstract the differences in dir2/dir3 via an ops vector · 32c5483a
      Dave Chinner authored
      Lots of the dir code now goes through switches to determine what is
      the correct on-disk format to parse. It generally involves a
      "xfs_sbversion_hasfoo" check, deferencing the superblock version and
      feature fields and hence touching several cache lines per operation
      in the process. Some operations do multiple checks because they nest
      conditional operations and they don't pass the information in a
      direct fashion between each other.
      
      Hence, add an ops vector to the xfs_inode structure that is
      configured when the inode is initialised to point to all the correct
      decode and encoding operations.  This will significantly reduce the
      branchiness and cacheline footprint of the directory object decoding
      and encoding.
      
      This is the first patch in a series of conversion patches. It will
      introduce the ops structure, the setup of it and add the first
      operation to the vector. Subsequent patches will convert directory
      ops one at a time to keep the changes simple and obvious.
      
      Just this patch shows the benefit of such an approach on code size.
      Just converting the two shortform dir operations as this patch does
      decreases the built binary size by ~1500 bytes:
      
      $ size fs/xfs/xfs.o.orig fs/xfs/xfs.o.p1
         text    data     bss     dec     hex filename
       794490   96802    1096  892388   d9de4 fs/xfs/xfs.o.orig
       792986   96802    1096  890884   d9804 fs/xfs/xfs.o.p1
      $
      
      That's a significant decrease in the instruction cache footprint of
      the directory code for such a simple change, and indicates that this
      approach is definitely worth pursuing further.
      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>
      32c5483a
  7. 23 Oct, 2013 7 commits
  8. 21 Oct, 2013 5 commits
  9. 17 Oct, 2013 4 commits
    • Eric Sandeen's avatar
      xfs: don't break from growfs ag update loop on error · 59e5a0e8
      Eric Sandeen authored
      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: Mark Tinguely <tinguely@sgi.com> 
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      59e5a0e8
    • Eric Sandeen's avatar
      xfs: don't emit corruption noise on fs probes · 31625f28
      Eric Sandeen authored
      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>
      31625f28
    • Eric Sandeen's avatar
      xfs: remove newlines from strings passed to __xfs_printk · 08e96e1a
      Eric Sandeen authored
      __xfs_printk adds its own "\n".  Having it in the original string
      leads to unintentional blank lines from these messages.
      
      Most format strings have no newline, but a few do, leading to
      i.e.:
      
      [ 7347.119911] XFS (sdb2): Access to block zero in inode 132 start_block: 0 start_off: 0 blkcnt: 0 extent-state: 0 lastx: 1a05
      [ 7347.119911] 
      [ 7347.119919] XFS (sdb2): Access to block zero in inode 132 start_block: 0 start_off: 0 blkcnt: 0 extent-state: 0 lastx: 1a05
      [ 7347.119919] 
      
      Fix them all.
      Signed-off-by: default avatarEric Sandeen <sandeen@redhat.com>
      Reviewed-by: default avatarMark Tinguely <tinguely@sgi.com>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      08e96e1a
    • Dave Chinner's avatar
      xfs: prevent deadlock trying to cover an active log · 2c6e24ce
      Dave Chinner authored
      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>
      2c6e24ce