On Sun, Oct 14, 2018 at 08:48:22PM +0200, Christoph Hellwig wrote:
> We need to make sure we have no outstanding COW blocks before we swap
> extents, as there is nothing preventing us from having preallocated COW
> delalloc on either inode that swapext is called on. That case can
> easily be reproduced by running generic/324 in always_cow mode:
>
> [ 620.760572] XFS: Assertion failed: tip->i_delayed_blks == 0, file: fs/xfs/xfs_bmap_util.c, line: 1669
> [ 620.761608] ------------[ cut here ]------------
> [ 620.762171] kernel BUG at fs/xfs/xfs_message.c:102!
> [ 620.762732] invalid opcode: 0000 [#1] SMP PTI
> [ 620.763272] CPU: 0 PID: 24153 Comm: xfs_fsr Tainted: G W 4.19.0-rc1+ #4182
> [ 620.764203] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.11.1-1 04/01/2014
> [ 620.765202] RIP: 0010:assfail+0x20/0x28
> [ 620.765646] Code: 31 ff e8 83 fc ff ff 0f 0b c3 48 89 f1 41 89 d0 48 c7 c6 48 ca 8d 82 48 89 fa 38
> [ 620.767758] RSP: 0018:ffffc9000898bc10 EFLAGS: 00010202
> [ 620.768359] RAX: 0000000000000000 RBX: ffff88012f14ba40 RCX: 0000000000000000
> [ 620.769174] RDX: 00000000ffffffc0 RSI: 000000000000000a RDI: ffffffff828560d9
> [ 620.769982] RBP: ffff88012f14b300 R08: 0000000000000000 R09: 0000000000000000
> [ 620.770788] R10: 000000000000000a R11: f000000000000000 R12: ffffc9000898bc98
> [ 620.771638] R13: ffffc9000898bc9c R14: ffff880130b5e2b8 R15: ffff88012a1fa2a8
> [ 620.772504] FS: 00007fdc36e0fbc0(0000) GS:ffff88013ba00000(0000) knlGS:0000000000000000
> [ 620.773475] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 620.774168] CR2: 00007fdc3604d000 CR3: 0000000132afc000 CR4: 00000000000006f0
> [ 620.774978] Call Trace:
> [ 620.775274] xfs_swap_extent_forks+0x2a0/0x2e0
> [ 620.775792] xfs_swap_extents+0x38b/0xab0
> [ 620.776256] xfs_ioc_swapext+0x121/0x140
> [ 620.776709] xfs_file_ioctl+0x328/0xc90
> [ 620.777154] ? rcu_read_lock_sched_held+0x50/0x60
> [ 620.777694] ? xfs_iunlock+0x233/0x260
> [ 620.778127] ? xfs_setattr_nonsize+0x3be/0x6a0
> [ 620.778647] do_vfs_ioctl+0x9d/0x680
> [ 620.779071] ? ksys_fchown+0x47/0x80
> [ 620.779552] ksys_ioctl+0x35/0x70
> [ 620.780040] __x64_sys_ioctl+0x11/0x20
> [ 620.780530] do_syscall_64+0x4b/0x190
> [ 620.780927] entry_SYSCALL_64_after_hwframe+0x49/0xbe
> [ 620.781467] RIP: 0033:0x7fdc364d0f07
> [ 620.781900] Code: b3 66 90 48 8b 05 81 5f 2c 00 64 c7 00 26 00 00 00 48 c7 c0 ff ff ff ff c3 66 28
> [ 620.784044] RSP: 002b:00007ffe2a766038 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
> [ 620.784896] RAX: ffffffffffffffda RBX: 0000000000000025 RCX: 00007fdc364d0f07
> [ 620.785667] RDX: 0000560296ca2fc0 RSI: 00000000c0c0586d RDI: 0000000000000005
> [ 620.786398] RBP: 0000000000000025 R08: 0000000000001200 R09: 0000000000000000
> [ 620.787283] R10: 0000000000000432 R11: 0000000000000246 R12: 0000000000000005
> [ 620.788051] R13: 0000000000000000 R14: 0000000000001000 R15: 0000000000000006
> [ 620.788927] Modules linked in:
> [ 620.789340] ---[ end trace 9503b7417ffdbdb0 ]---
> [ 620.790065] RIP: 0010:assfail+0x20/0x28
> [ 620.790642] Code: 31 ff e8 83 fc ff ff 0f 0b c3 48 89 f1 41 89 d0 48 c7 c6 48 ca 8d 82 48 89 fa 38
> [ 620.793038] RSP: 0018:ffffc9000898bc10 EFLAGS: 00010202
> [ 620.793609] RAX: 0000000000000000 RBX: ffff88012f14ba40 RCX: 0000000000000000
> [ 620.794317] RDX: 00000000ffffffc0 RSI: 000000000000000a RDI: ffffffff828560d9
> [ 620.795025] RBP: ffff88012f14b300 R08: 0000000000000000 R09: 0000000000000000
> [ 620.795778] R10: 000000000000000a R11: f000000000000000 R12: ffffc9000898bc98
> [ 620.796675] R13: ffffc9000898bc9c R14: ffff880130b5e2b8 R15: ffff88012a1fa2a8
> [ 620.797782] FS: 00007fdc36e0fbc0(0000) GS:ffff88013ba00000(0000) knlGS:0000000000000000
> [ 620.798908] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 620.799594] CR2: 00007fdc3604d000 CR3: 0000000132afc000 CR4: 00000000000006f0
> [ 620.800424] Kernel panic - not syncing: Fatal exception
> [ 620.801191] Kernel Offset: disabled
> [ 620.801597] ---[ end Kernel panic - not syncing: Fatal exception ]---
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
>
> Now only cancels the COW range on the target inode to address the comment
> from Brian.
>
> fs/xfs/xfs_bmap_util.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/fs/xfs/xfs_bmap_util.c b/fs/xfs/xfs_bmap_util.c
> index 7cfda25f1bb1..5d263dfdb3bc 100644
> --- a/fs/xfs/xfs_bmap_util.c
> +++ b/fs/xfs/xfs_bmap_util.c
> @@ -1775,6 +1775,12 @@ xfs_swap_extents(
> if (error)
> goto out_unlock;
>
> + if (xfs_inode_has_cow_data(tip)) {
> + error = xfs_reflink_cancel_cow_range(tip, 0, NULLFILEOFF, true);
> + if (error)
> + return error;
> + }
> +
This could use a comment to explain why it's necessary IMO, but looking
ahead in xfs_swap_extent_forks() shows that we assume the temp inode has
no delalloc blocks. This change seems fine, though I am kind of
wondering whether the same problem is still possible outside of COW
through a partial conversion of delalloc blocks on the temp inode. E.g.,
data is copied from source to temp, temp has speculative preallocation,
flush only allocates to EOF due to fragmentation and the temp inode
post-eof blocks end up breaking accounting in the same way.
That aside:
Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>
> /*
> * Extent "swapping" with rmap requires a permanent reservation and
> * a block reservation because it's really just a remap operation
> --
> 2.19.1
>
