[PATCH] Btrfs: don't monopolize a core when evicting inode

[PATCH] Btrfs: don't monopolize a core when evicting inode

[Filipe Manana]

If an inode has a very large number of extent maps, we can spend
a lot of time freeing them, which triggers a soft lockup warning.
Therefore reschedule if we need to when freeing the extent maps
while evicting the inode.

I could trigger this all the time by running xfstests/generic/299 on
a file system with the no-holes feature enabled. That test creates
an inode with 11386677 extent maps.

    $ mkfs.btrfs -f -O no-holes $TEST_DEV
    $ MKFS_OPTIONS="-O no-holes" ./check generic/299
    generic/299 382s ...
    Message from syslogd@debian-vm3 at Aug  7 10:44:29 ...
     kernel:[85304.208017] BUG: soft lockup - CPU#0 stuck for 22s! [umount:25330]
     384s
    Ran: generic/299
    Passed all 1 tests

    $ dmesg
    (...)
    [86304.300017] BUG: soft lockup - CPU#0 stuck for 23s! [umount:25330]
    (...)
    [86304.300036] Call Trace:
    [86304.300036]  [<ffffffff81698ba9>] __slab_free+0x54/0x295
    [86304.300036]  [<ffffffffa02ee9cc>] ? free_extent_map+0x5c/0xb0 [btrfs]
    [86304.300036]  [<ffffffff811a6cd2>] kmem_cache_free+0x282/0x2a0
    [86304.300036]  [<ffffffffa02ee9cc>] free_extent_map+0x5c/0xb0 [btrfs]
    [86304.300036]  [<ffffffffa02e3775>] btrfs_evict_inode+0xd5/0x660 [btrfs]
    [86304.300036]  [<ffffffff811e7c8d>] ? __inode_wait_for_writeback+0x6d/0xc0
    [86304.300036]  [<ffffffff816a389b>] ? _raw_spin_unlock+0x2b/0x40
    [86304.300036]  [<ffffffff811d8cbb>] evict+0xab/0x180
    [86304.300036]  [<ffffffff811d8dce>] dispose_list+0x3e/0x60
    [86304.300036]  [<ffffffff811d9b04>] evict_inodes+0xf4/0x110
    [86304.300036]  [<ffffffff811bd953>] generic_shutdown_super+0x53/0x110
    [86304.300036]  [<ffffffff811bdaa6>] kill_anon_super+0x16/0x30
    [86304.300036]  [<ffffffffa02a78ba>] btrfs_kill_super+0x1a/0xa0 [btrfs]
    [86304.300036]  [<ffffffff811bd3a9>] deactivate_locked_super+0x59/0x80
    [86304.300036]  [<ffffffff811be44e>] deactivate_super+0x4e/0x70
    [86304.300036]  [<ffffffff811dec14>] mntput_no_expire+0x174/0x1f0
    [86304.300036]  [<ffffffff811deab7>] ? mntput_no_expire+0x17/0x1f0
    [86304.300036]  [<ffffffff811e0517>] SyS_umount+0x97/0x100
    (...)

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/inode.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 8ad3ea9..00b4bd3 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -4718,6 +4718,11 @@ static void evict_inode_truncate_pages(struct inode *inode)
 		clear_bit(EXTENT_FLAG_LOGGING, &em->flags);
 		remove_extent_mapping(map_tree, em);
 		free_extent_map(em);
+		if (need_resched()) {
+			write_unlock(&map_tree->lock);
+			cond_resched();
+			write_lock(&map_tree->lock);
+		}
 	}
 	write_unlock(&map_tree->lock);
 
@@ -4740,6 +4745,7 @@ static void evict_inode_truncate_pages(struct inode *inode)
 				 &cached_state, GFP_NOFS);
 		free_extent_state(state);
 
+		cond_resched();
 		spin_lock(&io_tree->lock);
 	}
 	spin_unlock(&io_tree->lock);
-- 
1.9.1

[PATCH] Btrfs: clone, don't create invalid hole extent map

[Filipe Manana]

When cloning a file that consists of an inline extent, we were creating
an extent map that represents a non-existing trailing hole starting at a
file offset that isn't a multiple of the sector size. This happened because
when processing an inline extent we weren't aligning the extent's length to
the sector size, and therefore incorrectly treating the range
[inline_extent_length; sector_size[ as a hole.

Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/ioctl.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index d490abd..6e3a0d1 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -3494,7 +3494,8 @@ process_slot:
 			btrfs_mark_buffer_dirty(leaf);
 			btrfs_release_path(path);
 
-			last_dest_end = new_key.offset + datal;
+			last_dest_end = ALIGN(new_key.offset + datal,
+					      root->sectorsize);
 			ret = clone_finish_inode_update(trans, inode,
 							last_dest_end,
 							destoff, olen);
-- 
1.9.1

Re: [PATCH] Btrfs: don't monopolize a core when evicting inode

[Satoru Takeuchi]

(2014/08/08 10:47), Filipe Manana wrote:
> If an inode has a very large number of extent maps, we can spend
> a lot of time freeing them, which triggers a soft lockup warning.
> Therefore reschedule if we need to when freeing the extent maps
> while evicting the inode.
> 
> I could trigger this all the time by running xfstests/generic/299 on
> a file system with the no-holes feature enabled. That test creates
> an inode with 11386677 extent maps.
> 
>      $ mkfs.btrfs -f -O no-holes $TEST_DEV
>      $ MKFS_OPTIONS="-O no-holes" ./check generic/299
>      generic/299 382s ...
>      Message from syslogd@debian-vm3 at Aug  7 10:44:29 ...
>       kernel:[85304.208017] BUG: soft lockup - CPU#0 stuck for 22s! [umount:25330]
>       384s
>      Ran: generic/299
>      Passed all 1 tests
> 
>      $ dmesg
>      (...)
>      [86304.300017] BUG: soft lockup - CPU#0 stuck for 23s! [umount:25330]
>      (...)
>      [86304.300036] Call Trace:
>      [86304.300036]  [<ffffffff81698ba9>] __slab_free+0x54/0x295
>      [86304.300036]  [<ffffffffa02ee9cc>] ? free_extent_map+0x5c/0xb0 [btrfs]
>      [86304.300036]  [<ffffffff811a6cd2>] kmem_cache_free+0x282/0x2a0
>      [86304.300036]  [<ffffffffa02ee9cc>] free_extent_map+0x5c/0xb0 [btrfs]
>      [86304.300036]  [<ffffffffa02e3775>] btrfs_evict_inode+0xd5/0x660 [btrfs]
>      [86304.300036]  [<ffffffff811e7c8d>] ? __inode_wait_for_writeback+0x6d/0xc0
>      [86304.300036]  [<ffffffff816a389b>] ? _raw_spin_unlock+0x2b/0x40
>      [86304.300036]  [<ffffffff811d8cbb>] evict+0xab/0x180
>      [86304.300036]  [<ffffffff811d8dce>] dispose_list+0x3e/0x60
>      [86304.300036]  [<ffffffff811d9b04>] evict_inodes+0xf4/0x110
>      [86304.300036]  [<ffffffff811bd953>] generic_shutdown_super+0x53/0x110
>      [86304.300036]  [<ffffffff811bdaa6>] kill_anon_super+0x16/0x30
>      [86304.300036]  [<ffffffffa02a78ba>] btrfs_kill_super+0x1a/0xa0 [btrfs]
>      [86304.300036]  [<ffffffff811bd3a9>] deactivate_locked_super+0x59/0x80
>      [86304.300036]  [<ffffffff811be44e>] deactivate_super+0x4e/0x70
>      [86304.300036]  [<ffffffff811dec14>] mntput_no_expire+0x174/0x1f0
>      [86304.300036]  [<ffffffff811deab7>] ? mntput_no_expire+0x17/0x1f0
>      [86304.300036]  [<ffffffff811e0517>] SyS_umount+0x97/0x100
>      (...)
> 
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

Reviewed-by: Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com>
Tested-by: Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com>

> ---
>   fs/btrfs/inode.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 8ad3ea9..00b4bd3 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -4718,6 +4718,11 @@ static void evict_inode_truncate_pages(struct inode *inode)
>   		clear_bit(EXTENT_FLAG_LOGGING, &em->flags);
>   		remove_extent_mapping(map_tree, em);
>   		free_extent_map(em);
> +		if (need_resched()) {
> +			write_unlock(&map_tree->lock);
> +			cond_resched();
> +			write_lock(&map_tree->lock);
> +		}
>   	}
>   	write_unlock(&map_tree->lock);
>   
> @@ -4740,6 +4745,7 @@ static void evict_inode_truncate_pages(struct inode *inode)
>   				 &cached_state, GFP_NOFS);
>   		free_extent_state(state);
>   
> +		cond_resched();
>   		spin_lock(&io_tree->lock);
>   	}
>   	spin_unlock(&io_tree->lock);
> 

Re: [PATCH] Btrfs: clone, don't create invalid hole extent map

[Satoru Takeuchi]

(2014/08/08 10:47), Filipe Manana wrote:
> When cloning a file that consists of an inline extent, we were creating
> an extent map that represents a non-existing trailing hole starting at a
> file offset that isn't a multiple of the sector size. This happened because
> when processing an inline extent we weren't aligning the extent's length to
> the sector size, and therefore incorrectly treating the range
> [inline_extent_length; sector_size[ as a hole.
> 
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

Reviewed-by: Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com>

> ---
>   fs/btrfs/ioctl.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index d490abd..6e3a0d1 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -3494,7 +3494,8 @@ process_slot:
>   			btrfs_mark_buffer_dirty(leaf);
>   			btrfs_release_path(path);
>   
> -			last_dest_end = new_key.offset + datal;
> +			last_dest_end = ALIGN(new_key.offset + datal,
> +					      root->sectorsize);
>   			ret = clone_finish_inode_update(trans, inode,
>   							last_dest_end,
>   							destoff, olen);
>