1. 29 Nov, 2012 7 commits
  2. 19 Nov, 2012 5 commits
  3. 18 Nov, 2012 1 commit
  4. 17 Nov, 2012 3 commits
    • Dave Chinner's avatar
      xfs: drop buffer io reference when a bad bio is built · d69043c4
      Dave Chinner authored
      
      
      Error handling in xfs_buf_ioapply_map() does not handle IO reference
      counts correctly. We increment the b_io_remaining count before
      building the bio, but then fail to decrement it in the failure case.
      This leads to the buffer never running IO completion and releasing
      the reference that the IO holds, so at unmount we can leak the
      buffer. This leak is captured by this assert failure during unmount:
      
      XFS: Assertion failed: atomic_read(&pag->pag_ref) == 0, file: fs/xfs/xfs_mount.c, line: 273
      
      This is not a new bug - the b_io_remaining accounting has had this
      problem for a long, long time - it's just very hard to get a
      zero length bio being built by this code...
      
      Further, the buffer IO error can be overwritten on a multi-segment
      buffer by subsequent bio completions for partial sections of the
      buffer. Hence we should only set the buffer error status if the
      buffer is not already carrying an error status. This ensures that a
      partial IO error on a multi-segment buffer will not be lost. This
      part of the problem is a regression, however.
      
      cc: <stable@vger.kernel.org>
      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>
      d69043c4
    • Dave Chinner's avatar
      xfs: fix broken error handling in xfs_vm_writepage · 3daed8bc
      Dave Chinner authored
      
      
      When we shut down the filesystem, it might first be detected in
      writeback when we are allocating a inode size transaction. This
      happens after we have moved all the pages into the writeback state
      and unlocked them. Unfortunately, if we fail to set up the
      transaction we then abort writeback and try to invalidate the
      current page. This then triggers are BUG() in block_invalidatepage()
      because we are trying to invalidate an unlocked page.
      
      Fixing this is a bit of a chicken and egg problem - we can't
      allocate the transaction until we've clustered all the pages into
      the IO and we know the size of it (i.e. whether the last block of
      the IO is beyond the current EOF or not). However, we don't want to
      hold pages locked for long periods of time, especially while we lock
      other pages to cluster them into the write.
      
      To fix this, we need to make a clear delineation in writeback where
      errors can only be handled by IO completion processing. That is,
      once we have marked a page for writeback and unlocked it, we have to
      report errors via IO completion because we've already started the
      IO. We may not have submitted any IO, but we've changed the page
      state to indicate that it is under IO so we must now use the IO
      completion path to report errors.
      
      To do this, add an error field to xfs_submit_ioend() to pass it the
      error that occurred during the building on the ioend chain. When
      this is non-zero, mark each ioend with the error and call
      xfs_finish_ioend() directly rather than building bios. This will
      immediately push the ioends through completion processing with the
      error that has occurred.
      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>
      3daed8bc
    • Dave Chinner's avatar
      xfs: fix attr tree double split corruption · 42e2976f
      Dave Chinner authored
      
      
      In certain circumstances, a double split of an attribute tree is
      needed to insert or replace an attribute. In rare situations, this
      can go wrong, leaving the attribute tree corrupted. In this case,
      the attr being replaced is the last attr in a leaf node, and the
      replacement is larger so doesn't fit in the same leaf node.
      When we have the initial condition of a node format attribute
      btree with two leaves at index 1 and 2. Call them L1 and L2.  The
      leaf L1 is completely full, there is not a single byte of free space
      in it. L2 is mostly empty.  The attribute being replaced - call it X
      - is the last attribute in L1.
      
      The way an attribute replace is executed is that the replacement
      attribute - call it Y - is first inserted into the tree, but has an
      INCOMPLETE flag set on it so that list traversals ignore it. Once
      this transaction is committed, a second transaction it run to
      atomically mark Y as COMPLETE and X as INCOMPLETE, so that a
      traversal will now find Y and skip X. Once that transaction is
      committed, attribute X is then removed.
      
      So, the initial condition is:
      
           +--------+     +--------+
           |   L1   |     |   L2   |
           | fwd: 2 |---->| fwd: 0 |
           | bwd: 0 |<----| bwd: 1 |
           | fsp: 0 |     | fsp: N |
           |--------|     |--------|
           | attr A |     | attr 1 |
           |--------|     |--------|
           | attr B |     | attr 2 |
           |--------|     |--------|
           ..........     ..........
           |--------|     |--------|
           | attr X |     | attr n |
           +--------+     +--------+
      
      So now we go to replace X, and see that L1:fsp = 0 - it is full so
      we can't insert Y in the same leaf. So we record the the location of
      attribute X so we can track it for later use, then we split L1 into
      L1 and L3 and reblance across the two leafs. We end with:
      
           +--------+     +--------+     +--------+
           |   L1   |     |   L3   |     |   L2   |
           | fwd: 3 |---->| fwd: 2 |---->| fwd: 0 |
           | bwd: 0 |<----| bwd: 1 |<----| bwd: 3 |
           | fsp: M |     | fsp: J |     | fsp: N |
           |--------|     |--------|     |--------|
           | attr A |     | attr X |     | attr 1 |
           |--------|     +--------+     |--------|
           | attr B |                    | attr 2 |
           |--------|                    |--------|
           ..........                    ..........
           |--------|                    |--------|
           | attr W |                    | attr n |
           +--------+                    +--------+
      
      And we track that the original attribute is now at L3:0.
      
      We then try to insert Y into L1 again, and find that there isn't
      enough room because the new attribute is larger than the old one.
      Hence we have to split again to make room for Y. We end up with
      this:
      
           +--------+     +--------+     +--------+     +--------+
           |   L1   |     |   L4   |     |   L3   |     |   L2   |
           | fwd: 4 |---->| fwd: 3 |---->| fwd: 2 |---->| fwd: 0 |
           | bwd: 0 |<----| bwd: 1 |<----| bwd: 4 |<----| bwd: 3 |
           | fsp: M |     | fsp: J |     | fsp: J |     | fsp: N |
           |--------|     |--------|     |--------|     |--------|
           | attr A |     | attr Y |     | attr X |     | attr 1 |
           |--------|     + INCOMP +     +--------+     |--------|
           | attr B |     +--------+                    | attr 2 |
           |--------|                                   |--------|
           ..........                                   ..........
           |--------|                                   |--------|
           | attr W |                                   | attr n |
           +--------+                                   +--------+
      
      And now we have the new (incomplete) attribute @ L4:0, and the
      original attribute at L3:0. At this point, the first transaction is
      committed, and we move to the flipping of the flags.
      
      This is where we are supposed to end up with this:
      
           +--------+     +--------+     +--------+     +--------+
           |   L1   |     |   L4   |     |   L3   |     |   L2   |
           | fwd: 4 |---->| fwd: 3 |---->| fwd: 2 |---->| fwd: 0 |
           | bwd: 0 |<----| bwd: 1 |<----| bwd: 4 |<----| bwd: 3 |
           | fsp: M |     | fsp: J |     | fsp: J |     | fsp: N |
           |--------|     |--------|     |--------|     |--------|
           | attr A |     | attr Y |     | attr X |     | attr 1 |
           |--------|     +--------+     + INCOMP +     |--------|
           | attr B |                    +--------+     | attr 2 |
           |--------|                                   |--------|
           ..........                                   ..........
           |--------|                                   |--------|
           | attr W |                                   | attr n |
           +--------+                                   +--------+
      
      But that doesn't happen properly - the attribute tracking indexes
      are not pointing to the right locations. What we end up with is both
      the old attribute to be removed pointing at L4:0 and the new
      attribute at L4:1.  On a debug kernel, this assert fails like so:
      
      XFS: Assertion failed: args->index2 < be16_to_cpu(leaf2->hdr.count), file: fs/xfs/xfs_attr_leaf.c, line: 2725
      
      because the new attribute location does not exist. On a production
      kernel, this goes unnoticed and the code proceeds ahead merrily and
      removes L4 because it thinks that is the block that is no longer
      needed. This leaves the hash index node pointing to entries
      L1, L4 and L2, but only blocks L1, L3 and L2 to exist. Further, the
      leaf level sibling list is L1 <-> L4 <-> L2, but L4 is now free
      space, and so everything is busted. This corruption is caused by the
      removal of the old attribute triggering a join - it joins everything
      correctly but then frees the wrong block.
      
      xfs_repair will report something like:
      
      bad sibling back pointer for block 4 in attribute fork for inode 131
      problem with attribute contents in inode 131
      would clear attr fork
      bad nblocks 8 for inode 131, would reset to 3
      bad anextents 4 for inode 131, would reset to 0
      
      The problem lies in the assignment of the old/new blocks for
      tracking purposes when the double leaf split occurs. The first split
      tries to place the new attribute inside the current leaf (i.e.
      "inleaf == true") and moves the old attribute (X) to the new block.
      This sets up the old block/index to L1:X, and newly allocated
      block to L3:0. It then moves attr X to the new block and tries to
      insert attr Y at the old index. That fails, so it splits again.
      
      With the second split, the rebalance ends up placing the new attr in
      the second new block - L4:0 - and this is where the code goes wrong.
      What is does is it sets both the new and old block index to the
      second new block. Hence it inserts attr Y at the right place (L4:0)
      but overwrites the current location of the attr to replace that is
      held in the new block index (currently L3:0). It over writes it with
      L4:1 - the index we later assert fail on.
      
      Hopefully this table will show this in a foramt that is a bit easier
      to understand:
      
      Split		old attr index		new attr index
      		vanilla	patched		vanilla	patched
      before 1st	L1:26	L1:26		N/A	N/A
      after 1st	L3:0	L3:0		L1:26	L1:26
      after 2nd	L4:0	L3:0		L4:1	L4:0
                      ^^^^			^^^^
      		wrong			wrong
      
      The fix is surprisingly simple, for all this analysis - just stop
      the rebalance on the out-of leaf case from overwriting the new attr
      index - it's already correct for the double split case.
      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>
      42e2976f
  5. 16 Nov, 2012 1 commit
  6. 15 Nov, 2012 1 commit
  7. 12 Nov, 2012 1 commit
  8. 09 Nov, 2012 3 commits
    • Thomas Betker's avatar
      jffs2: Fix lock acquisition order bug in jffs2_write_begin · 5ffd3412
      Thomas Betker authored
      
      
      jffs2_write_begin() first acquires the page lock, then f->sem. This
      causes an AB-BA deadlock with jffs2_garbage_collect_live(), which first
      acquires f->sem, then the page lock:
      
      jffs2_garbage_collect_live
          mutex_lock(&f->sem)                         (A)
          jffs2_garbage_collect_dnode
              jffs2_gc_fetch_page
                  read_cache_page_async
                      do_read_cache_page
                          lock_page(page)             (B)
      
      jffs2_write_begin
          grab_cache_page_write_begin
              find_lock_page
                  lock_page(page)                     (B)
          mutex_lock(&f->sem)                         (A)
      
      We fix this by restructuring jffs2_write_begin() to take f->sem before
      the page lock. However, we make sure that f->sem is not held when
      calling jffs2_reserve_space(), as this is not permitted by the locking
      rules.
      
      The deadlock above was observed multiple times on an SoC with a dual
      ARMv7 (Cortex-A9), running the long-term 3.4.11 kernel; it occurred
      when using scp to copy files from a host system to the ARM target
      system. The fix was heavily tested on the same target system.
      
      Cc: stable@vger.kernel.org
      Signed-off-by: default avatarThomas Betker <thomas.betker@rohde-schwarz.com>
      Acked-by: default avatarJoakim Tjernlund <Joakim.Tjernlund@transmode.se>
      Signed-off-by: default avatarArtem Bityutskiy <artem.bityutskiy@linux.intel.com>
      5ffd3412
    • Eric Paris's avatar
      fanotify: fix missing break · 848561d3
      Eric Paris authored
      Anders Blomdell noted in 2010 that Fanotify lost events and provided a
      test case.  Eric Paris confirmed it was a bug and posted a fix to the
      list
      
        https://groups.google.com/forum/?fromgroups=#!topic/linux.kernel/RrJfTfyW2BE
      
      
      
      but never applied it.  Repeated attempts over time to actually get him
      to apply it have never had a reply from anyone who has raised it
      
      So apply it anyway
      Signed-off-by: default avatarAlan Cox <alan@linux.intel.com>
      Reported-by: default avatarAnders Blomdell <anders.blomdell@control.lth.se>
      Cc: Eric Paris <eparis@redhat.com>
      Cc: <stable@vger.kernel.org>
      Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
      Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      848561d3
    • Andrew Morton's avatar
      revert "epoll: support for disabling items, and a self-test app" · a80a6b85
      Andrew Morton authored
      Revert commit 03a7beb5
      
       ("epoll: support for disabling items, and a
      self-test app") pending resolution of the issues identified by Michael
      Kerrisk, copied below.
      
      We'll revisit this for 3.8.
      
      : I've taken a look at this patch as it currently stands in 3.7-rc1, and
      : done a bit of testing. (By the way, the test program
      : tools/testing/selftests/epoll/test_epoll.c does not compile...)
      :
      : There are one or two places where the behavior seems a little strange,
      : so I have a question or two at the end of this mail. But other than
      : that, I want to check my understanding so that the interface can be
      : correctly documented.
      :
      : Just to go though my understanding, the problem is the following
      : scenario in a multithreaded application:
      :
      : 1. Multiple threads are performing epoll_wait() operations,
      :    and maintaining a user-space cache that contains information
      :    corresponding to each file descriptor being monitored by
      :    epoll_wait().
      :
      : 2. At some point, a thread wants to delete (EPOLL_CTL_DEL)
      :    a file descriptor from the epoll interest list, and
      :    delete the corresponding record from the user-space cache.
      :
      : 3. The problem with (2) is that some other thread may have
      :    previously done an epoll_wait() that retrieved information
      :    about the fd in question, and may be in the middle of using
      :    information in the cache that relates to that fd. Thus,
      :    there is a potential race.
      :
      : 4. The race can't solved purely in user space, because doing
      :    so would require applying a mutex across the epoll_wait()
      :    call, which would of course blow thread concurrency.
      :
      : Right?
      :
      : Your solution is the EPOLL_CTL_DISABLE operation. I want to
      : confirm my understanding about how to use this flag, since
      : the description that has accompanied the patches so far
      : has been a bit sparse
      :
      : 0. In the scenario you're concerned about, deleting a file
      :    descriptor means (safely) doing the following:
      :    (a) Deleting the file descriptor from the epoll interest list
      :        using EPOLL_CTL_DEL
      :    (b) Deleting the corresponding record in the user-space cache
      :
      : 1. It's only meaningful to use this EPOLL_CTL_DISABLE in
      :    conjunction with EPOLLONESHOT.
      :
      : 2. Using EPOLL_CTL_DISABLE without using EPOLLONESHOT in
      :    conjunction is a logical error.
      :
      : 3. The correct way to code multithreaded applications using
      :    EPOLL_CTL_DISABLE and EPOLLONESHOT is as follows:
      :
      :    a. All EPOLL_CTL_ADD and EPOLL_CTL_MOD operations should
      :       should EPOLLONESHOT.
      :
      :    b. When a thread wants to delete a file descriptor, it
      :       should do the following:
      :
      :       [1] Call epoll_ctl(EPOLL_CTL_DISABLE)
      :       [2] If the return status from epoll_ctl(EPOLL_CTL_DISABLE)
      :           was zero, then the file descriptor can be safely
      :           deleted by the thread that made this call.
      :       [3] If the epoll_ctl(EPOLL_CTL_DISABLE) fails with EBUSY,
      :           then the descriptor is in use. In this case, the calling
      :           thread should set a flag in the user-space cache to
      :           indicate that the thread that is using the descriptor
      :           should perform the deletion operation.
      :
      : Is all of the above correct?
      :
      : The implementation depends on checking on whether
      : (events & ~EP_PRIVATE_BITS) == 0
      : This replies on the fact that EPOLL_CTL_AD and EPOLL_CTL_MOD always
      : set EPOLLHUP and EPOLLERR in the 'events' mask, and EPOLLONESHOT
      : causes those flags (as well as all others in ~EP_PRIVATE_BITS) to be
      : cleared.
      :
      : A corollary to the previous paragraph is that using EPOLL_CTL_DISABLE
      : is only useful in conjunction with EPOLLONESHOT. However, as things
      : stand, one can use EPOLL_CTL_DISABLE on a file descriptor that does
      : not have EPOLLONESHOT set in 'events' This results in the following
      : (slightly surprising) behavior:
      :
      : (a) The first call to epoll_ctl(EPOLL_CTL_DISABLE) returns 0
      :     (the indicator that the file descriptor can be safely deleted).
      : (b) The next call to epoll_ctl(EPOLL_CTL_DISABLE) fails with EBUSY.
      :
      : This doesn't seem particularly useful, and in fact is probably an
      : indication that the user made a logic error: they should only be using
      : epoll_ctl(EPOLL_CTL_DISABLE) on a file descriptor for which
      : EPOLLONESHOT was set in 'events'. If that is correct, then would it
      : not make sense to return an error to user space for this case?
      
      Cc: Michael Kerrisk <mtk.manpages@gmail.com>
      Cc: "Paton J. Lewis" <palewis@adobe.com>
      Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
      Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
      a80a6b85
  9. 08 Nov, 2012 10 commits
    • Dave Chinner's avatar
      xfs: fix reading of wrapped log data · 6ce377af
      Dave Chinner authored
      Commit 44396476
      
       ("xfs: reset buffer pointers before freeing them") in
      3.0-rc1 introduced a regression when recovering log buffers that
      wrapped around the end of log. The second part of the log buffer at
      the start of the physical log was being read into the header buffer
      rather than the data buffer, and hence recovery was seeing garbage
      in the data buffer when it got to the region of the log buffer that
      was incorrectly read.
      
      Cc: <stable@vger.kernel.org> # 3.0.x, 3.2.x, 3.4.x 3.6.x
      Reported-by: default avatarTorsten Kaiser <just.for.lkml@googlemail.com>
      Signed-off-by: default avatarDave Chinner <dchinner@redhat.com>
      Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
      Reviewed-by: default avatarMark Tinguely <tinguely@sgi.com>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      6ce377af
    • Dave Chinner's avatar
      xfs: fix buffer shudown reference count mismatch · 03b1293e
      Dave Chinner authored
      
      
      When we shut down the filesystem, we have to unpin and free all the
      buffers currently active in the CIL. To do this we unpin and remove
      them in one operation as a result of a failed iclogbuf write. For
      buffers, we do this removal via a simultated IO completion of after
      marking the buffer stale.
      
      At the time we do this, we have two references to the buffer - the
      active LRU reference and the buf log item.  The LRU reference is
      removed by marking the buffer stale, and the active CIL reference is
      by the xfs_buf_iodone() callback that is run by
      xfs_buf_do_callbacks() during ioend processing (via the bp->b_iodone
      callback).
      
      However, ioend processing requires one more reference - that of the
      IO that it is completing. We don't have this reference, so we free
      the buffer prematurely and use it after it is freed. For buffers
      marked with XBF_ASYNC, this leads to assert failures in
      xfs_buf_rele() on debug kernels because the b_hold count is zero.
      
      Fix this by making sure we take the necessary IO reference before
      starting IO completion processing on the stale buffer, and set the
      XBF_ASYNC flag to ensure that IO completion processing removes all
      the active references from the buffer to ensure it is fully torn
      down.
      
      Cc: <stable@vger.kernel.org>
      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>
      03b1293e
    • Dave Chinner's avatar
      xfs: don't vmap inode cluster buffers during free · 4b62acfe
      Dave Chinner authored
      Inode buffers do not need to be mapped as inodes are read or written
      directly from/to the pages underlying the buffer. This fixes a
      regression introduced by commit 611c9946
      
       ("xfs: make XBF_MAPPED the
      default behaviour").
      Signed-off-by: default avatarDave Chinner <dchinner@redhat.com>
      Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
      Reviewed-by: default avatarMark Tinguely <tinguely@sgi.com>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      4b62acfe
    • Dave Chinner's avatar
      xfs: invalidate allocbt blocks moved to the free list · ca250b1b
      Dave Chinner authored
      
      
      When we free a block from the alloc btree tree, we move it to the
      freelist held in the AGFL and mark it busy in the busy extent tree.
      This typically happens when we merge btree blocks.
      
      Once the transaction is committed and checkpointed, the block can
      remain on the free list for an indefinite amount of time.  Now, this
      isn't the end of the world at this point - if the free list is
      shortened, the buffer is invalidated in the transaction that moves
      it back to free space. If the buffer is allocated as metadata from
      the free list, then all the modifications getted logged, and we have
      no issues, either. And if it gets allocated as userdata direct from
      the freelist, it gets invalidated and so will never get written.
      
      However, during the time it sits on the free list, pressure on the
      log can cause the AIL to be pushed and the buffer that covers the
      block gets pushed for write. IOWs, we end up writing a freed
      metadata block to disk. Again, this isn't the end of the world
      because we know from the above we are only writing to free space.
      
      The problem, however, is for validation callbacks. If the block was
      on old btree root block, then the level of the block is going to be
      higher than the current tree root, and so will fail validation.
      There may be other inconsistencies in the block as well, and
      currently we don't care because the block is in free space. Shutting
      down the filesystem because a freed block doesn't pass write
      validation, OTOH, is rather unfriendly.
      
      So, make sure we always invalidate buffers as they move from the
      free space trees to the free list so that we guarantee they never
      get written to disk while on the free list.
      Signed-off-by: default avatarDave Chinner <dchinner@redhat.com>
      Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
      Reviewed-by: default avatarPhil White <pwhite@sgi.com>
      Reviewed-by: default avatarMark Tinguely <tinguely@sgi.com>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      ca250b1b
    • Dave Chinner's avatar
      xfs: silence uninitialised f.file warning. · 1e7acbb7
      Dave Chinner authored
      Uninitialised variable build warning introduced by 2903ff01
      
       ("switch
      simple cases of fget_light to fdget"), gcc is not smart enough to
      work out that the variable is not used uninitialised, and the commit
      removed the initialisation at declaration that the old variable had.
      Signed-off-by: default avatarDave Chinner <dchinner@redhat.com>
      Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
      Reviewed-by: default avatarMark Tinguely <tinguely@sgi.com>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      1e7acbb7
    • Dave Chinner's avatar
      xfs: growfs: don't read garbage for new secondary superblocks · eaef8543
      Dave Chinner authored
      
      
      When updating new secondary superblocks in a growfs operation, the
      superblock buffer is read from the newly grown region of the
      underlying device. This is not guaranteed to be zero, so violates
      the underlying assumption that the unused parts of superblocks are
      zero filled. Get a new buffer for these secondary superblocks to
      ensure that the unused regions are zero filled correctly.
      Signed-off-by: default avatarDave Chinner <dchinner@redhat.com>
      Reviewed-by: default avatarCarlos Maiolino <cmaiolino@redhat.com>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      eaef8543
    • Dave Chinner's avatar
      xfs: move allocation stack switch up to xfs_bmapi_allocate · 1f3c785c
      Dave Chinner authored
      
      
      Switching stacks are xfs_alloc_vextent can cause deadlocks when we
      run out of worker threads on the allocation workqueue. This can
      occur because xfs_bmap_btalloc can make multiple calls to
      xfs_alloc_vextent() and even if xfs_alloc_vextent() fails it can
      return with the AGF locked in the current allocation transaction.
      
      If we then need to make another allocation, and all the allocation
      worker contexts are exhausted because the are blocked waiting for
      the AGF lock, holder of the AGF cannot get it's xfs-alloc_vextent
      work completed to release the AGF.  Hence allocation effectively
      deadlocks.
      
      To avoid this, move the stack switch one layer up to
      xfs_bmapi_allocate() so that all of the allocation attempts in a
      single switched stack transaction occur in a single worker context.
      This avoids the problem of an allocation being blocked waiting for
      a worker thread whilst holding the AGF.
      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>
      1f3c785c
    • Dave Chinner's avatar
      xfs: introduce XFS_BMAPI_STACK_SWITCH · 326c0355
      Dave Chinner authored
      
      
      Certain allocation paths through xfs_bmapi_write() are in situations
      where we have limited stack available. These are almost always in
      the buffered IO writeback path when convertion delayed allocation
      extents to real extents.
      
      The current stack switch occurs for userdata allocations, which
      means we also do stack switches for preallocation, direct IO and
      unwritten extent conversion, even those these call chains have never
      been implicated in a stack overrun.
      
      Hence, let's target just the single stack overun offended for stack
      switches. To do that, introduce a XFS_BMAPI_STACK_SWITCH flag that
      the caller can pass xfs_bmapi_write() to indicate it should switch
      stacks if it needs to do allocation.
      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>
      326c0355
    • Mark Tinguely's avatar
      xfs: zero allocation_args on the kernel stack · 408cc4e9
      Mark Tinguely authored
      
      
      Zero the kernel stack space that makes up the xfs_alloc_arg structures.
      Signed-off-by: default avatarMark Tinguely <tinguely@sgi.com>
      Reviewed-by: default avatarBen Myers <bpm@sgi.com>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      408cc4e9
    • Dave Chinner's avatar
      xfs: only update the last_sync_lsn when a transaction completes · 7e9620f2
      Dave Chinner authored
      
      
      The log write code stamps each iclog with the current tail LSN in
      the iclog header so that recovery knows where to find the tail of
      thelog once it has found the head. Normally this is taken from the
      first item on the AIL - the log item that corresponds to the oldest
      active item in the log.
      
      The problem is that when the AIL is empty, the tail lsn is dervied
      from the the l_last_sync_lsn, which is the LSN of the last iclog to
      be written to the log. In most cases this doesn't happen, because
      the AIL is rarely empty on an active filesystem. However, when it
      does, it opens up an interesting case when the transaction being
      committed to the iclog spans multiple iclogs.
      
      That is, the first iclog is stamped with the l_last_sync_lsn, and IO
      is issued. Then the next iclog is setup, the changes copied into the
      iclog (takes some time), and then the l_last_sync_lsn is stamped
      into the header and IO is issued. This is still the same
      transaction, so the tail lsn of both iclogs must be the same for log
      recovery to find the entire transaction to be able to replay it.
      
      The problem arises in that the iclog buffer IO completion updates
      the l_last_sync_lsn with it's own LSN. Therefore, If the first iclog
      completes it's IO before the second iclog is filled and has the tail
      lsn stamped in it, it will stamp the LSN of the first iclog into
      it's tail lsn field. If the system fails at this point, log recovery
      will not see a complete transaction, so the transaction will no be
      replayed.
      
      The fix is simple - the l_last_sync_lsn is updated when a iclog
      buffer IO completes, and this is incorrect. The l_last_sync_lsn
      shoul dbe updated when a transaction is completed by a iclog buffer
      IO. That is, only iclog buffers that have transaction commit
      callbacks attached to them should update the l_last_sync_lsn. This
      means that the last_sync_lsn will only move forward when a commit
      record it written, not in the middle of a large transaction that is
      rolling through multiple iclog buffers.
      Signed-off-by: default avatarDave Chinner <dchinner@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>
      7e9620f2
  10. 07 Nov, 2012 7 commits
  11. 05 Nov, 2012 1 commit