1. 20 Mar, 2013 1 commit
    • Theodore Ts'o's avatar
      ext4: fix ext4_evict_inode() racing against workqueue processing code · 1ada47d9
      Theodore Ts'o authored
      Commit 84c17543 (ext4: move work from io_end to inode) triggered a
      regression when running xfstest #270 when the file system is mounted
      with dioread_nolock.
      
      The problem is that after ext4_evict_inode() calls ext4_ioend_wait(),
      this guarantees that last io_end structure has been freed, but it does
      not guarantee that the workqueue structure, which was moved into the
      inode by commit 84c17543, is actually finished.  Once
      ext4_flush_completed_IO() calls ext4_free_io_end() on CPU #1, this
      will allow ext4_ioend_wait() to return on CPU #2, at which point the
      evict_inode() codepath can race against the workqueue code on CPU #1
      accessing EXT4_I(inode)->i_unwritten_work to find the next item of
      work to do.
      
      Fix this by calling cancel_work_sync() in ext4_ioend_wait(), which
      will be renamed ext4_ioend_shutdown(), since it is only used by
      ext4_evict_inode().  Also, move the call to ext4_ioend_shutdown()
      until after truncate_inode_pages() and filemap_write_and_wait() are
      called, to make sure all dirty pages have been written back and
      flushed from the page cache first.
      
      BUG: unable to handle kernel NULL pointer dereference at   (null)
      IP: [<c01dda6a>] cwq_activate_delayed_work+0x3b/0x7e
      *pdpt = 0000000030bc3001 *pde = 0000000000000000 
      Oops: 0000 [#1] SMP DEBUG_PAGEALLOC
      Modules linked in:
      Pid: 6, comm: kworker/u:0 Not tainted 3.8.0-rc3-00013-g84c17543-dirty #91 Bochs Bochs
      EIP: 0060:[<c01dda6a>] EFLAGS: 00010046 CPU: 0
      EIP is at cwq_activate_delayed_work+0x3b/0x7e
      EAX: 00000000 EBX: 00000000 ECX: f505fe54 EDX: 00000000
      ESI: ed5b697c EDI: 00000006 EBP: f64b7e8c ESP: f64b7e84
       DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068
      CR0: 8005003b CR2: 00000000 CR3: 30bc2000 CR4: 000006f0
      DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
      DR6: ffff0ff0 DR7: 00000400
      Process kworker/u:0 (pid: 6, ti=f64b6000 task=f64b4160 task.ti=f64b6000)
      Stack:
       f505fe00 00000006 f64b7e9c c01de3d7 f6435540 00000003 f64b7efc c01def1d
       f6435540 00000002 00000000 0000008a c16d0808 c040a10b c16d07d8 c16d08b0
       f505fe00 c16d0780 00000000 00000000 ee153df4 c1ce4a30 c17d0e30 00000000
      Call Trace:
       [<c01de3d7>] cwq_dec_nr_in_flight+0x71/0xfb
       [<c01def1d>] process_one_work+0x5d8/0x637
       [<c040a10b>] ? ext4_end_bio+0x300/0x300
       [<c01e3105>] worker_thread+0x249/0x3ef
       [<c01ea317>] kthread+0xd8/0xeb
       [<c01e2ebc>] ? manage_workers+0x4bb/0x4bb
       [<c023a370>] ? trace_hardirqs_on+0x27/0x37
       [<c0f1b4b7>] ret_from_kernel_thread+0x1b/0x28
       [<c01ea23f>] ? __init_kthread_worker+0x71/0x71
      Code: 01 83 15 ac ff 6c c1 00 31 db 89 c6 8b 00 a8 04 74 12 89 c3 30 db 83 05 b0 ff 6c c1 01 83 15 b4 ff 6c c1 00 89 f0 e8 42 ff ff ff <8b> 13 89 f0 83 05 b8 ff 6c c1
       6c c1 00 31 c9 83
      EIP: [<c01dda6a>] cwq_activate_delayed_work+0x3b/0x7e SS:ESP 0068:f64b7e84
      CR2: 0000000000000000
      ---[ end trace a1923229da53d8a4 ]---
      Signed-off-by: default avatar"Theodore Ts'o" <tytso@mit.edu>
      Cc: Jan Kara <jack@suse.cz>
      1ada47d9
  2. 18 Mar, 2013 1 commit
  3. 12 Mar, 2013 2 commits
  4. 11 Mar, 2013 11 commits
    • Jan Kara's avatar
      jbd2: fix use after free in jbd2_journal_dirty_metadata() · ad56edad
      Jan Kara authored
      jbd2_journal_dirty_metadata() didn't get a reference to journal_head it
      was working with. This is OK in most of the cases since the journal head
      should be attached to a transaction but in rare occasions when we are
      journalling data, __ext4_journalled_writepage() can race with
      jbd2_journal_invalidatepage() stripping buffers from a page and thus
      journal head can be freed under hands of jbd2_journal_dirty_metadata().
      
      Fix the problem by getting own journal head reference in
      jbd2_journal_dirty_metadata() (and also in jbd2_journal_set_triggers()
      which can possibly have the same issue).
      Reported-by: default avatarZheng Liu <gnehzuil.liu@gmail.com>
      Signed-off-by: default avatarJan Kara <jack@suse.cz>
      Signed-off-by: default avatar"Theodore Ts'o" <tytso@mit.edu>
      Cc: stable@vger.kernel.org
      ad56edad
    • Lukas Czerner's avatar
      ext4: reserve metadata block for every delayed write · 386ad67c
      Lukas Czerner authored
      Currently we only reserve space (data+metadata) in delayed allocation if
      we're allocating from new cluster (which is always in non-bigalloc file
      system) which is ok for data blocks, because we reserve the whole cluster.
      
      However we have to reserve metadata for every delayed block we're going
      to write because every block could potentially require metedata block
      when we need to grow the extent tree.
      Signed-off-by: default avatarLukas Czerner <lczerner@redhat.com>
      386ad67c
    • Lukas Czerner's avatar
      ext4: update reserved space after the 'correction' · 232ec872
      Lukas Czerner authored
      Currently in ext4_ext_map_blocks() in delayed allocation writeback
      we would update the reservation and after that check whether we claimed
      cluster outside of the range of the allocation and if so, we'll give the
      block back to the reservation pool.
      
      However this also means that if the number of reserved data block
      dropped to zero before the correction, we would release all the metadata
      reservation as well, however we might still need it because the we're
      not done with the delayed allocation and there might be more blocks to
      come. This will result in error messages such as:
      
      EXT4-fs warning (device sdb): ext4_da_update_reserve_space:361: ino 12,
      allocated 1 with only 0 reserved metadata blocks (releasing 1 blocks
      with reserved 1 data blocks)
      
      This will only happen on bigalloc file system and it can be easily
      reproduced using fiemap-tester from xfstests like this:
      
      ./src/fiemap-tester -m DHDHDHDHD -S -p0 /mnt/test/file
      
      Or using xfstests such as 225.
      
      Fix this by doing the correction first and updating the reservation
      after that so that we do not accidentally decrease
      i_reserved_data_blocks to zero.
      Signed-off-by: default avatarLukas Czerner <lczerner@redhat.com>
      Signed-off-by: default avatar"Theodore Ts'o" <tytso@mit.edu>
      232ec872
    • Lukas Czerner's avatar
      ext4: do not use yield() · bb8b20ed
      Lukas Czerner authored
      Using yield() is strongly discouraged (see sched/core.c) especially
      since we can just use cond_resched().
      
      Replace all use of yield() with cond_resched().
      Signed-off-by: default avatarLukas Czerner <lczerner@redhat.com>
      Signed-off-by: default avatar"Theodore Ts'o" <tytso@mit.edu>
      bb8b20ed
    • Lukas Czerner's avatar
      ext4: remove unused variable in ext4_free_blocks() · e3d85c36
      Lukas Czerner authored
      Remove unused variable 'freed' in ext4_free_blocks().
      Signed-off-by: default avatarLukas Czerner <lczerner@redhat.com>
      Signed-off-by: default avatar"Theodore Ts'o" <tytso@mit.edu>
      e3d85c36
    • Jan Kara's avatar
      ext4: fix WARN_ON from ext4_releasepage() · e1c36595
      Jan Kara authored
      ext4_releasepage() warns when it is passed a page with PageChecked set.
      However this can correctly happen when invalidate_inode_pages2_range()
      invalidates pages - and we should fail the release in that case. Since
      the page was dirty anyway, it won't be discarded and no harm has
      happened but it's good to be safe. Also remove bogus page_has_buffers()
      check - we are guaranteed page has buffers in this function.
      Reported-by: default avatarZheng Liu <gnehzuil.liu@gmail.com>
      Tested-by: default avatarZheng Liu <wenqing.lz@taobao.com>
      Signed-off-by: default avatar"Theodore Ts'o" <tytso@mit.edu>
      Signed-off-by: default avatarJan Kara <jack@suse.cz>
      e1c36595
    • Zheng Liu's avatar
      ext4: fix the wrong number of the allocated blocks in ext4_split_extent() · 3a225670
      Zheng Liu authored
      This commit fixes a wrong return value of the number of the allocated
      blocks in ext4_split_extent.  When the length of blocks we want to
      allocate is greater than the length of the current extent, we return a
      wrong number.  Let's see what happens in the following case when we
      call ext4_split_extent().
      
        map: [48, 72]
        ex:  [32, 64, u]
      
      'ex' will be split into two parts:
        ex1: [32, 47, u]
        ex2: [48, 64, w]
      
      'map->m_len' is returned from this function, and the value is 24.  But
      the real length is 16.  So it should be fixed.
      
      Meanwhile in this commit we use right length of the allocated blocks
      when get_reserved_cluster_alloc in ext4_ext_handle_uninitialized_extents
      is called.
      Signed-off-by: default avatarZheng Liu <wenqing.lz@taobao.com>
      Signed-off-by: default avatar"Theodore Ts'o" <tytso@mit.edu>
      Cc: Dmitry Monakhov <dmonakhov@openvz.org>
      Cc: stable@vger.kernel.org
      3a225670
    • Zheng Liu's avatar
      ext4: update extent status tree after an extent is zeroed out · adb23551
      Zheng Liu authored
      When we try to split an extent, this extent could be zeroed out and mark
      as initialized.  But we don't know this in ext4_map_blocks because it
      only returns a length of allocated extent.  Meanwhile we will mark this
      extent as uninitialized because we only check m_flags.
      
      This commit update extent status tree when we try to split an unwritten
      extent.  We don't need to worry about the status of this extent because
      we always mark it as initialized.
      Signed-off-by: default avatarZheng Liu <wenqing.lz@taobao.com>
      Signed-off-by: default avatar"Theodore Ts'o" <tytso@mit.edu>
      Cc: Dmitry Monakhov <dmonakhov@openvz.org>
      adb23551
    • Zheng Liu's avatar
      ext4: fix wrong m_len value after unwritten extent conversion · cdee7843
      Zheng Liu authored
      The ext4_ext_handle_uninitialized_extents() function was assuming the
      return value of ext4_ext_map_blocks() is equal to map->m_len.  This
      incorrect assumption was harmless until we started use status tree as
      a extent cache because we need to update status tree according to
      'm_len' value.
      
      Meanwhile this commit marks EXT4_MAP_MAPPED flag after unwritten extent
      conversion.  It shouldn't cause a bug because we update status tree
      according to checking EXT4_MAP_UNWRITTEN flag.  But it should be fixed.
      
      After applied this commit, the following error message from self-testing
      infrastructure disappears.
      
          ...
          kernel: ES len assertation failed for inode: 230 retval 1 !=
          map->m_len 3 in ext4_map_blocks (allocation)
          ...
      Signed-off-by: default avatarZheng Liu <wenqing.lz@taobao.com>
      Signed-off-by: default avatar"Theodore Ts'o" <tytso@mit.edu>
      Cc: Dmitry Monakhov <dmonakhov@openvz.org>
      cdee7843
    • Dmitry Monakhov's avatar
      ext4: add self-testing infrastructure to do a sanity check · 921f266b
      Dmitry Monakhov authored
      This commit adds a self-testing infrastructure like extent tree does to
      do a sanity check for extent status tree.  After status tree is as a
      extent cache, we'd better to make sure that it caches right result.
      
      After applied this commit, we will get a lot of messages when we run
      xfstests as below.
      
      ...
      kernel: ES len assertation failed for inode: 230 retval 1 != map->m_len
      3 in ext4_map_blocks (allocation)
      ...
      kernel: ES cache assertation failed for inode: 230 es_cached ex
      [974/2/4781/20] != found ex [974/1/4781/1000]
      ...
      kernel: ES insert assertation failed for inode: 635 ex_status
      [0/45/21388/w] != es_status [44/1/21432/u]
      ...
      Signed-off-by: default avatarDmitry Monakhov <dmonakhov@openvz.org>
      Signed-off-by: default avatarZheng Liu <wenqing.lz@taobao.com>
      Signed-off-by: default avatar"Theodore Ts'o" <tytso@mit.edu>
      921f266b
    • Zheng Liu's avatar
      ext4: avoid a potential overflow in ext4_es_can_be_merged() · bd384364
      Zheng Liu authored
      Check the length of an extent to avoid a potential overflow in
      ext4_es_can_be_merged().
      Signed-off-by: default avatarZheng Liu <wenqing.lz@taobao.com>
      Signed-off-by: default avatar"Theodore Ts'o" <tytso@mit.edu>
      Cc: Dmitry Monakhov <dmonakhov@openvz.org>
      bd384364
  5. 04 Mar, 2013 5 commits
  6. 02 Mar, 2013 7 commits
  7. 01 Mar, 2013 1 commit
    • Theodore Ts'o's avatar
      ext4: optimize ext4_es_shrink() · 24630774
      Theodore Ts'o authored
      When the system is under memory pressure, ext4_es_srhink() will get
      called very often.  So optimize returning the number of items in the
      file system's extent status cache by keeping a per-filesystem count,
      instead of calculating it each time by scanning all of the inodes in
      the extent status cache.
      
      Also rename the slab used for the extent status cache to be
      "ext4_extent_status" so it's obviousl the slab in question is created
      by ext4.
      Signed-off-by: default avatar"Theodore Ts'o" <tytso@mit.edu>
      Cc: Zheng Liu <gnehzuil.liu@gmail.com>
      24630774
  8. 27 Feb, 2013 1 commit
  9. 22 Feb, 2013 2 commits
    • Lukas Czerner's avatar
      ext4: fix free clusters calculation in bigalloc filesystem · 304e220f
      Lukas Czerner authored
      ext4_has_free_clusters() should tell us whether there is enough free
      clusters to allocate, however number of free clusters in the file system
      is converted to blocks using EXT4_C2B() which is not only wrong use of
      the macro (we should have used EXT4_NUM_B2C) but it's also completely
      wrong concept since everything else is in cluster units.
      
      Moreover when calculating number of root clusters we should be using
      macro EXT4_NUM_B2C() instead of EXT4_B2C() otherwise the result might be
      off by one. However r_blocks_count should always be a multiple of the
      cluster ratio so doing a plain bit shift should be enough here. We
      avoid using EXT4_B2C() because it's confusing.
      
      As a result of the first problem number of free clusters is much bigger
      than it should have been and ext4_has_free_clusters() would return 1 even
      if there is really not enough free clusters available.
      
      Fix this by removing the EXT4_C2B() conversion of free clusters and
      using bit shift when calculating number of root clusters. This bug
      affects number of xfstests tests covering file system ENOSPC situation
      handling. With this patch most of the ENOSPC problems with bigalloc file
      system disappear, especially the errors caused by delayed allocation not
      having enough space when the actual allocation is finally requested.
      Signed-off-by: default avatarLukas Czerner <lczerner@redhat.com>
      Signed-off-by: default avatar"Theodore Ts'o" <tytso@mit.edu>
      Cc: stable@vger.kernel.org
      304e220f
    • Eryu Guan's avatar
      ext4: no need to remove extent if len is 0 in ext4_es_remove_extent() · d4381472
      Eryu Guan authored
      len is 0 means no extent needs to be removed, so return immediately.
      Otherwise it could trigger the following BUG_ON() in
      ext4_es_remove_extent()
      
      	end = lblk + len - 1;
      	BUG_ON(end < lblk);
      
      This could be reproduced by a simple truncate(1) command by an
      unprivileged user
      
      	truncate -s $(($((2**32 - 1)) * 4096)) /mnt/ext4/testfile
      
      The same is true for __es_insert_extent().
      
      Patched kernel passed xfstests regression test.
      Signed-off-by: default avatarEryu Guan <guaneryu@gmail.com>
      Signed-off-by: default avatar"Theodore Ts'o" <tytso@mit.edu>
      Reviewed-by: default avatarZheng Liu <wenqing.lz@taobao.com>
      d4381472
  10. 18 Feb, 2013 9 commits
    • Lukas Czerner's avatar
      ext4: fix xattr block allocation/release with bigalloc · 1231b3a1
      Lukas Czerner authored
      Currently when new xattr block is created or released we we would call
      dquot_free_block() or dquot_alloc_block() respectively, among the else
      decrementing or incrementing the number of blocks assigned to the
      inode by one block.
      
      This however does not work for bigalloc file system because we always
      allocate/free the whole cluster so we have to count with that in
      dquot_free_block() and dquot_alloc_block() as well.
      
      Use the clusters-to-blocks conversion EXT4_C2B() when passing number of
      blocks to the dquot_alloc/free functions to fix the problem.
      
      The problem has been revealed by xfstests #117 (and possibly others).
      Signed-off-by: default avatarLukas Czerner <lczerner@redhat.com>
      Signed-off-by: default avatar"Theodore Ts'o" <tytso@mit.edu>
      Reviewed-by: default avatarEric Sandeen <sandeen@redhat.com>
      Cc: stable@vger.kernel.org
      1231b3a1
    • Zheng Liu's avatar
      ext4: reclaim extents from extent status tree · 74cd15cd
      Zheng Liu authored
      Although extent status is loaded on-demand, we also need to reclaim
      extent from the tree when we are under a heavy memory pressure because
      in some cases fragmented extent tree causes status tree costs too much
      memory.
      
      Here we maintain a lru list in super_block.  When the extent status of
      an inode is accessed and changed, this inode will be move to the tail
      of the list.  The inode will be dropped from this list when it is
      cleared.  In the inode, a counter is added to count the number of
      cached objects in extent status tree.  Here only written/unwritten/hole
      extent is counted because delayed extent doesn't be reclaimed due to
      fiemap, bigalloc and seek_data/hole need it.  The counter will be
      increased as a new extent is allocated, and it will be decreased as a
      extent is freed.
      
      In this commit we use normal shrinker framework to reclaim memory from
      the status tree.  ext4_es_reclaim_extents_count() traverses the lru list
      to count the number of reclaimable extents.  ext4_es_shrink() tries to
      reclaim written/unwritten/hole extents from extent status tree.  The
      inode that has been shrunk is moved to the tail of lru list.
      Signed-off-by: default avatarZheng Liu <wenqing.lz@taobao.com>
      Signed-off-by: default avatar"Theodore Ts'o" <tytso@mit.edu>
      Cc: Jan kara <jack@suse.cz>
      74cd15cd
    • Zheng Liu's avatar
      ext4: adjust some functions for reclaiming extents from extent status tree · bdedbb7b
      Zheng Liu authored
      This commit changes some interfaces in extent status tree because we
      need to use inode to count the cached objects in a extent status tree.
      Signed-off-by: default avatarZheng Liu <wenqing.lz@taobao.com>
      Signed-off-by: default avatar"Theodore Ts'o" <tytso@mit.edu>
      Cc: Jan kara <jack@suse.cz>
      bdedbb7b
    • Zheng Liu's avatar
      ext4: remove single extent cache · 69eb33dc
      Zheng Liu authored
      Single extent cache could be removed because we have extent status tree
      as a extent cache, and it would be better.
      Signed-off-by: default avatarZheng Liu <wenqing.lz@taobao.com>
      Signed-off-by: default avatar"Theodore Ts'o" <tytso@mit.edu>
      Cc: Jan kara <jack@suse.cz>
      69eb33dc
    • Zheng Liu's avatar
      ext4: lookup block mapping in extent status tree · d100eef2
      Zheng Liu authored
      After tracking all extent status, we already have a extent cache in
      memory.  Every time we want to lookup a block mapping, we can first
      try to lookup it in extent status tree to avoid a potential disk I/O.
      
      A new function called ext4_es_lookup_extent is defined to finish this
      work.  When we try to lookup a block mapping, we always call
      ext4_map_blocks and/or ext4_da_map_blocks.  So in these functions we
      first try to lookup a block mapping in extent status tree.
      
      A new flag EXT4_GET_BLOCKS_NO_PUT_HOLE is used in ext4_da_map_blocks
      in order not to put a hole into extent status tree because this hole
      will be converted to delayed extent in the tree immediately.
      Signed-off-by: default avatarZheng Liu <wenqing.lz@taobao.com>
      Signed-off-by: default avatar"Theodore Ts'o" <tytso@mit.edu>
      Cc: Jan kara <jack@suse.cz>
      d100eef2
    • Zheng Liu's avatar
      ext4: track all extent status in extent status tree · f7fec032
      Zheng Liu authored
      By recording the phycisal block and status, extent status tree is able
      to track the status of every extents.  When we call _map_blocks
      functions to lookup an extent or create a new written/unwritten/delayed
      extent, this extent will be inserted into extent status tree.
      
      We don't load all extents from disk in alloc_inode() because it costs
      too much memory, and if a file is opened and closed frequently it will
      takes too much time to load all extent information.  So currently when
      we create/lookup an extent, this extent will be inserted into extent
      status tree.  Hence, the extent status tree may not comprehensively
      contain all of the extents found in the file.
      
      Here a condition we need to take care is that an extent might contains
      unwritten and delayed status simultaneously because an extent is delayed
      allocated and could be allocated by fallocate.  At this time we need to
      keep delayed status because later we need to update delayed reservation
      space using it.
      Signed-off-by: default avatarZheng Liu <wenqing.lz@taobao.com>
      Signed-off-by: default avatar"Theodore Ts'o" <tytso@mit.edu>
      Cc: Jan kara <jack@suse.cz>
      f7fec032
    • Zheng Liu's avatar
      ext4: let ext4_ext_map_blocks return EXT4_MAP_UNWRITTEN flag · a25a4e1a
      Zheng Liu authored
      This commit lets ext4_ext_map_blocks return EXT4_MAP_UNWRITTEN flag
      because in later commit ext4_map_blocks needs to use this flag to
      determine the extent status.
      Signed-off-by: default avatarZheng Liu <wenqing.lz@taobao.com>
      Signed-off-by: default avatar"Theodore Ts'o" <tytso@mit.edu>
      Reviewed-by: default avatarJan Kara <jack@suse.cz>
      a25a4e1a
    • Zheng Liu's avatar
      ext4: rename and improbe ext4_es_find_extent() · be401363
      Zheng Liu authored
      This commit renames ext4_es_find_extent with ext4_es_find_delayed_extent
      and improve this function.  First, we split input and output parameter.
      Second, this function never return the first block of the next delayed
      extent after 'es'.
      Signed-off-by: default avatarZheng Liu <wenqing.lz@taobao.com>
      Signed-off-by: default avatar"Theodore Ts'o" <tytso@mit.edu>
      Cc: Jan kara <jack@suse.cz>
      be401363
    • Zheng Liu's avatar
      ext4: add physical block and status member into extent status tree · fdc0212e
      Zheng Liu authored
      This commit adds two members in extent_status structure to let it record
      physical block and extent status.  Here es_pblk is used to record both
      of them because physical block only has 48 bits.  So extent status could
      be stashed into it so that we can save some memory.  Now written,
      unwritten, delayed and hole are defined as status.
      
      Due to new member is added into extent status tree, all interfaces need
      to be adjusted.
      Signed-off-by: default avatarZheng Liu <wenqing.lz@taobao.com>
      Signed-off-by: default avatar"Theodore Ts'o" <tytso@mit.edu>
      Reviewed-by: default avatarJan Kara <jack@suse.cz>
      fdc0212e