Re: [PATCH] btrfs: fix outstanding extents calculation

[Filipe Manana <fdmanana@kernel.org>]

On Mon, Mar 28, 2022 at 02:08:30PM +0100, Filipe Manana wrote:
> On Mon, Mar 28, 2022 at 09:32:05PM +0900, Naohiro Aota wrote:
> > Running generic/406 causes the following WARNING in btrfs_destroy_inode()
> > which tells there is outstanding extents left.
> 
> I can't trigger the warning with generic/406.
> Any special setup or config to trigger it?
> 
> The change looks fine to me, however I'm curious why this isn't triggered
> with generic/406, nor anyone else reported it before, since the test is
> fully deterministic.

Also the subject "btrfs: fix outstanding extents calculation" is confusing,
as the problem is not in the outstanding extents calculation code, but
rather not releasing delalloc by the correct amount in a specific code path.

So something like "btrfs: release correct delalloc amount in direct IO write path"
would be more correct and specific.

> 
> Thanks.
> 
> > 
> > In btrfs_get_blocks_direct_write(), we reserve a temporary outstanding
> > extents with btrfs_delalloc_reserve_metadata() (or indirectly from
> > btrfs_delalloc_reserve_space(()). We then release the outstanding extents
> > with btrfs_delalloc_release_extents(). However, the "len" can be modified
> > in the CoW case, which releasing fewer outstanding extents than expected.
> > 
> > Fix it by calling btrfs_delalloc_release_extents() for the original length.
> > 
> >     ------------[ cut here ]------------
> >     WARNING: CPU: 0 PID: 757 at fs/btrfs/inode.c:8848 btrfs_destroy_inode+0x1e6/0x210 [btrfs]
> >     Modules linked in: btrfs blake2b_generic xor lzo_compress
> >     lzo_decompress raid6_pq zstd zstd_decompress zstd_compress xxhash zram
> >     zsmalloc
> >     CPU: 0 PID: 757 Comm: umount Not tainted 5.17.0-rc8+ #101
> >     Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS d55cb5a 04/01/2014
> >     RIP: 0010:btrfs_destroy_inode+0x1e6/0x210 [btrfs]
> >     Code: fe ff ff 0f 0b e9 d6 fe ff ff 0f 0b 48 83 bb e0 01 00 00 00 0f 84
> >     65 fe ff ff 0f 0b 48 83 bb 78 ff ff ff 00 0f 84 63 fe ff ff <0f> 0b 48
> >     83 bb 70 ff ff ff 00 0f 84 61 fe ff ff 0f 0b e9 5a fe ff
> >     RSP: 0018:ffffc9000327bda8 EFLAGS: 00010206
> >     RAX: 0000000000000000 RBX: ffff888100548b78 RCX: 0000000000000000
> >     RDX: 0000000000026900 RSI: 0000000000000000 RDI: ffff888100548b78
> >     RBP: ffff888100548940 R08: 0000000000000000 R09: ffff88810b48aba8
> >     R10: 0000000000000001 R11: ffff8881004eb240 R12: ffff88810b48a800
> >     R13: ffff88810b48ec08 R14: ffff88810b48ed00 R15: ffff888100490c68
> >     FS:  00007f8549ea0b80(0000) GS:ffff888237c00000(0000) knlGS:0000000000000000
> >     CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> >     CR2: 00007f854a09e733 CR3: 000000010a2e9003 CR4: 0000000000370eb0
> >     DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> >     DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> >     Call Trace:
> >      <TASK>
> >      destroy_inode+0x33/0x70
> >      dispose_list+0x43/0x60
> >      evict_inodes+0x161/0x1b0
> >      generic_shutdown_super+0x2d/0x110
> >      kill_anon_super+0xf/0x20
> >      btrfs_kill_super+0xd/0x20 [btrfs]
> >      deactivate_locked_super+0x27/0x90
> >      cleanup_mnt+0x12c/0x180
> >      task_work_run+0x54/0x80
> >      exit_to_user_mode_prepare+0x152/0x160
> >      syscall_exit_to_user_mode+0x12/0x30
> >      do_syscall_64+0x42/0x80
> >      entry_SYSCALL_64_after_hwframe+0x44/0xae
> >     RIP: 0033:0x7f854a000fb7
> > 
> > Fixes: f0bfa76a11e9 ("btrfs: fix ENOSPC failure when attempting direct IO write into NOCOW range")
> > CC: stable@vger.kernel.org # 5.17
> > Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
> > ---
> >  fs/btrfs/inode.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> > index c7b15634fe70..5c0c54057921 100644
> > --- a/fs/btrfs/inode.c
> > +++ b/fs/btrfs/inode.c
> > @@ -7409,6 +7409,7 @@ static int btrfs_get_blocks_direct_write(struct extent_map **map,
> >  	u64 block_start, orig_start, orig_block_len, ram_bytes;
> >  	bool can_nocow = false;
> >  	bool space_reserved = false;
> > +	u64 prev_len;
> >  	int ret = 0;
> >  
> >  	/*
> > @@ -7436,6 +7437,7 @@ static int btrfs_get_blocks_direct_write(struct extent_map **map,
> >  			can_nocow = true;
> >  	}
> >  
> > +	prev_len = len;
> >  	if (can_nocow) {
> >  		struct extent_map *em2;
> >  
> > @@ -7465,8 +7467,6 @@ static int btrfs_get_blocks_direct_write(struct extent_map **map,
> >  			goto out;
> >  		}
> >  	} else {
> > -		const u64 prev_len = len;
> > -
> >  		/* Our caller expects us to free the input extent map. */
> >  		free_extent_map(em);
> >  		*map = NULL;
> > @@ -7497,7 +7497,7 @@ static int btrfs_get_blocks_direct_write(struct extent_map **map,
> >  	 * We have created our ordered extent, so we can now release our reservation
> >  	 * for an outstanding extent.
> >  	 */
> > -	btrfs_delalloc_release_extents(BTRFS_I(inode), len);
> > +	btrfs_delalloc_release_extents(BTRFS_I(inode), prev_len);
> >  
> >  	/*
> >  	 * Need to update the i_size under the extent lock so buffered
> > -- 
> > 2.35.1
> >