Re: [PATCH] Btrfs: fix handling of faults from btrfs_copy_from_user

2016-05-19 Thread Adam Borowski
On Thu, May 19, 2016 at 04:10:20PM -0400, Chris Mason wrote:
> On Tue, May 17, 2016 at 10:47:27PM +0200, Adam Borowski wrote:
> > On Tue, May 17, 2016 at 12:23:44PM -0400, Chris Mason wrote:
> > > On Tue, May 17, 2016 at 05:14:51PM +0200, Adam Borowski wrote:
> > > > On Mon, May 16, 2016 at 05:06:55PM -0400, Chris Mason wrote:
> > > > > And now for the patch:
> > > > [...]
> > > > 
> > > > I then tried your test case, and alas:
> > > > 
> > > > Here's another run on an untainted kernel built with frame pointers etc:
> > > > 
> > > > ./dammitdave foo
> > > > [  236.500280] WARNING: CPU: 3 PID: 2940 at fs/btrfs/extent-tree.c:4233 
> > > > btrfs_free_reserved_data_space_noquota+0xdd/0x160
> > > > rm foo
> > > > [  323.851851] BTRFS info (device sda1): btrfs_destroy_inode: leftover 
> > > > csum_bytes
> > > 
> > > Hmmm, some of your traces mentioned compression, do you have compression
> > > enabled?
> > 
> > Yeah, I mount with noatime,compress=lzo.
> > 
> > > I'll try to reproduce here, but could you try the same test on v4.5?
> > 
> > I've ran it for half an hour on vanilla 4.5.4 without any patches, no
> > failures of any kind.
>  >
> > > gdb> list *__btrfs_buffered_write+0x748
> > 
> > 0x8152eb78 is in __btrfs_buffered_write (fs/btrfs/file.c:1564).
> 
> Hmpf, even with forcing btrfs_delalloc_reserve_metadata to randomly
> fail, I can't trigger this warning.  Something else is going on.
> 
> For now I'm going to send in the fault patch, I'm confident this new
> warning is something unrelated.

After having balanced my sda1, I can't reproduce this anymore, even after
having re-allocated all chunks.

As the original bug produced different results (like, no warnings other than
btrfs_destroy_inode:csum_bytes), it indeed looks like an unrelated
regression of some kind.

I re-checked both real loads and dammitdave yesterday:
* 4.5.4 works ok
* 4.6.0 without your patch warns btrfs_destroy_inode
* 4.6.0 with the patch works ok
thus, it at least fixes that one.

Tested-by: Adam Borowski 

-- 
An imaginary friend squared is a real enemy.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Btrfs: fix handling of faults from btrfs_copy_from_user

2016-05-19 Thread Chris Mason
On Tue, May 17, 2016 at 10:47:27PM +0200, Adam Borowski wrote:
> On Tue, May 17, 2016 at 12:23:44PM -0400, Chris Mason wrote:
> > On Tue, May 17, 2016 at 05:14:51PM +0200, Adam Borowski wrote:
> > > On Mon, May 16, 2016 at 05:06:55PM -0400, Chris Mason wrote:
> > > > And now for the patch:
> > > [...]
> > > 
> > > I then tried your test case, and alas:
> > > 
> > > Here's another run on an untainted kernel built with frame pointers etc:
> > > 
> > > ./dammitdave foo
> > > [  236.500257] [ cut here ]
> > > [  236.500280] WARNING: CPU: 3 PID: 2940 at fs/btrfs/extent-tree.c:4233 
> > > btrfs_free_reserved_data_space_noquota+0xdd/0x160
> > > [  236.500285] Modules linked in:
> > > [  236.500295] CPU: 3 PID: 2940 Comm: dammit Not tainted 4.6.0debug+ #1
> > > [  236.500301] Hardware name: System manufacturer System Product 
> > > Name/M4A77T, BIOS 240105/18/2011
> > > [  236.500306]   42ec2fb0 88022d5ffbd8 
> > > 816b1920
> > > [  236.500315]  81f86e3f 42ec2fb0  
> > > 
> > > [  236.500322]  88022d5ffc20 81118c2c 88022dfd4000 
> > > 1089
> > > [  236.500330] Call Trace:
> > > [  236.500342]  [] dump_stack+0x60/0xa0
> > > [  236.500352]  [] __warn+0x10c/0x150
> > > [  236.500360]  [] warn_slowpath_null+0x18/0x20
> > > [  236.500368]  [] 
> > > btrfs_free_reserved_data_space_noquota+0xdd/0x160
> > > [  236.500376]  [] 
> > > btrfs_free_reserved_data_space+0x22/0x40
> > > [  236.500385]  [] __btrfs_buffered_write+0x748/0xa20
> > > [  236.500394]  [] ? 
> > > security_inode_need_killpriv+0x3c/0x60
> > > [  236.500401]  [] btrfs_file_write_iter+0x4ff/0xb90
> > > [  236.500410]  [] __vfs_write+0x117/0x1d0
> > > [  236.500417]  [] vfs_write+0xdd/0x290
> > > [  236.500425]  [] ? __fget_light+0x4d/0x120
> > > [  236.500432]  [] SyS_pwrite64+0x9e/0xc0
> > > [  236.500441]  [] entry_SYSCALL_64_fastpath+0x17/0x93
> > > [  236.500446] ---[ end trace 7df747a6a0962ae6 ]---
> > > rm foo
> > > [  323.851851] BTRFS info (device sda1): btrfs_destroy_inode: leftover 
> > > csum_bytes
> > 
> > Hmmm, some of your traces mentioned compression, do you have compression
> > enabled?
> 
> Yeah, I mount with noatime,compress=lzo.
> 
> > I'll try to reproduce here, but could you try the same test on v4.5?
> 
> I've ran it for half an hour on vanilla 4.5.4 without any patches, no
> failures of any kind.
> 
> > Also, if you can gdb your vmlinux (or btrfs.ko) and find this line:
> > 
> > gdb> list *__btrfs_buffered_write+0x748
> 
> 0x8152eb78 is in __btrfs_buffered_write (fs/btrfs/file.c:1564).
> 1559  
> 1560  reserve_metadata:
> 1561  ret = btrfs_delalloc_reserve_metadata(inode, 
> reserve_bytes);
> 1562  if (ret) {
> 1563  if (!only_release_metadata)
> 1564  btrfs_free_reserved_data_space(inode, 
> pos,
> 1565 
> write_bytes);
> 1566  else
> 1567  btrfs_end_write_no_snapshoting(root);
> 1568  break;
> 

Hmpf, even with forcing btrfs_delalloc_reserve_metadata to randomly
fail, I can't trigger this warning.  Something else is going on.

For now I'm going to send in the fault patch, I'm confident this new
warning is something unrelated.

-chris

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Btrfs: fix handling of faults from btrfs_copy_from_user

2016-05-17 Thread Chris Mason
On Tue, May 17, 2016 at 11:57:07PM +0200, Adam Borowski wrote:
> On Tue, May 17, 2016 at 05:00:49PM -0400, Chris Mason wrote:
> > On Tue, May 17, 2016 at 10:47:27PM +0200, Adam Borowski wrote:
> > > > Hmmm, some of your traces mentioned compression, do you have compression
> > > > enabled?
> > > 
> > > Yeah, I mount with noatime,compress=lzo.
> > > 
> > > > Also, if you can gdb your vmlinux (or btrfs.ko) and find this line:
> > > > 
> > > > gdb> list *__btrfs_buffered_write+0x748
> > > 
> > > 0x8152eb78 is in __btrfs_buffered_write (fs/btrfs/file.c:1564).
> > > 1559  
> > > 1560  reserve_metadata:
> > > 1561  ret = btrfs_delalloc_reserve_metadata(inode, 
> > > reserve_bytes);
> > > 1562  if (ret) {
> > > 1563  if (!only_release_metadata)
> > > 1564  
> > > btrfs_free_reserved_data_space(inode, pos,
> > > 1565 
> > > write_bytes);
> > > 1566  else
> > > 1567  
> > > btrfs_end_write_no_snapshoting(root);
> > > 1568  break;
> > > 
> > 
> > Aha, so you're getting an enospc with compression on and the accounting
> > doesn't line up.  I should be able to trigger this.
> 
> This partition was fully balanced just a few days ago so I didn't even think
> to look at df.
> 
> [~]# df -h
> /dev/sda1   216G  161G   53G  76% /
> 
> [~]# btrfs files df /
> Data, single: total=209.12GiB, used=156.46GiB
> System, single: total=32.00MiB, used=48.00KiB
> Metadata, single: total=6.00GiB, used=3.51GiB
> GlobalReserve, single: total=512.00MiB, used=0.00B
> 
> However, after partially balancing it a bit:
> 
> [~]# btrfs files df /
> Data, single: total=172.00GiB, used=156.03GiB
> System, single: total=32.00MiB, used=48.00KiB
> Metadata, single: total=5.00GiB, used=3.50GiB
> GlobalReserve, single: total=512.00MiB, used=0.00B
> 
> I can't reproduce the failure anymore.

Yeah, I'm not sure yet if this is related to the page faulting problem,
but Dave Jones triggered this error back in v4.4.  I'll still fix it, it
should be an alignment issue with write_bytes.

-chris

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Btrfs: fix handling of faults from btrfs_copy_from_user

2016-05-17 Thread Adam Borowski
On Tue, May 17, 2016 at 05:00:49PM -0400, Chris Mason wrote:
> On Tue, May 17, 2016 at 10:47:27PM +0200, Adam Borowski wrote:
> > > Hmmm, some of your traces mentioned compression, do you have compression
> > > enabled?
> > 
> > Yeah, I mount with noatime,compress=lzo.
> > 
> > > Also, if you can gdb your vmlinux (or btrfs.ko) and find this line:
> > > 
> > > gdb> list *__btrfs_buffered_write+0x748
> > 
> > 0x8152eb78 is in __btrfs_buffered_write (fs/btrfs/file.c:1564).
> > 1559
> > 1560reserve_metadata:
> > 1561ret = btrfs_delalloc_reserve_metadata(inode, 
> > reserve_bytes);
> > 1562if (ret) {
> > 1563if (!only_release_metadata)
> > 1564
> > btrfs_free_reserved_data_space(inode, pos,
> > 1565   
> > write_bytes);
> > 1566else
> > 1567
> > btrfs_end_write_no_snapshoting(root);
> > 1568break;
> > 
> 
> Aha, so you're getting an enospc with compression on and the accounting
> doesn't line up.  I should be able to trigger this.

This partition was fully balanced just a few days ago so I didn't even think
to look at df.

[~]# df -h
/dev/sda1   216G  161G   53G  76% /

[~]# btrfs files df /
Data, single: total=209.12GiB, used=156.46GiB
System, single: total=32.00MiB, used=48.00KiB
Metadata, single: total=6.00GiB, used=3.51GiB
GlobalReserve, single: total=512.00MiB, used=0.00B

However, after partially balancing it a bit:

[~]# btrfs files df /
Data, single: total=172.00GiB, used=156.03GiB
System, single: total=32.00MiB, used=48.00KiB
Metadata, single: total=5.00GiB, used=3.50GiB
GlobalReserve, single: total=512.00MiB, used=0.00B

I can't reproduce the failure anymore.

-- 
A tit a day keeps the vet away.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Btrfs: fix handling of faults from btrfs_copy_from_user

2016-05-17 Thread Chris Mason
On Tue, May 17, 2016 at 10:47:27PM +0200, Adam Borowski wrote:
> On Tue, May 17, 2016 at 12:23:44PM -0400, Chris Mason wrote:
> > On Tue, May 17, 2016 at 05:14:51PM +0200, Adam Borowski wrote:
> > > On Mon, May 16, 2016 at 05:06:55PM -0400, Chris Mason wrote:
> > > > And now for the patch:
> > > [...]
> > > 
> > > I then tried your test case, and alas:
> > > 
> > > Here's another run on an untainted kernel built with frame pointers etc:
> > > 
> > > ./dammitdave foo
> > > [  236.500257] [ cut here ]
> > > [  236.500280] WARNING: CPU: 3 PID: 2940 at fs/btrfs/extent-tree.c:4233 
> > > btrfs_free_reserved_data_space_noquota+0xdd/0x160
> > > [  236.500285] Modules linked in:
> > > [  236.500295] CPU: 3 PID: 2940 Comm: dammit Not tainted 4.6.0debug+ #1
> > > [  236.500301] Hardware name: System manufacturer System Product 
> > > Name/M4A77T, BIOS 240105/18/2011
> > > [  236.500306]   42ec2fb0 88022d5ffbd8 
> > > 816b1920
> > > [  236.500315]  81f86e3f 42ec2fb0  
> > > 
> > > [  236.500322]  88022d5ffc20 81118c2c 88022dfd4000 
> > > 1089
> > > [  236.500330] Call Trace:
> > > [  236.500342]  [] dump_stack+0x60/0xa0
> > > [  236.500352]  [] __warn+0x10c/0x150
> > > [  236.500360]  [] warn_slowpath_null+0x18/0x20
> > > [  236.500368]  [] 
> > > btrfs_free_reserved_data_space_noquota+0xdd/0x160
> > > [  236.500376]  [] 
> > > btrfs_free_reserved_data_space+0x22/0x40
> > > [  236.500385]  [] __btrfs_buffered_write+0x748/0xa20
> > > [  236.500394]  [] ? 
> > > security_inode_need_killpriv+0x3c/0x60
> > > [  236.500401]  [] btrfs_file_write_iter+0x4ff/0xb90
> > > [  236.500410]  [] __vfs_write+0x117/0x1d0
> > > [  236.500417]  [] vfs_write+0xdd/0x290
> > > [  236.500425]  [] ? __fget_light+0x4d/0x120
> > > [  236.500432]  [] SyS_pwrite64+0x9e/0xc0
> > > [  236.500441]  [] entry_SYSCALL_64_fastpath+0x17/0x93
> > > [  236.500446] ---[ end trace 7df747a6a0962ae6 ]---
> > > rm foo
> > > [  323.851851] BTRFS info (device sda1): btrfs_destroy_inode: leftover 
> > > csum_bytes
> > 
> > Hmmm, some of your traces mentioned compression, do you have compression
> > enabled?
> 
> Yeah, I mount with noatime,compress=lzo.
> 
> > I'll try to reproduce here, but could you try the same test on v4.5?
> 
> I've ran it for half an hour on vanilla 4.5.4 without any patches, no
> failures of any kind.
> 
> > Also, if you can gdb your vmlinux (or btrfs.ko) and find this line:
> > 
> > gdb> list *__btrfs_buffered_write+0x748
> 
> 0x8152eb78 is in __btrfs_buffered_write (fs/btrfs/file.c:1564).
> 1559  
> 1560  reserve_metadata:
> 1561  ret = btrfs_delalloc_reserve_metadata(inode, 
> reserve_bytes);
> 1562  if (ret) {
> 1563  if (!only_release_metadata)
> 1564  btrfs_free_reserved_data_space(inode, 
> pos,
> 1565 
> write_bytes);
> 1566  else
> 1567  btrfs_end_write_no_snapshoting(root);
> 1568  break;
> 

Aha, so you're getting an enospc with compression on and the accounting
doesn't line up.  I should be able to trigger this.

-chris

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Btrfs: fix handling of faults from btrfs_copy_from_user

2016-05-17 Thread Adam Borowski
On Tue, May 17, 2016 at 12:23:44PM -0400, Chris Mason wrote:
> On Tue, May 17, 2016 at 05:14:51PM +0200, Adam Borowski wrote:
> > On Mon, May 16, 2016 at 05:06:55PM -0400, Chris Mason wrote:
> > > And now for the patch:
> > [...]
> > 
> > I then tried your test case, and alas:
> > 
> > Here's another run on an untainted kernel built with frame pointers etc:
> > 
> > ./dammitdave foo
> > [  236.500257] [ cut here ]
> > [  236.500280] WARNING: CPU: 3 PID: 2940 at fs/btrfs/extent-tree.c:4233 
> > btrfs_free_reserved_data_space_noquota+0xdd/0x160
> > [  236.500285] Modules linked in:
> > [  236.500295] CPU: 3 PID: 2940 Comm: dammit Not tainted 4.6.0debug+ #1
> > [  236.500301] Hardware name: System manufacturer System Product 
> > Name/M4A77T, BIOS 240105/18/2011
> > [  236.500306]   42ec2fb0 88022d5ffbd8 
> > 816b1920
> > [  236.500315]  81f86e3f 42ec2fb0  
> > 
> > [  236.500322]  88022d5ffc20 81118c2c 88022dfd4000 
> > 1089
> > [  236.500330] Call Trace:
> > [  236.500342]  [] dump_stack+0x60/0xa0
> > [  236.500352]  [] __warn+0x10c/0x150
> > [  236.500360]  [] warn_slowpath_null+0x18/0x20
> > [  236.500368]  [] 
> > btrfs_free_reserved_data_space_noquota+0xdd/0x160
> > [  236.500376]  [] 
> > btrfs_free_reserved_data_space+0x22/0x40
> > [  236.500385]  [] __btrfs_buffered_write+0x748/0xa20
> > [  236.500394]  [] ? 
> > security_inode_need_killpriv+0x3c/0x60
> > [  236.500401]  [] btrfs_file_write_iter+0x4ff/0xb90
> > [  236.500410]  [] __vfs_write+0x117/0x1d0
> > [  236.500417]  [] vfs_write+0xdd/0x290
> > [  236.500425]  [] ? __fget_light+0x4d/0x120
> > [  236.500432]  [] SyS_pwrite64+0x9e/0xc0
> > [  236.500441]  [] entry_SYSCALL_64_fastpath+0x17/0x93
> > [  236.500446] ---[ end trace 7df747a6a0962ae6 ]---
> > rm foo
> > [  323.851851] BTRFS info (device sda1): btrfs_destroy_inode: leftover 
> > csum_bytes
> 
> Hmmm, some of your traces mentioned compression, do you have compression
> enabled?

Yeah, I mount with noatime,compress=lzo.

> I'll try to reproduce here, but could you try the same test on v4.5?

I've ran it for half an hour on vanilla 4.5.4 without any patches, no
failures of any kind.

> Also, if you can gdb your vmlinux (or btrfs.ko) and find this line:
> 
> gdb> list *__btrfs_buffered_write+0x748

0x8152eb78 is in __btrfs_buffered_write (fs/btrfs/file.c:1564).
1559
1560reserve_metadata:
1561ret = btrfs_delalloc_reserve_metadata(inode, 
reserve_bytes);
1562if (ret) {
1563if (!only_release_metadata)
1564btrfs_free_reserved_data_space(inode, 
pos,
1565   
write_bytes);
1566else
1567btrfs_end_write_no_snapshoting(root);
1568break;


Meow!
-- 
A tit a day keeps the vet away.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Btrfs: fix handling of faults from btrfs_copy_from_user

2016-05-17 Thread Chris Mason
On Tue, May 17, 2016 at 05:14:51PM +0200, Adam Borowski wrote:
> On Mon, May 16, 2016 at 05:06:55PM -0400, Chris Mason wrote:
> > Hi everyone,
> > 
> > I've tried to cc most of the recent reports of this warning.  It was
> > pretty hard to track down, but I've got a test program for it now.  My
> > goal is to change xfs_io to add the madvise loop under
> > --do-horrible-things, instead of adding yet another special doio.c
> > function to xfstests.
> [...]
> > And now for the patch:
> [...]
> 
> 
> I've applied the patch and hammered it with real workloads (mostly sbuild).
> They used to trigger the warning nearly immediately, with the patch all
> seemed ok -- so there is an improvement.

Thanks for trying these out.

> 
> I then tried your test case, and alas:
> 
> Here's another run on an untainted kernel built with frame pointers etc:
> 
> ./dammitdave foo
> [  236.500257] [ cut here ]
> [  236.500280] WARNING: CPU: 3 PID: 2940 at fs/btrfs/extent-tree.c:4233 
> btrfs_free_reserved_data_space_noquota+0xdd/0x160
> [  236.500285] Modules linked in:
> [  236.500295] CPU: 3 PID: 2940 Comm: dammit Not tainted 4.6.0debug+ #1
> [  236.500301] Hardware name: System manufacturer System Product Name/M4A77T, 
> BIOS 240105/18/2011
> [  236.500306]   42ec2fb0 88022d5ffbd8 
> 816b1920
> [  236.500315]  81f86e3f 42ec2fb0  
> 
> [  236.500322]  88022d5ffc20 81118c2c 88022dfd4000 
> 1089
> [  236.500330] Call Trace:
> [  236.500342]  [] dump_stack+0x60/0xa0
> [  236.500352]  [] __warn+0x10c/0x150
> [  236.500360]  [] warn_slowpath_null+0x18/0x20
> [  236.500368]  [] 
> btrfs_free_reserved_data_space_noquota+0xdd/0x160
> [  236.500376]  [] btrfs_free_reserved_data_space+0x22/0x40
> [  236.500385]  [] __btrfs_buffered_write+0x748/0xa20
> [  236.500394]  [] ? security_inode_need_killpriv+0x3c/0x60
> [  236.500401]  [] btrfs_file_write_iter+0x4ff/0xb90
> [  236.500410]  [] __vfs_write+0x117/0x1d0
> [  236.500417]  [] vfs_write+0xdd/0x290
> [  236.500425]  [] ? __fget_light+0x4d/0x120
> [  236.500432]  [] SyS_pwrite64+0x9e/0xc0
> [  236.500441]  [] entry_SYSCALL_64_fastpath+0x17/0x93
> [  236.500446] ---[ end trace 7df747a6a0962ae6 ]---
> rm foo
> [  323.851851] BTRFS info (device sda1): btrfs_destroy_inode: leftover 
> csum_bytes

Hmmm, some of your traces mentioned compression, do you have compression
enabled?  I'll try to reproduce here, but could you try the same test on
v4.5?

Also, if you can gdb your vmlinux (or btrfs.ko) and find this line:

gdb> list *__btrfs_buffered_write+0x748

It'll help.

Thanks!

-chris
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html