1. 14 Jun, 2013 4 commits
    • Dave Chinner's avatar
      xfs: don't shutdown log recovery on validation errors · d302cf1d
      Dave Chinner authored
      Unfortunately, we cannot guarantee that items logged multiple times
      and replayed by log recovery do not take objects back in time. When
      they are taken back in time, the go into an intermediate state which
      is corrupt, and hence verification that occurs on this intermediate
      state causes log recovery to abort with a corruption shutdown.
      
      Instead of causing a shutdown and unmountable filesystem, don't
      verify post-recovery items before they are written to disk. This is
      less than optimal, but there is no way to detect this issue for
      non-CRC filesystems If log recovery successfully completes, this
      will be undone and the object will be consistent by subsequent
      transactions that are replayed, so in most cases we don't need to
      take drastic action.
      
      For CRC enabled filesystems, leave the verifiers in place - we need
      to call them to recalculate the CRCs on the objects anyway. This
      recovery problem can be solved for such filesystems - we have a LSN
      stamped in all metadata at writeback time that we can to determine
      whether the item should be replayed or not. This is a separate piece
      of work, so is not addressed by this patch.
      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>
      
      (cherry picked from commit 9222a9cf)
      d302cf1d
    • Dave Chinner's avatar
      xfs: ensure btree root split sets blkno correctly · 088c9f67
      Dave Chinner authored
      For CRC enabled filesystems, the BMBT is rooted in an inode, so it
      passes through a different code path on root splits than the
      freespace and inode btrees. This is much less traversed by xfstests
      than the other trees. When testing on a 1k block size filesystem,
      I've been seeing ASSERT failures in generic/234 like:
      
      XFS: Assertion failed: cur->bc_btnum != XFS_BTNUM_BMAP || cur->bc_private.b.allocated == 0, file: fs/xfs/xfs_btree.c, line: 317
      
      which are generally preceded by a lblock check failure. I noticed
      this in the bmbt stats:
      
      $ pminfo -f xfs.btree.block_map
      
      xfs.btree.block_map.lookup
          value 39135
      
      xfs.btree.block_map.compare
          value 268432
      
      xfs.btree.block_map.insrec
          value 15786
      
      xfs.btree.block_map.delrec
          value 13884
      
      xfs.btree.block_map.newroot
          value 2
      
      xfs.btree.block_map.killroot
          value 0
      .....
      
      Very little coverage of root splits and merges. Indeed, on a 4k
      filesystem, block_map.newroot and block_map.killroot are both zero.
      i.e. the code is not exercised at all, and it's the only generic
      btree infrastructure operation that is not exercised by a default run
      of xfstests.
      
      Turns out that on a 1k filesystem, generic/234 accounts for one of
      those two root splits, and that is somewhat of a smoking gun. In
      fact, it's the same problem we saw in the directory/attr code where
      headers are memcpy()d from one block to another without updating the
      self describing metadata.
      
      Simple fix - when copying the header out of the root block, make
      sure the block number is updated 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>
      
      (cherry picked from commit ade1335a)
      088c9f67
    • Dave Chinner's avatar
      xfs: fix implicit padding in directory and attr CRC formats · 5170711d
      Dave Chinner authored
      Michael L. Semon has been testing CRC patches on a 32 bit system and
      been seeing assert failures in the directory code from xfs/080.
      Thanks to Michael's heroic efforts with printk debugging, we found
      that the problem was that the last free space being left in the
      directory structure was too small to fit a unused tag structure and
      it was being corrupted and attempting to log a region out of bounds.
      Hence the assert failure looked something like:
      
      .....
      #5 calling xfs_dir2_data_log_unused() 36 32
      #1 4092 4095 4096
      #2 8182 8183 4096
      XFS: Assertion failed: first <= last && last < BBTOB(bp->b_length), file: fs/xfs/xfs_trans_buf.c, line: 568
      
      Where #1 showed the first region of the dup being logged (i.e. the
      last 4 bytes of a directory buffer) and #2 shows the corrupt values
      being calculated from the length of the dup entry which overflowed
      the size of the buffer.
      
      It turns out that the problem was not in the logging code, nor in
      the freespace handling code. It is an initial condition bug that
      only shows up on 32 bit systems. When a new buffer is initialised,
      where's the freespace that is set up:
      
      [  172.316249] calling xfs_dir2_leaf_addname() from xfs_dir_createname()
      [  172.316346] #9 calling xfs_dir2_data_log_unused()
      [  172.316351] #1 calling xfs_trans_log_buf() 60 63 4096
      [  172.316353] #2 calling xfs_trans_log_buf() 4094 4095 4096
      
      Note the offset of the first region being logged? It's 60 bytes into
      the buffer. Once I saw that, I pretty much knew that the bug was
      going to be caused by this.
      
      Essentially, all direct entries are rounded to 8 bytes in length,
      and all entries start with an 8 byte alignment. This means that we
      can decode inplace as variables are naturally aligned. With the
      directory data supposedly starting on a 8 byte boundary, and all
      entries padded to 8 bytes, the minimum freespace in a directory
      block is supposed to be 8 bytes, which is large enough to fit a
      unused data entry structure (6 bytes in size). The fact we only have
      4 bytes of free space indicates a directory data block alignment
      problem.
      
      And what do you know - there's an implicit hole in the directory
      data block header for the CRC format, which means the header is 60
      byte on 32 bit intel systems and 64 bytes on 64 bit systems. Needs
      padding. And while looking at the structures, I found the same
      problem in the attr leaf header. Fix them both.
      
      Note that this only affects 32 bit systems with CRCs enabled.
      Everything else is just fine. Note that CRC enabled filesystems created
      before this fix on such systems will not be readable with this fix
      applied.
      Reported-by: default avatarMichael L. Semon <mlsemon35@gmail.com>
      Debugged-by: default avatarMichael L. Semon <mlsemon35@gmail.com>
      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>
      
      (cherry picked from commit 8a1fd295)
      5170711d
    • Dave Chinner's avatar
      xfs: don't emit v5 superblock warnings on write · 47ad2fcb
      Dave Chinner authored
      We write the superblock every 30s or so which results in the
      verifier being called. Right now that results in this output
      every 30s:
      
      XFS (vda): Version 5 superblock detected. This kernel has EXPERIMENTAL support enabled!
      Use of these features in this kernel is at your own risk!
      
      And spamming the logs.
      
      We don't need to check for whether we support v5 superblocks or
      whether there are feature bits we don't support set as these are
      only relevant when we first mount the filesytem. i.e. on superblock
      read. Hence for the write verification we can just skip all the
      checks (and hence verbose output) altogether.
      Signed-off-by: default avatarDave Chinner <dchinner@redhat.com>
      Reviewed-by: default avatarBrian Foster <bfoster@redhat.com>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      
      (cherry picked from commit 34510185)
      47ad2fcb
  2. 06 Jun, 2013 6 commits
    • Dave Chinner's avatar
      xfs: increase number of ACL entries for V5 superblocks · 0a8aa193
      Dave Chinner authored
      The limit of 25 ACL entries is arbitrary, but baked into the on-disk
      format.  For version 5 superblocks, increase it to the maximum nuber
      of ACLs that can fit into a single xattr.
      Signed-off-by: default avatarDave Chinner <dchinner@redhat.com>
      Reviewed-by: default avatarBrian Foster <bfoster@redhat.com>
      Reviewed-by: default avatarMark Tinguely <tinuguely@sgi.com>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      
      (cherry picked from commit 5c87d4bc)
      0a8aa193
    • Dave Chinner's avatar
      xfs: disable noattr2/attr2 mount options for CRC enabled filesystems · f763fd44
      Dave Chinner authored
      attr2 format is always enabled for v5 superblock filesystems, so the
      mount options to enable or disable it need to be cause mount errors.
      Signed-off-by: default avatarDave Chinner <dchinner@redhat.com>
      Reviewed-by: default avatarBrian Foster <bfoster@redhat.com>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      
      (cherry picked from commit d3eaace8)
      f763fd44
    • Dave Chinner's avatar
      xfs: inode unlinked list needs to recalculate the inode CRC · ad868afd
      Dave Chinner authored
      The inode unlinked list manipulations operate directly on the inode
      buffer, and so bypass the inode CRC calculation mechanisms. Hence an
      inode on the unlinked list has an invalid CRC. Fix this by
      recalculating the CRC whenever we modify an unlinked list pointer in
      an inode, ncluding during log recovery. This is trivial to do and
      results in  unlinked list operations always leaving a consistent
      inode in the buffer.
      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>
      
      (cherry picked from commit 0a32c26e)
      ad868afd
    • Dave Chinner's avatar
      xfs: fix log recovery transaction item reordering · 75406170
      Dave Chinner authored
      There are several constraints that inode allocation and unlink
      logging impose on log recovery. These all stem from the fact that
      inode alloc/unlink are logged in buffers, but all other inode
      changes are logged in inode items. Hence there are ordering
      constraints that recovery must follow to ensure the correct result
      occurs.
      
      As it turns out, this ordering has been working mostly by chance
      than good management. The existing code moves all buffers except
      cancelled buffers to the head of the list, and everything else to
      the tail of the list. The problem with this is that is interleaves
      inode items with the buffer cancellation items, and hence whether
      the inode item in an cancelled buffer gets replayed is essentially
      left to chance.
      
      Further, this ordering causes problems for log recovery when inode
      CRCs are enabled. It typically replays the inode unlink buffer long before
      it replays the inode core changes, and so the CRC recorded in an
      unlink buffer is going to be invalid and hence any attempt to
      validate the inode in the buffer is going to fail. Hence we really
      need to enforce the ordering that the inode alloc/unlink code has
      expected log recovery to have since inode chunk de-allocation was
      introduced back in 2003...
      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>
      
      (cherry picked from commit a775ad77)
      75406170
    • Dave Chinner's avatar
      xfs: fix remote attribute invalidation for a leaf · ea929536
      Dave Chinner authored
      When invalidating an attribute leaf block block, there might be
      remote attributes that it points to. With the recent rework of the
      remote attribute format, we have to make sure we calculate the
      length of the attribute correctly. We aren't doing that in
      xfs_attr3_leaf_inactive(), so fix it.
      Signed-off-by: default avatarDave Chinner <dchinner@redhat.com>
      Reviewed-by: default avatarBrian Foster <bfoster@redhat.com>
      Reviewed-by: default avatarMark Tinguely <tinuguely@sgi.com>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      
      (cherry picked from commit 59913f14)
      ea929536
    • Dave Chinner's avatar
      xfs: rework dquot CRCs · bb9b8e86
      Dave Chinner authored
      Calculating dquot CRCs when the backing buffer is written back just
      doesn't work reliably. There are several places which manipulate
      dquots directly in the buffers, and they don't calculate CRCs
      appropriately, nor do they always set the buffer up to calculate
      CRCs appropriately.
      
      Firstly, if we log a dquot buffer (e.g. during allocation) it gets
      logged without valid CRC, and so on recovery we end up with a dquot
      that is not valid.
      
      Secondly, if we recover/repair a dquot, we don't have a verifier
      attached to the buffer and hence CRCs are not calculated on the way
      down to disk.
      
      Thirdly, calculating the CRC after we've changed the contents means
      that if we re-read the dquot from the buffer, we cannot verify the
      contents of the dquot are valid, as the CRC is invalid.
      
      So, to avoid all the dquot CRC errors that are being detected by the
      read verifier, change to using the same model as for inodes. That
      is, dquot CRCs are calculated and written to the backing buffer at
      the time the dquot is flushed to the backing buffer. If we modify
      the dquot directly in the backing buffer, calculate the CRC
      immediately after the modification is complete. Hence the dquot in
      the on-disk buffer should always have a valid CRC.
      Signed-off-by: default avatarDave Chinner <dchinner@redhat.com>
      Reviewed-by: default avatarBrian Foster <bfoster@redhat.com>
      Reviewed-by: default avatarBen Myers <bpm@sgi.com>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      
      (cherry picked from commit 6fcdc59d)
      bb9b8e86
  3. 30 May, 2013 14 commits
    • Dave Chinner's avatar
      xfs: rework remote attr CRCs · 7bc0dc27
      Dave Chinner authored
      Note: this changes the on-disk remote attribute format. I assert
      that this is OK to do as CRCs are marked experimental and the first
      kernel it is included in has not yet reached release yet. Further,
      the userspace utilities are still evolving and so anyone using this
      stuff right now is a developer or tester using volatile filesystems
      for testing this feature. Hence changing the format right now to
      save longer term pain is the right thing to do.
      
      The fundamental change is to move from a header per extent in the
      attribute to a header per filesytem block in the attribute. This
      means there are more header blocks and the parsing of the attribute
      data is slightly more complex, but it has the advantage that we
      always know the size of the attribute on disk based on the length of
      the data it contains.
      
      This is where the header-per-extent method has problems. We don't
      know the size of the attribute on disk without first knowing how
      many extents are used to hold it. And we can't tell from a
      mapping lookup, either, because remote attributes can be allocated
      contiguously with other attribute blocks and so there is no obvious
      way of determining the actual size of the atribute on disk short of
      walking and mapping buffers.
      
      The problem with this approach is that if we map a buffer
      incorrectly (e.g. we make the last buffer for the attribute data too
      long), we then get buffer cache lookup failure when we map it
      correctly. i.e. we get a size mismatch on lookup. This is not
      necessarily fatal, but it's a cache coherency problem that can lead
      to returning the wrong data to userspace or writing the wrong data
      to disk. And debug kernels will assert fail if this occurs.
      
      I found lots of niggly little problems trying to fix this issue on a
      4k block size filesystem, finally getting it to pass with lots of
      fixes. The thing is, 1024 byte filesystems still failed, and it was
      getting really complex handling all the corner cases that were
      showing up. And there were clearly more that I hadn't found yet.
      
      It is complex, fragile code, and if we don't fix it now, it will be
      complex, fragile code forever more.
      
      Hence the simple fix is to add a header to each filesystem block.
      This gives us the same relationship between the attribute data
      length and the number of blocks on disk as we have without CRCs -
      it's a linear mapping and doesn't require us to guess anything. It
      is simple to implement, too - the remote block count calculated at
      lookup time can be used by the remote attribute set/get/remove code
      without modification for both CRC and non-CRC filesystems. The world
      becomes sane again.
      
      Because the copy-in and copy-out now need to iterate over each
      filesystem block, I moved them into helper functions so we separate
      the block mapping and buffer manupulations from the attribute data
      and CRC header manipulations. The code becomes much clearer as a
      result, and it is a lot easier to understand and debug. It also
      appears to be much more robust - once it worked on 4k block size
      filesystems, it has worked without failure on 1k block size
      filesystems, too.
      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>
      
      (cherry picked from commit ad1858d7)
      7bc0dc27
    • Dave Chinner's avatar
      xfs: fully initialise temp leaf in xfs_attr3_leaf_compact · 634fd532
      Dave Chinner authored
      xfs_attr3_leaf_compact() uses a temporary buffer for compacting the
      the entries in a leaf. It copies the the original buffer into the
      temporary buffer, then zeros the original buffer completely. It then
      copies the entries back into the original buffer.  However, the
      original buffer has not been correctly initialised, and so the
      movement of the entries goes horribly wrong.
      
      Make sure the zeroed destination buffer is fully initialised, and
      once we've set up the destination incore header appropriately, write
      is back to the buffer before starting to move entries around.
      
      While debugging this, the _d/_s prefixes weren't sufficient to
      remind me what buffer was what, so rename then all _src/_dst.
      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>
      
      (cherry picked from commit d4c712bc)
      634fd532
    • Dave Chinner's avatar
      xfs: fully initialise temp leaf in xfs_attr3_leaf_unbalance · 9e80c762
      Dave Chinner authored
      xfs_attr3_leaf_unbalance() uses a temporary buffer for recombining
      the entries in two leaves when the destination leaf requires
      compaction. The temporary buffer ends up being copied back over the
      original destination buffer, so the header in the temporary buffer
      needs to contain all the information that is in the destination
      buffer.
      
      To make sure the temporary buffer is fully initialised, once we've
      set up the temporary incore header appropriately, write is back to
      the temporary buffer before starting to move entries around.
      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>
      
      (cherry picked from commit 8517de2a)
      9e80c762
    • Dave Chinner's avatar
      xfs: correctly map remote attr buffers during removal · 58a72281
      Dave Chinner authored
      If we don't map the buffers correctly (same as for get/set
      operations) then the incore buffer lookup will fail. If a block
      number matches but a length is wrong, then debug kernels will ASSERT
      fail in _xfs_buf_find() due to the length mismatch. Ensure that we
      map the buffers correctly by basing the length of the buffer on the
      attribute data length rather than the remote block count.
      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>
      
      (cherry picked from commit 6863ef84)
      58a72281
    • Dave Chinner's avatar
      xfs: remote attribute tail zeroing does too much · 26f71445
      Dave Chinner authored
      When an attribute data does not fill then entire remote block, we
      zero the remaining part of the buffer. This, however, needs to take
      into account that the buffer has a header, and so the offset where
      zeroing starts and the length of zeroing need to take this into
      account. Otherwise we end up with zeros over the end of the
      attribute value when CRCs are enabled.
      
      While there, make sure we only ask to map an extent that covers the
      remaining range of the attribute, rather than asking every time for
      the full length of remote data. If the remote attribute blocks are
      contiguous with other parts of the attribute tree, it will map those
      blocks as well and we can potentially zero them incorrectly. We can
      also get buffer size mistmatches when trying to read or remove the
      remote attribute, and this can lead to not finding the correct
      buffer when looking it up in cache.
      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>
      
      (cherry picked from commit 4af3644c)
      26f71445
    • Dave Chinner's avatar
      xfs: remote attribute read too short · 551b382f
      Dave Chinner authored
      Reading a maximally size remote attribute fails when CRCs are
      enabled with this verification error:
      
      XFS (vdb): remote attribute header does not match required off/len/owner)
      
      There are two reasons for this, the first being that the
      length of the buffer being read is determined from the
      args->rmtblkcnt which doesn't take into account CRC headers. Hence
      the mapped length ends up being too short and so we need to
      calculate it directly from the value length.
      
      The second is that the byte count of valid data within a buffer is
      capped by the length of the data and so doesn't take into account
      that the buffer might be longer due to headers. Hence we need to
      calculate the data space in the buffer first before calculating the
      actual byte count of data.
      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>
      
      (cherry picked from commit 913e96bc)
      551b382f
    • Dave Chinner's avatar
      xfs: remote attribute allocation may be contiguous · 9531e2de
      Dave Chinner authored
      When CRCs are enabled, there may be multiple allocations made if the
      headers cause a length overflow. This, however, does not mean that
      the number of headers required increases, as the second and
      subsequent extents may be contiguous with the previous extent. Hence
      when we map the extents to write the attribute data, we may end up
      with less extents than allocations made. Hence the assertion that we
      consume the number of headers we calculated in the allocation loop
      is incorrect and needs to be removed.
      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>
      
      (cherry picked from commit 90253cf1)
      9531e2de
    • Dave Chinner's avatar
      xfs: fix dir3 freespace block corruption · e400d27d
      Dave Chinner authored
      When the directory freespace index grows to a second block (2017
      4k data blocks in the directory), the initialisation of the second
      new block header goes wrong. The write verifier fires a corruption
      error indicating that the block number in the header is zero. This
      was being tripped by xfs/110.
      
      The problem is that the initialisation of the new block is done just
      fine in xfs_dir3_free_get_buf(), but the caller then users a dirv2
      structure to zero on-disk header fields that xfs_dir3_free_get_buf()
      has already zeroed. These lined up with the block number in the dir
      v3 header format.
      
      While looking at this, I noticed that the struct xfs_dir3_free_hdr()
      had 4 bytes of padding in it that wasn't defined as padding or being
      zeroed by the initialisation. Add a pad field declaration and fully
      zero the on disk and in-core headers in xfs_dir3_free_get_buf() so
      that this is never an issue in the future. Note that this doesn't
      change the on-disk layout, just makes the 32 bits of padding in the
      layout explicit.
      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>
      
      (cherry picked from commit 5ae6e6a4)
      e400d27d
    • Dave Chinner's avatar
      xfs: disable swap extents ioctl on CRC enabled filesystems · 7c9950fd
      Dave Chinner authored
      Currently, swapping extents from one inode to another is a simple
      act of switching data and attribute forks from one inode to another.
      This, unfortunately in no longer so simple with CRC enabled
      filesystems as there is owner information embedded into the BMBT
      blocks that are swapped between inodes. Hence swapping the forks
      between inodes results in the inodes having mapping blocks that
      point to the wrong owner and hence are considered corrupt.
      
      To fix this we need an extent tree block or record based swap
      algorithm so that the BMBT block owner information can be updated
      atomically in the swap transaction. This is a significant piece of
      new work, so for the moment simply don't allow swap extent
      operations to succeed on CRC enabled filesystems.
      Signed-off-by: default avatarDave Chinner <dchinner@redhat.com>
      Reviewed-by: default avatarBen Myers <bpm@sgi.com>
      Reviewed-by: default avatarBrian Foster <bfoster@redhat.com>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      
      (cherry picked from commit 02f75405)
      7c9950fd
    • Dave Chinner's avatar
      xfs: add fsgeom flag for v5 superblock support. · e7927e87
      Dave Chinner authored
      Currently userspace has no way of determining that a filesystem is
      CRC enabled. Add a flag to the XFS_IOC_FSGEOMETRY ioctl output to
      indicate that the filesystem has v5 superblock support enabled.
      This will allow xfs_info to correctly report the state of the
      filesystem.
      Signed-off-by: default avatarDave Chinner <dchinner@redhat.com>
      Reviewed-by: default avatarEric Sandeen <sandeen@redhat.com>
      Reviewed-by: default avatarBrian Foster <bfoster@redhat.com>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      
      (cherry picked from commit 74137fff)
      e7927e87
    • Dave Chinner's avatar
      xfs: fix incorrect remote symlink block count · 1de09d1a
      Dave Chinner authored
      When CRCs are enabled, the number of blocks needed to hold a remote
      symlink on a 1k block size filesystem may be 2 instead of 1. The
      transaction reservation for the allocated blocks was not taking this
      into account and only allocating one block. Hence when trying to
      read or invalidate such symlinks, we are mapping a hole where there
      should be a block and things go bad at that point.
      
      Fix the reservation to use the correct block count, clean up the
      block count calculation similar to the remote attribute calculation,
      and add a debug guard to detect when we don't write the entire
      symlink to disk.
      Signed-off-by: default avatarDave Chinner <dchinner@redhat.com>
      Reviewed-by: default avatarBen Myers <bpm@sgi.com>
      Reviewed-by: default avatarBrian Foster <bfoster@redhat.com>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      
      (cherry picked from commit 321a9583)
      1de09d1a
    • Dave Chinner's avatar
      xfs: fix split buffer vector log recovery support · 7d2ffe80
      Dave Chinner authored
      A long time ago in a galaxy far away....
      
      .. the was a commit made to fix some ilinux specific "fragmented
      buffer" log recovery problem:
      
      http://oss.sgi.com/cgi-bin/gitweb.cgi?p=archive/xfs-import.git;a=commitdiff;h=b29c0bece51da72fb3ff3b61391a391ea54e1603
      
      That problem occurred when a contiguous dirty region of a buffer was
      split across across two pages of an unmapped buffer. It's been a
      long time since that has been done in XFS, and the changes to log
      the entire inode buffers for CRC enabled filesystems has
      re-introduced that corner case.
      
      And, of course, it turns out that the above commit didn't actually
      fix anything - it just ensured that log recovery is guaranteed to
      fail when this situation occurs. And now for the gory details.
      
      xfstest xfs/085 is failing with this assert:
      
      XFS (vdb): bad number of regions (0) in inode log format
      XFS: Assertion failed: 0, file: fs/xfs/xfs_log_recover.c, line: 1583
      
      Largely undocumented factoid #1: Log recovery depends on all log
      buffer format items starting with this format:
      
      struct foo_log_format {
      	__uint16_t	type;
      	__uint16_t	size;
      	....
      
      As recoery uses the size field and assumptions about 32 bit
      alignment in decoding format items.  So don't pay much attention to
      the fact log recovery thinks that it decoding an inode log format
      item - it just uses them to determine what the size of the item is.
      
      But why would it see a log format item with a zero size? Well,
      luckily enough xfs_logprint uses the same code and gives the same
      error, so with a bit of gdb magic, it turns out that it isn't a log
      format that is being decoded. What logprint tells us is this:
      
      Oper (130): tid: a0375e1a  len: 28  clientid: TRANS  flags: none
      BUF:  #regs: 2   start blkno: 144 (0x90)  len: 16  bmap size: 2  flags: 0x4000
      Oper (131): tid: a0375e1a  len: 4096  clientid: TRANS  flags: none
      BUF DATA
      ----------------------------------------------------------------------------
      Oper (132): tid: a0375e1a  len: 4096  clientid: TRANS  flags: none
      xfs_logprint: unknown log operation type (4e49)
      **********************************************************************
      * ERROR: data block=2                                                 *
      **********************************************************************
      
      That we've got a buffer format item (oper 130) that has two regions;
      the format item itself and one dirty region. The subsequent region
      after the buffer format item and it's data is them what we are
      tripping over, and the first bytes of it at an inode magic number.
      Not a log opheader like there is supposed to be.
      
      That means there's a problem with the buffer format item. It's dirty
      data region is 4096 bytes, and it contains - you guessed it -
      initialised inodes. But inode buffers are 8k, not 4k, and we log
      them in their entirety. So something is wrong here. The buffer
      format item contains:
      
      (gdb) p /x *(struct xfs_buf_log_format *)in_f
      $22 = {blf_type = 0x123c, blf_size = 0x2, blf_flags = 0x4000,
             blf_len = 0x10, blf_blkno = 0x90, blf_map_size = 0x2,
             blf_data_map = {0xffffffff, 0xffffffff, .... }}
      
      Two regions, and a signle dirty contiguous region of 64 bits.  64 *
      128 = 8k, so this should be followed by a single 8k region of data.
      And the blf_flags tell us that the type of buffer is a
      XFS_BLFT_DINO_BUF. It contains inodes. And because it doesn't have
      the XFS_BLF_INODE_BUF flag set, that means it's an inode allocation
      buffer. So, it should be followed by 8k of inode data.
      
      But we know that the next region has a header of:
      
      (gdb) p /x *ohead
      $25 = {oh_tid = 0x1a5e37a0, oh_len = 0x100000, oh_clientid = 0x69,
             oh_flags = 0x0, oh_res2 = 0x0}
      
      and so be32_to_cpu(oh_len) = 0x1000 = 4096 bytes. It's simply not
      long enough to hold all the logged data. There must be another
      region. There is - there's a following opheader for another 4k of
      data that contains the other half of the inode cluster data - the
      one we assert fail on because it's not a log format header.
      
      So why is the second part of the data not being accounted to the
      correct buffer log format structure? It took a little more work with
      gdb to work out that the buffer log format structure was both
      expecting it to be there but hadn't accounted for it. It was at that
      point I went to the kernel code, as clearly this wasn't a bug in
      xfs_logprint and the kernel was writing bad stuff to the log.
      
      First port of call was the buffer item formatting code, and the
      discontiguous memory/contiguous dirty region handling code
      immediately stood out. I've wondered for a long time why the code
      had this comment in it:
      
                              vecp->i_addr = xfs_buf_offset(bp, buffer_offset);
                              vecp->i_len = nbits * XFS_BLF_CHUNK;
                              vecp->i_type = XLOG_REG_TYPE_BCHUNK;
      /*
       * You would think we need to bump the nvecs here too, but we do not
       * this number is used by recovery, and it gets confused by the boundary
       * split here
       *                      nvecs++;
       */
                              vecp++;
      
      And it didn't account for the extra vector pointer. The case being
      handled here is that a contiguous dirty region lies across a
      boundary that cannot be memcpy()d across, and so has to be split
      into two separate operations for xlog_write() to perform.
      
      What this code assumes is that what is written to the log is two
      consecutive blocks of data that are accounted in the buf log format
      item as the same contiguous dirty region and so will get decoded as
      such by the log recovery code.
      
      The thing is, xlog_write() knows nothing about this, and so just
      does it's normal thing of adding an opheader for each vector. That
      means the 8k region gets written to the log as two separate regions
      of 4k each, but because nvecs has not been incremented, the buf log
      format item accounts for only one of them.
      
      Hence when we come to log recovery, we process the first 4k region
      and then expect to come across a new item that starts with a log
      format structure of some kind that tells us whenteh next data is
      going to be. Instead, we hit raw buffer data and things go bad real
      quick.
      
      So, the commit from 2002 that commented out nvecs++ is just plain
      wrong. It breaks log recovery completely, and it would seem the only
      reason this hasn't been since then is that we don't log large
      contigous regions of multi-page unmapped buffers very often. Never
      would be a closer estimate, at least until the CRC code came along....
      
      So, lets fix that by restoring the nvecs accounting for the extra
      region when we hit this case.....
      
      .... and there's the problemin log recovery it is apparently working
      around:
      
      XFS: Assertion failed: i == item->ri_total, file: fs/xfs/xfs_log_recover.c, line: 2135
      
      Yup, xlog_recover_do_reg_buffer() doesn't handle contigous dirty
      regions being broken up into multiple regions by the log formatting
      code. That's an easy fix, though - if the number of contiguous dirty
      bits exceeds the length of the region being copied out of the log,
      only account for the number of dirty bits that region covers, and
      then loop again and copy more from the next region. It's a 2 line
      fix.
      
      Now xfstests xfs/085 passes, we have one less piece of mystery
      code, and one more important piece of knowledge about how to
      structure new log format items..
      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>
      
      (cherry picked from commit 709da6a6)
      7d2ffe80
    • Dave Chinner's avatar
      xfs: kill suid/sgid through the truncate path. · 2962f5a5
      Dave Chinner authored
      XFS has failed to kill suid/sgid bits correctly when truncating
      files of non-zero size since commit c4ed4243 ("xfs: split
      xfs_setattr") introduced in the 3.1 kernel. Fix it.
      
      Fix it.
      
      cc: stable kernel <stable@vger.kernel.org>
      Signed-off-by: default avatarDave Chinner <dchinner@redhat.com>
      Reviewed-by: default avatarBrian Foster <bfoster@redhat.com>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      
      (cherry picked from commit 56c19e89)
      2962f5a5
    • Dave Chinner's avatar
      xfs: avoid nesting transactions in xfs_qm_scall_setqlim() · 08fb3905
      Dave Chinner authored
      Lockdep reports:
      
      =============================================
      [ INFO: possible recursive locking detected ]
      3.9.0+ #3 Not tainted
      ---------------------------------------------
      setquota/28368 is trying to acquire lock:
       (sb_internal){++++.?}, at: [<c11e8846>] xfs_trans_alloc+0x26/0x50
      
      but task is already holding lock:
       (sb_internal){++++.?}, at: [<c11e8846>] xfs_trans_alloc+0x26/0x50
      
      from xfs_qm_scall_setqlim()->xfs_dqread() when a dquot needs to be
      allocated.
      
      xfs_qm_scall_setqlim() is starting a transaction and then not
      passing it into xfs_qm_dqet() and so it starts it's own transaction
      when allocating the dquot.  Splat!
      
      Fix this by not allocating the dquot in xfs_qm_scall_setqlim()
      inside the setqlim transaction. This requires getting the dquot
      first (and allocating it if necessary) then dropping and relocking
      the dquot before joining it to the setqlim transaction.
      Reported-by: default avatarMichael L. Semon <mlsemon35@gmail.com>
      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>
      (cherry picked from commit f648167f)
      08fb3905
  4. 24 May, 2013 7 commits
    • Dave Chinner's avatar
      xfs: remote attribute lookups require the value length · 7ae07780
      Dave Chinner authored
      When reading a remote attribute, to correctly calculate the length
      of the data buffer for CRC enable filesystems, we need to know the
      length of the attribute data. We get this information when we look
      up the attribute, but we don't store it in the args structure along
      with the other remote attr information we get from the lookup. Add
      this information to the args structure so we can use it
      appropriately.
      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>
      
      (cherry picked from commit e461fcb1)
      7ae07780
    • Dave Chinner's avatar
      xfs: xfs_attr_shortform_allfit() does not handle attr3 format. · cf257abf
      Dave Chinner authored
      xfstests generic/117 fails with:
      
      XFS: Assertion failed: leaf->hdr.info.magic == cpu_to_be16(XFS_ATTR_LEAF_MAGIC)
      
      indicating a function that does not handle the attr3 format
      correctly. Fix it.
      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>
      (cherry picked from commit b38958d7)
      cf257abf
    • Dave Chinner's avatar
      xfs: xfs_da3_node_read_verify() doesn't handle XFS_ATTR3_LEAF_MAGIC · 7ced60ca
      Dave Chinner authored
      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>
      
      (cherry picked from commit 72916fb8)
      7ced60ca
    • Dave Chinner's avatar
      xfs: fix missing KM_NOFS tags to keep lockdep happy · b17cb364
      Dave Chinner authored
      There are several places where we use KM_SLEEP allocation contexts
      and use the fact that they are called from transaction context to
      add KM_NOFS where appropriate. Unfortunately, there are several
      places where the code makes this assumption but can be called from
      outside transaction context but with filesystem locks held. These
      places need explicit KM_NOFS annotations to avoid lockdep
      complaining about reclaim contexts.
      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>
      
      (cherry picked from commit ac14876c)
      b17cb364
    • Dave Chinner's avatar
      xfs: Don't reference the EFI after it is freed · 509e708a
      Dave Chinner authored
      Checking the EFI for whether it is being released from recovery
      after we've already released the known active reference is a mistake
      worthy of a brown paper bag. Fix the (now) obvious use after free
      that it can cause.
      Reported-by: default avatarDave Jones <davej@redhat.com>
      Signed-off-by: default avatarDave Chinner <dchinner@redhat.com>
      Reviewed-by: default avatarBrian Foster <bfoster@redhat.com>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      
      (cherry picked from commit 52c24ad3)
      509e708a
    • Dave Chinner's avatar
      xfs: fix rounding in xfs_free_file_space · 7031d0e1
      Dave Chinner authored
      The offset passed into xfs_free_file_space() needs to be rounded
      down to a certain size, but the rounding mask is built by a 32 bit
      variable. Hence the mask will always mask off the upper 32 bits of
      the offset and lead to incorrect writeback and invalidation ranges.
      
      This is not actually exposed as a bug because we writeback and
      invalidate from the rounded offset to the end of the file, and hence
      the offset we are actually punching a hole out of will always be
      covered by the code. This needs fixing, however, if we ever want to
      use exact ranges for writeback/invalidation here...
      Signed-off-by: default avatarDave Chinner <dchinner@redhat.com>
      Reviewed-by: default avatarBrian Foster <bfoster@redhat.com>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      
      (cherry picked from commit 28ca489c)
      7031d0e1
    • Dave Chinner's avatar
      xfs: fix sub-page blocksize data integrity writes · 480d7467
      Dave Chinner authored
      FSX on 512 byte block size filesystems has been failing for some
      time with corrupted data. The fault dates back to the change in
      the writeback data integrity algorithm that uses a mark-and-sweep
      approach to avoid data writeback livelocks.
      
      Unfortunately, a side effect of this mark-and-sweep approach is that
      each page will only be written once for a data integrity sync, and
      there is a condition in writeback in XFS where a page may require
      two writeback attempts to be fully written. As a result of the high
      level change, we now only get a partial page writeback during the
      integrity sync because the first pass through writeback clears the
      mark left on the page index to tell writeback that the page needs
      writeback....
      
      The cause is writing a partial page in the clustering code. This can
      happen when a mapping boundary falls in the middle of a page - we
      end up writing back the first part of the page that the mapping
      covers, but then never revisit the page to have the remainder mapped
      and written.
      
      The fix is simple - if the mapping boundary falls inside a page,
      then simple abort clustering without touching the page. This means
      that the next ->writepage entry that write_cache_pages() will make
      is the page we aborted on, and xfs_vm_writepage() will map all
      sections of the page correctly. This behaviour is also optimal for
      non-data integrity writes, as it results in contiguous sequential
      writeback of the file rather than missing small holes and having to
      write them a "random" writes in a future pass.
      
      With this fix, all the fsx tests in xfstests now pass on a 512 byte
      block size filesystem on a 4k page machine.
      Signed-off-by: default avatarDave Chinner <dchinner@redhat.com>
      Reviewed-by: default avatarBrian Foster <bfoster@redhat.com>
      Signed-off-by: default avatarBen Myers <bpm@sgi.com>
      
      (cherry picked from commit 49b137cb)
      480d7467
  5. 12 May, 2013 2 commits
    • Linus Torvalds's avatar
      Linux 3.10-rc1 · f722406f
      Linus Torvalds authored
      f722406f
    • Linus Torvalds's avatar
      Merge tag 'trace-fixes-v3.10' of git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace · 26b840ae
      Linus Torvalds authored
      Pull tracing/kprobes update from Steven Rostedt:
       "The majority of these changes are from Masami Hiramatsu bringing
        kprobes up to par with the latest changes to ftrace (multi buffering
        and the new function probes).
      
        He also discovered and fixed some bugs in doing so.  When pulling in
        his patches, I also found a few minor bugs as well and fixed them.
      
        This also includes a compile fix for some archs that select the ring
        buffer but not tracing.
      
        I based this off of the last patch you took from me that fixed the
        merge conflict error, as that was the commit that had all the changes
        I needed for this set of changes."
      
      * tag 'trace-fixes-v3.10' of git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace:
        tracing/kprobes: Support soft-mode disabling
        tracing/kprobes: Support ftrace_event_file base multibuffer
        tracing/kprobes: Pass trace_probe directly from dispatcher
        tracing/kprobes: Increment probe hit-count even if it is used by perf
        tracing/kprobes: Use bool for retprobe checker
        ftrace: Fix function probe when more than one probe is added
        ftrace: Fix the output of enabled_functions debug file
        ftrace: Fix locking in register_ftrace_function_probe()
        tracing: Add helper function trace_create_new_event() to remove duplicate code
        tracing: Modify soft-mode only if there's no other referrer
        tracing: Indicate enabled soft-mode in enable file
        tracing/kprobes: Fix to increment return event probe hit-count
        ftrace: Cleanup regex_lock and ftrace_lock around hash updating
        ftrace, kprobes: Fix a deadlock on ftrace_regex_lock
        ftrace: Have ftrace_regex_write() return either read or error
        tracing: Return error if register_ftrace_function_probe() fails for event_enable_func()
        tracing: Don't succeed if event_enable_func did not register anything
        ring-buffer: Select IRQ_WORK
      26b840ae
  6. 11 May, 2013 4 commits
    • Linus Torvalds's avatar
      Merge tag 'stable/for-linus-3.10-rc0-tag-two' of... · 607eeb0b
      Linus Torvalds authored
      Merge tag 'stable/for-linus-3.10-rc0-tag-two' of git://git.kernel.org/pub/scm/linux/kernel/git/konrad/xen
      
      Pull Xen bug-fixes from Konrad Rzeszutek Wilk:
       - More fixes in the vCPU PVHVM hotplug path.
       - Add more documentation.
       - Fix various ARM related issues in the Xen generic drivers.
       - Updates in the xen-pciback driver per Bjorn's updates.
       - Mask the x2APIC feature for PV guests.
      
      * tag 'stable/for-linus-3.10-rc0-tag-two' of git://git.kernel.org/pub/scm/linux/kernel/git/konrad/xen:
        xen/pci: Used cached MSI-X capability offset
        xen/pci: Use PCI_MSIX_TABLE_BIR, not PCI_MSIX_FLAGS_BIRMASK
        xen: clear IRQ_NOAUTOEN and IRQ_NOREQUEST
        xen: mask x2APIC feature in PV
        xen: SWIOTLB is only used on x86
        xen/spinlock: Fix check from greater than to be also be greater or equal to.
        xen/smp/pvhvm: Don't point per_cpu(xen_vpcu, 33 and larger) to shared_info
        xen/vcpu: Document the xen_vcpu_info and xen_vcpu
        xen/vcpu/pvhvm: Fix vcpu hotplugging hanging.
      607eeb0b
    • Linus Torvalds's avatar
      Merge tag 'scsi-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi · 4c444501
      Linus Torvalds authored
      Pull second SCSI update from James "Jaj B" Bottomley:
       "This is the final round of SCSI patches for the merge window.  It
        consists mostly of driver updates (bnx2fc, ibmfc, fnic, lpfc,
        be2iscsi, pm80xx, qla4x and ipr).
      
        There's also the power management updates that complete the patches in
        Jens' tree, an iscsi refcounting problem fix from the last pull, some
        dif handling in scsi_debug fixes, a few nice code cleanups and an
        error handling busy bug fix."
      
      * tag 'scsi-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi: (92 commits)
        [SCSI] qla2xxx: Update firmware link in Kconfig file.
        [SCSI] iscsi class, qla4xxx: fix sess/conn refcounting when find fns are used
        [SCSI] sas: unify the pointlessly separated enums sas_dev_type and sas_device_type
        [SCSI] pm80xx: thermal, sas controller config and error handling update
        [SCSI] pm80xx: NCQ error handling changes
        [SCSI] pm80xx: WWN Modification for PM8081/88/89 controllers
        [SCSI] pm80xx: Changed module name and debug messages update
        [SCSI] pm80xx: Firmware flash memory free fix, with addition of new memory region for it
        [SCSI] pm80xx: SPC new firmware changes for device id 0x8081 alone
        [SCSI] pm80xx: Added SPCv/ve specific hardware functionalities and relevant changes in common files
        [SCSI] pm80xx: MSI-X implementation for using 64 interrupts
        [SCSI] pm80xx: Updated common functions common for SPC and SPCv/ve
        [SCSI] pm80xx: Multiple inbound/outbound queue configuration
        [SCSI] pm80xx: Added SPCv/ve specific ids, variables and modify for SPC
        [SCSI] lpfc: fix up Kconfig dependencies
        [SCSI] Handle MLQUEUE busy response in scsi_send_eh_cmnd
        [SCSI] sd: change to auto suspend mode
        [SCSI] sd: use REQ_PM in sd's runtime suspend operation
        [SCSI] qla4xxx: Fix iocb_cnt calculation in qla4xxx_send_mbox_iocb()
        [SCSI] ufs: Correct the expected data transfersize
        ...
      4c444501
    • Linus Torvalds's avatar
      Merge branch 'release' of git://git.kernel.org/pub/scm/linux/kernel/git/lenb/linux · ac4e0109
      Linus Torvalds authored
      Pull idle update from Len Brown:
       "Add support for new Haswell-ULT CPU idle power states"
      
      * 'release' of git://git.kernel.org/pub/scm/linux/kernel/git/lenb/linux:
        intel_idle: initial C8, C9, C10 support
        tools/power turbostat: display C8, C9, C10 residency
      ac4e0109
    • Linus Torvalds's avatar
      Merge git://git.infradead.org/users/eparis/audit · c4cc75c3
      Linus Torvalds authored
      Pull audit changes from Eric Paris:
       "Al used to send pull requests every couple of years but he told me to
        just start pushing them to you directly.
      
        Our touching outside of core audit code is pretty straight forward.  A
        couple of interface changes which hit net/.  A simple argument bug
        calling audit functions in namei.c and the removal of some assembly
        branch prediction code on ppc"
      
      * git://git.infradead.org/users/eparis/audit: (31 commits)
        audit: fix message spacing printing auid
        Revert "audit: move kaudit thread start from auditd registration to kaudit init"
        audit: vfs: fix audit_inode call in O_CREAT case of do_last
        audit: Make testing for a valid loginuid explicit.
        audit: fix event coverage of AUDIT_ANOM_LINK
        audit: use spin_lock in audit_receive_msg to process tty logging
        audit: do not needlessly take a lock in tty_audit_exit
        audit: do not needlessly take a spinlock in copy_signal
        audit: add an option to control logging of passwords with pam_tty_audit
        audit: use spin_lock_irqsave/restore in audit tty code
        helper for some session id stuff
        audit: use a consistent audit helper to log lsm information
        audit: push loginuid and sessionid processing down
        audit: stop pushing loginid, uid, sessionid as arguments
        audit: remove the old depricated kernel interface
        audit: make validity checking generic
        audit: allow checking the type of audit message in the user filter
        audit: fix build break when AUDIT_DEBUG == 2
        audit: remove duplicate export of audit_enabled
        Audit: do not print error when LSMs disabled
        ...
      c4cc75c3
  7. 10 May, 2013 3 commits