[PATCH] Btrfs: skip looking for delalloc if we don't have ->fill_delalloc

[PATCH] Btrfs: skip looking for delalloc if we don't have ->fill_delalloc

[Josef Bacik]

We always look for delalloc bytes in our io_tree so we can fill in delalloc.
This is fine in most cases, but if we're writing out the btree_inode this is
just a superfluous tree search on the io_tree, and if we have a lot of metadata
dirty this could be an expensive check.  So instead check to see if our io_tree
has a ->fill_delalloc op, and if not don't even bother doing the lookup.
Thanks,

Signed-off-by: Josef Bacik <josef@redhat.com>
---
 fs/btrfs/extent_io.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 067b174..e16dcbf 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -2180,6 +2180,7 @@ static int __extent_writepage(struct page *page, struct writeback_control *wbc,
 	int compressed;
 	int write_flags;
 	unsigned long nr_written = 0;
+	bool fill_delalloc = true;
 
 	if (wbc->sync_mode == WB_SYNC_ALL)
 		write_flags = WRITE_SYNC;
@@ -2210,10 +2211,13 @@ static int __extent_writepage(struct page *page, struct writeback_control *wbc,
 
 	set_page_extent_mapped(page);
 
+	if (!tree->ops || !tree->ops->fill_delalloc)
+		fill_delalloc = false;
+
 	delalloc_start = start;
 	delalloc_end = 0;
 	page_started = 0;
-	if (!epd->extent_locked) {
+	if (!epd->extent_locked && fill_delalloc) {
 		u64 delalloc_to_write = 0;
 		/*
 		 * make sure the wbc mapping index is at least updated
-- 
1.7.5.2

Re: [PATCH] Btrfs: skip looking for delalloc if we don't have ->fill_delalloc

[liubo]

On 08/02/2011 12:11 AM, Josef Bacik wrote:
> We always look for delalloc bytes in our io_tree so we can fill in delalloc.
> This is fine in most cases, but if we're writing out the btree_inode this is
> just a superfluous tree search on the io_tree, and if we have a lot of metadata
> dirty this could be an expensive check.  So instead check to see if our io_tree
> has a ->fill_delalloc op, and if not don't even bother doing the lookup.
> Thanks,
> 
> Signed-off-by: Josef Bacik <josef@redhat.com>
> ---

With the patch,

mkfs.btrfs /dev/sda15
mount /dev/sda15 /mnt/btrfs
dd if=/dev/zero of=/mnt/btrfs/tmp bs=1G

then it comes the following bug:

Btrfs loaded
device fsid 91d23288-d352-4346-979f-d6f93cac04a3 devid 1 transid 7 /dev/sda15
------------[ cut here ]------------
kernel BUG at fs/btrfs/inode.c:1583!
...
Call Trace:
 [<ffffffffa05b00d8>] worker_loop+0x138/0x510 [btrfs]
 [<ffffffffa05affa0>] ? btrfs_queue_worker+0x2d0/0x2d0 [btrfs]
 [<ffffffffa05affa0>] ? btrfs_queue_worker+0x2d0/0x2d0 [btrfs]
 [<ffffffff81074f06>] kthread+0x96/0xa0
 [<ffffffff81467bf4>] kernel_thread_helper+0x4/0x10
 [<ffffffff81074e70>] ? kthread_worker_fn+0x1a0/0x1a0
 [<ffffffff81467bf0>] ? gs_change+0xb/0xb
Code: e0 48 83 c4 28 5b 41 5c 41 5d 41 5e 41 5f c9 c3 48 8b 7d b8 48 8d 4d c8 41 b8 50 00 00 00 4c 89 fa 4c 89 e6 e8 19 cf 01 00 eb bd <0f> 0b eb fe 48 89 df e8 1b 48 b6 e0 eb 9d 66 0f 1f 84 00 00 00 
RIP  [<ffffffffa0587f59>] btrfs_writepage_fixup_worker+0x139/0x150 [btrfs]
 RSP <ffff88000887bdd0>
---[ end trace 5089b598ce74fcfc ]---

thanks,
liubo

Re: [PATCH] Btrfs: skip looking for delalloc if we don't have ->fill_delalloc

[liubo]

On 08/02/2011 09:32 AM, liubo wrote:
> On 08/02/2011 12:11 AM, Josef Bacik wrote:
>> We always look for delalloc bytes in our io_tree so we can fill in delalloc.
>> This is fine in most cases, but if we're writing out the btree_inode this is
>> just a superfluous tree search on the io_tree, and if we have a lot of metadata
>> dirty this could be an expensive check.  So instead check to see if our io_tree
>> has a ->fill_delalloc op, and if not don't even bother doing the lookup.
>> Thanks,
>>
>> Signed-off-by: Josef Bacik <josef@redhat.com>
>> ---
> 

sorry, I mixed the patch with others...

The patch is ok.

> With the patch,
> 
> mkfs.btrfs /dev/sda15
> mount /dev/sda15 /mnt/btrfs
> dd if=/dev/zero of=/mnt/btrfs/tmp bs=1G
> 
> then it comes the following bug:
> 
> Btrfs loaded
> device fsid 91d23288-d352-4346-979f-d6f93cac04a3 devid 1 transid 7 /dev/sda15
> ------------[ cut here ]------------
> kernel BUG at fs/btrfs/inode.c:1583!
> ...
> Call Trace:
>  [<ffffffffa05b00d8>] worker_loop+0x138/0x510 [btrfs]
>  [<ffffffffa05affa0>] ? btrfs_queue_worker+0x2d0/0x2d0 [btrfs]
>  [<ffffffffa05affa0>] ? btrfs_queue_worker+0x2d0/0x2d0 [btrfs]
>  [<ffffffff81074f06>] kthread+0x96/0xa0
>  [<ffffffff81467bf4>] kernel_thread_helper+0x4/0x10
>  [<ffffffff81074e70>] ? kthread_worker_fn+0x1a0/0x1a0
>  [<ffffffff81467bf0>] ? gs_change+0xb/0xb
> Code: e0 48 83 c4 28 5b 41 5c 41 5d 41 5e 41 5f c9 c3 48 8b 7d b8 48 8d 4d c8 41 b8 50 00 00 00 4c 89 fa 4c 89 e6 e8 19 cf 01 00 eb bd <0f> 0b eb fe 48 89 df e8 1b 48 b6 e0 eb 9d 66 0f 1f 84 00 00 00 
> RIP  [<ffffffffa0587f59>] btrfs_writepage_fixup_worker+0x139/0x150 [btrfs]
>  RSP <ffff88000887bdd0>
> ---[ end trace 5089b598ce74fcfc ]---
> 
> thanks,
> liubo
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH] Btrfs: skip looking for delalloc if we don't have ->fill_delalloc

[Josef Bacik]

On 08/01/2011 09:43 PM, liubo wrote:
> On 08/02/2011 09:32 AM, liubo wrote:
>> On 08/02/2011 12:11 AM, Josef Bacik wrote:
>>> We always look for delalloc bytes in our io_tree so we can fill in delalloc.
>>> This is fine in most cases, but if we're writing out the btree_inode this is
>>> just a superfluous tree search on the io_tree, and if we have a lot of metadata
>>> dirty this could be an expensive check.  So instead check to see if our io_tree
>>> has a ->fill_delalloc op, and if not don't even bother doing the lookup.
>>> Thanks,
>>>
>>> Signed-off-by: Josef Bacik <josef@redhat.com>
>>> ---
>>
> 
> sorry, I mixed the patch with others...
> 
> The patch is ok.
> 

Good because I was horribly confused for a moment :),

Josef