Re: [PATCH 0/5] Fix delalloc inodes leaking on btrfs unmount

2018-05-16 Thread David Sterba
On Tue, May 01, 2018 at 04:02:34PM +0200, David Sterba wrote:
> On Fri, Apr 27, 2018 at 12:21:49PM +0300, Nikolay Borisov wrote:
> > After investigating crashes on generic/176 it turned that the culprit in 
> > fact
> > is the random failure induced by generic/019. As it happens, if on unmount 
> > the 
> > filesystem is in BTRFS_FS_STATE_ERROR then btrfs_error_commit_super is 
> > called. 
> > This unveiled 2 bugs:
> >  1. btrfs_destroy_delalloc_inodes's implementation was completely bogus, 
> > since
> >  it only called btrfs_invalidate_inodes which only pruned dentries and 
> > didn't 
> >  do anything to free any inodes with pending delalloc bytes. Once this is 
> > fixed 
> >  with the use of invalide_inode_pages2 the second bug transpired. 
> >  2. The last call ot run_delayed_iputs is made before 
> > btrfs_cleanup_transaction
> >  is called. The latter in turn could queue up more delayed iputs resulting 
> > from 
> >  invalidates_inode_pages2. 
> > 
> > This series fixes the problem by first fixing btrfs_destroy_delalloc_inode 
> > to 
> > properly cleanup delalloc inodes and as a result cleans up the code a bit. 
> > 
> > I've given it a good bashing through xfstest (4 full xfstest cycles + 100 
> > iterations of generic/475 since it was hitting some early assertion 
> > failures,
> > which are fixed in the final version) so am pretty confident in the change. 
> 
> Thanks. I'll add it as topic branch to next, this needs some testing
> exposure. The plan is to push the core patches to some rc, possibly rc5.
> 
> Review of patch 3 is required.

The whole series is now in misc-next. I did not see another occurence of
the crash, so that was probably unrelated to this patchset but still
worth analyzing.

As there are going to be more patches in post-rc5, this patch is in the
pool.
--
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 0/5] Fix delalloc inodes leaking on btrfs unmount

2018-05-08 Thread David Sterba
On Tue, May 08, 2018 at 08:16:37AM +0300, Nikolay Borisov wrote:
> >> properly cleanup delalloc inodes and as a result cleans up the code a bit. 
> >>
> >> I've given it a good bashing through xfstest (4 full xfstest cycles + 100 
> >> iterations of generic/475 since it was hitting some early assertion 
> >> failures,
> >> which are fixed in the final version) so am pretty confident in the 
> >> change. 
> > 
> > One qemu testmachine complains.
> > 
> > The branch was ext/nikbor/delalloc-invalidate in my github repo. Other
> > tests seem "fine", unlikely to be related to this patchset.
> > 
> > The error here is a null pointer deref in end bio callback, which
> > matches a use-after-free scenario, so I think there's still something
> > left to fix.
> > 
> > generic/335 [22:34:50]
> 
> How easy is to repro this on this particular test? Like every other run
> or is it spurious?

It happened once, on first run of the testing setup, so I don't have
enough data to answer. I'll start another round and we'll see.
--
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 0/5] Fix delalloc inodes leaking on btrfs unmount

2018-05-07 Thread Nikolay Borisov


On  8.05.2018 01:58, David Sterba wrote:
> On Fri, Apr 27, 2018 at 12:21:49PM +0300, Nikolay Borisov wrote:
>> After investigating crashes on generic/176 it turned that the culprit in fact
>> is the random failure induced by generic/019. As it happens, if on unmount 
>> the 
>> filesystem is in BTRFS_FS_STATE_ERROR then btrfs_error_commit_super is 
>> called. 
>> This unveiled 2 bugs:
>>  1. btrfs_destroy_delalloc_inodes's implementation was completely bogus, 
>> since
>>  it only called btrfs_invalidate_inodes which only pruned dentries and 
>> didn't 
>>  do anything to free any inodes with pending delalloc bytes. Once this is 
>> fixed 
>>  with the use of invalide_inode_pages2 the second bug transpired. 
>>  2. The last call ot run_delayed_iputs is made before 
>> btrfs_cleanup_transaction
>>  is called. The latter in turn could queue up more delayed iputs resulting 
>> from 
>>  invalidates_inode_pages2. 
>>
>> This series fixes the problem by first fixing btrfs_destroy_delalloc_inode 
>> to 
>> properly cleanup delalloc inodes and as a result cleans up the code a bit. 
>>
>> I've given it a good bashing through xfstest (4 full xfstest cycles + 100 
>> iterations of generic/475 since it was hitting some early assertion failures,
>> which are fixed in the final version) so am pretty confident in the change. 
> 
> One qemu testmachine complains.
> 
> The branch was ext/nikbor/delalloc-invalidate in my github repo. Other
> tests seem "fine", unlikely to be related to this patchset.
> 
> The error here is a null pointer deref in end bio callback, which
> matches a use-after-free scenario, so I think there's still something
> left to fix.
> 
> generic/335 [22:34:50]

How easy is to repro this on this particular test? Like every other run
or is it spurious?

> [26281.970322] run fstests generic/335 at 2018-05-07 22:34:50
> [26282.440728] BUG: unable to handle kernel NULL pointer dereference 
> at
> [26282.445060] PGD 0 P4D 0
> [26282.446526] Oops:  [#1] PREEMPT SMP
> [26282.448562] Modules linked in: btrfs libcrc32c xor 
> zstd_decompresszstd_compress xxhash raid6_pq loop af_packet [last unloaded: 
> libcrc32c]
> [26282.454384] CPU: 2 PID: 30005 Comm: btrfs-endio-met Tainted: GW 
> 4.17.0-rc4-default+ #73
> [26282.457247] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),BIOS 
> 1.0.0-prebuilt.qemu-project.org 04/01/2014
> [26282.459386] RIP: 0010:__queue_work+0x189/0x3f0
> [26282.460342] RSP: 0018:8ea47fd03f00 EFLAGS: 00010046
> [26282.461506] RAX:  RBX:  
> RCX:
> [26282.463061] RDX: 8ea47fbda640 RSI: 7fff 
> RDI:8ea47fbda640
> [26282.464606] RBP: 000d R08:  
> R09:0001
> [26282.466169] R10: 1000 R11: 8ea469244000 
> R12:8ea40819c800
> [26282.467697] R13: 0002 R14: 0200 
> R15:8ea47fbda640
> [26282.469205] FS:  () 
> GS:8ea47fd0()knlGS:
> [26282.470971] CS:  0010 DS:  ES:  CR0: 80050033
> [26282.472173] CR2:  CR3: 42ed4000 
> CR4:06e0
> [26282.473674] Call Trace:
> [26282.474300]  
> [26282.474848]  queue_work_on+0x34/0x40
> [26282.475717]  btrfs_end_bio+0x71/0x110 [btrfs]
> [26282.476750]  blk_update_request+0x78/0x2d0
> [26282.477675]  blk_mq_end_request+0x18/0x70
> [26282.478599]  flush_smp_call_function_queue+0x6f/0xe0
> [26282.479690]  smp_call_function_single_interrupt+0x2c/0xe0
> [26282.480825]  call_function_single_interrupt+0xf/0x20
> [26282.481888]  
> [26282.482423] RIP: 0010:exit_shm+0x0/0x1c0
> [26282.483281] RSP: 0018:a9a1c4787ea0 EFLAGS: 0292 
> ORIG_RAX:ff04
> [26282.484944] RAX: b2e37960 RBX: 8ea47ccc1c00 
> RCX:
> [26282.486423] RDX: 8ea413270e40 RSI: 0282 
> RDI:8ea47ccc1c00
> [26282.487870] RBP:  R08: 8ea413297630 
> R09:
> [26282.489104] R10:  R11: 0256 
> R12:
> [26282.490306] R13: 8ea47ccc2301 R14: 0001 
> R15:8ea47ccc1c00
> [26282.491500]  do_exit+0x274/0xb00
> [26282.492198]  ? rescuer_thread+0x2be/0x310
> [26282.492990]  ? worker_thread+0x380/0x380
> [26282.493795]  kthread+0xe0/0x130
> [26282.494443]  ? kthread_create_worker_on_cpu+0x50/0x50
> [26282.495395]  ret_from_fork+0x1f/0x30
> [26282.499564] RIP: __queue_work+0x189/0x3f0 RSP: 8ea47fd03f00
> [26282.500614] CR2: 
> [26282.501248] ---[ end trace f7e701988bc2b82f ]---
> [26282.502141] Kernel panic - not syncing: Fatal exception in interrupt
> [26282.503419] Kernel Offset: 0x3100 from 0x8100
> (relocation range: 0x8000-0xbfff)
> [26282.505348] Rebooting in 90 seconds..
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org

Re: [PATCH 0/5] Fix delalloc inodes leaking on btrfs unmount

2018-05-07 Thread David Sterba
On Fri, Apr 27, 2018 at 12:21:49PM +0300, Nikolay Borisov wrote:
> After investigating crashes on generic/176 it turned that the culprit in fact
> is the random failure induced by generic/019. As it happens, if on unmount 
> the 
> filesystem is in BTRFS_FS_STATE_ERROR then btrfs_error_commit_super is 
> called. 
> This unveiled 2 bugs:
>  1. btrfs_destroy_delalloc_inodes's implementation was completely bogus, since
>  it only called btrfs_invalidate_inodes which only pruned dentries and didn't 
>  do anything to free any inodes with pending delalloc bytes. Once this is 
> fixed 
>  with the use of invalide_inode_pages2 the second bug transpired. 
>  2. The last call ot run_delayed_iputs is made before 
> btrfs_cleanup_transaction
>  is called. The latter in turn could queue up more delayed iputs resulting 
> from 
>  invalidates_inode_pages2. 
> 
> This series fixes the problem by first fixing btrfs_destroy_delalloc_inode to 
> properly cleanup delalloc inodes and as a result cleans up the code a bit. 
> 
> I've given it a good bashing through xfstest (4 full xfstest cycles + 100 
> iterations of generic/475 since it was hitting some early assertion failures,
> which are fixed in the final version) so am pretty confident in the change. 

One qemu testmachine complains.

The branch was ext/nikbor/delalloc-invalidate in my github repo. Other
tests seem "fine", unlikely to be related to this patchset.

The error here is a null pointer deref in end bio callback, which
matches a use-after-free scenario, so I think there's still something
left to fix.

generic/335 [22:34:50]
[26281.970322] run fstests generic/335 at 2018-05-07 22:34:50
[26282.440728] BUG: unable to handle kernel NULL pointer dereference 
at
[26282.445060] PGD 0 P4D 0
[26282.446526] Oops:  [#1] PREEMPT SMP
[26282.448562] Modules linked in: btrfs libcrc32c xor 
zstd_decompresszstd_compress xxhash raid6_pq loop af_packet [last unloaded: 
libcrc32c]
[26282.454384] CPU: 2 PID: 30005 Comm: btrfs-endio-met Tainted: GW 
4.17.0-rc4-default+ #73
[26282.457247] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),BIOS 
1.0.0-prebuilt.qemu-project.org 04/01/2014
[26282.459386] RIP: 0010:__queue_work+0x189/0x3f0
[26282.460342] RSP: 0018:8ea47fd03f00 EFLAGS: 00010046
[26282.461506] RAX:  RBX:  RCX:
[26282.463061] RDX: 8ea47fbda640 RSI: 7fff RDI:8ea47fbda640
[26282.464606] RBP: 000d R08:  R09:0001
[26282.466169] R10: 1000 R11: 8ea469244000 R12:8ea40819c800
[26282.467697] R13: 0002 R14: 0200 R15:8ea47fbda640
[26282.469205] FS:  () 
GS:8ea47fd0()knlGS:
[26282.470971] CS:  0010 DS:  ES:  CR0: 80050033
[26282.472173] CR2:  CR3: 42ed4000 CR4:06e0
[26282.473674] Call Trace:
[26282.474300]  
[26282.474848]  queue_work_on+0x34/0x40
[26282.475717]  btrfs_end_bio+0x71/0x110 [btrfs]
[26282.476750]  blk_update_request+0x78/0x2d0
[26282.477675]  blk_mq_end_request+0x18/0x70
[26282.478599]  flush_smp_call_function_queue+0x6f/0xe0
[26282.479690]  smp_call_function_single_interrupt+0x2c/0xe0
[26282.480825]  call_function_single_interrupt+0xf/0x20
[26282.481888]  
[26282.482423] RIP: 0010:exit_shm+0x0/0x1c0
[26282.483281] RSP: 0018:a9a1c4787ea0 EFLAGS: 0292 
ORIG_RAX:ff04
[26282.484944] RAX: b2e37960 RBX: 8ea47ccc1c00 RCX:
[26282.486423] RDX: 8ea413270e40 RSI: 0282 RDI:8ea47ccc1c00
[26282.487870] RBP:  R08: 8ea413297630 R09:
[26282.489104] R10:  R11: 0256 R12:
[26282.490306] R13: 8ea47ccc2301 R14: 0001 R15:8ea47ccc1c00
[26282.491500]  do_exit+0x274/0xb00
[26282.492198]  ? rescuer_thread+0x2be/0x310
[26282.492990]  ? worker_thread+0x380/0x380
[26282.493795]  kthread+0xe0/0x130
[26282.494443]  ? kthread_create_worker_on_cpu+0x50/0x50
[26282.495395]  ret_from_fork+0x1f/0x30
[26282.499564] RIP: __queue_work+0x189/0x3f0 RSP: 8ea47fd03f00
[26282.500614] CR2: 
[26282.501248] ---[ end trace f7e701988bc2b82f ]---
[26282.502141] Kernel panic - not syncing: Fatal exception in interrupt
[26282.503419] Kernel Offset: 0x3100 from 0x8100
(relocation range: 0x8000-0xbfff)
[26282.505348] Rebooting in 90 seconds..
--
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 0/5] Fix delalloc inodes leaking on btrfs unmount

2018-05-01 Thread David Sterba
On Fri, Apr 27, 2018 at 12:21:49PM +0300, Nikolay Borisov wrote:
> After investigating crashes on generic/176 it turned that the culprit in fact
> is the random failure induced by generic/019. As it happens, if on unmount 
> the 
> filesystem is in BTRFS_FS_STATE_ERROR then btrfs_error_commit_super is 
> called. 
> This unveiled 2 bugs:
>  1. btrfs_destroy_delalloc_inodes's implementation was completely bogus, since
>  it only called btrfs_invalidate_inodes which only pruned dentries and didn't 
>  do anything to free any inodes with pending delalloc bytes. Once this is 
> fixed 
>  with the use of invalide_inode_pages2 the second bug transpired. 
>  2. The last call ot run_delayed_iputs is made before 
> btrfs_cleanup_transaction
>  is called. The latter in turn could queue up more delayed iputs resulting 
> from 
>  invalidates_inode_pages2. 
> 
> This series fixes the problem by first fixing btrfs_destroy_delalloc_inode to 
> properly cleanup delalloc inodes and as a result cleans up the code a bit. 
> 
> I've given it a good bashing through xfstest (4 full xfstest cycles + 100 
> iterations of generic/475 since it was hitting some early assertion failures,
> which are fixed in the final version) so am pretty confident in the change. 

Thanks. I'll add it as topic branch to next, this needs some testing
exposure. The plan is to push the core patches to some rc, possibly rc5.

Review of patch 3 is required.
--
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