From: Chandan Babu R <chandanrlinux@gmail.com>
To: "Darrick J. Wong" <djwong@kernel.org>
Cc: fstests@vger.kernel.org, linux-xfs@vger.kernel.org
Subject: Re: [PATCH V6 13/13] xfs: Stress test with bmap_alloc_minlen_extent error tag enabled
Date: Tue, 23 Mar 2021 10:58:11 +0530 [thread overview]
Message-ID: <877dlyqw0k.fsf@garuda> (raw)
In-Reply-To: <20210322185413.GH1670408@magnolia>
On 23 Mar 2021 at 00:24, Darrick J. Wong wrote:
> On Tue, Mar 09, 2021 at 10:31:24AM +0530, Chandan Babu R wrote:
>> This commit adds a stress test that executes fsstress with
>> bmap_alloc_minlen_extent error tag enabled.
>
> Continuing along the theme of watching the magic smoke come out when dir
> block size > fs block size, I also observed the following assertion when
> running this test:
>
> XFS: Assertion failed: done, file: fs/xfs/libxfs/xfs_dir2.c, line: 687
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 3892 at fs/xfs/xfs_message.c:112 assfail+0x3c/0x40 [xfs]
> Modules linked in: xfs(O) libcrc32c ip6t_REJECT nf_reject_ipv6 ipt_REJECT nf_reject_ipv4 ip_set_hash_ip ip_set_hash_net xt_tcpudp xt_set ip_set_hash_mac ip_set nfnetlink ip6table_filter ip6_tables iptable_filter bfq sch_fq_codel ip_tables x_tables overlay nfsv4 af_packet
> CPU: 0 PID: 3892 Comm: fsstress Tainted: G O 5.12.0-rc4-xfsx #rc4
> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.13.0-1ubuntu1.1 04/01/2014
> RIP: 0010:assfail+0x3c/0x40 [xfs]
> Code: d0 d5 41 a0 e8 81 f9 ff ff 8a 1d 5b 44 0e 00 80 fb 01 76 0f 0f b6 f3 48 c7 c7 b0 d5 4d a0 e8 93 dc fc e0 80 e3 01 74 02 0f 0b <0f> 0b 5b c3 48 8d 45 10 48 89 e2 4c 89 e6 48 89 1c 24 48 89 44 24
> RSP: 0018:ffffc900035bba38 EFLAGS: 00010246
> RAX: 0000000000000000 RBX: ffff88804f204100 RCX: 0000000000000000
> RDX: 00000000ffffffc0 RSI: 0000000000000000 RDI: ffffffffa040c157
> RBP: 0000000000000000 R08: 0000000000000000 R09: 000000000000000a
> R10: 000000000000000a R11: f000000000000000 R12: ffff88805920b880
> R13: ffff888003778bb0 R14: 0000000000000000 R15: ffff88800f0f63c0
> FS: 00007fe7b5e2f740(0000) GS:ffff88803ec00000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00007fe7b6055000 CR3: 0000000053094005 CR4: 00000000001706b0
> Call Trace:
> xfs_dir2_shrink_inode+0x22f/0x270 [xfs]
> xfs_dir2_block_to_sf+0x29a/0x420 [xfs]
> xfs_dir2_block_removename+0x221/0x290 [xfs]
> xfs_dir_removename+0x1a0/0x220 [xfs]
> xfs_dir_rename+0x343/0x3b0 [xfs]
> xfs_rename+0x79e/0xae0 [xfs]
> xfs_vn_rename+0xdb/0x150 [xfs]
> vfs_rename+0x4e2/0x8e0
> do_renameat2+0x393/0x550
> __x64_sys_rename+0x40/0x50
> do_syscall_64+0x2d/0x40
> entry_SYSCALL_64_after_hwframe+0x44/0xae
> RIP: 0033:0x7fe7b5e9800b
> Code: e8 aa ce 0a 00 85 c0 0f 95 c0 0f b6 c0 f7 d8 5d c3 66 0f 1f 44 00 00 b8 ff ff ff ff 5d c3 90 f3 0f 1e fa b8 52 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 05 c3 0f 1f 40 00 48 8b 15 51 4e 18 00 f7 d8
> RSP: 002b:00007ffeb526c698 EFLAGS: 00000246 ORIG_RAX: 0000000000000052
> RAX: ffffffffffffffda RBX: 00007ffeb526c970 RCX: 00007fe7b5e9800b
> RDX: 0000000000000000 RSI: 000055d6ccdb9250 RDI: 000055d6ccdb9270
> RBP: 00007ffeb526c980 R08: 0000000000000001 R09: 0000000000000003
> R10: 000055d6cc3b20dc R11: 0000000000000246 R12: 0000000000000000
> R13: 0000000000000040 R14: 00007ffeb526c970 R15: 00007ffeb526c980
> ---[ end trace 98f99784621d65fe ]---
>
> It looks to me as though we return from xfs_bunmapi having not completed
> all the unmapping work, though I can't tell if that's because bunmapi
> returned early because it thought it would overflow the extent count; or
> some other reason.
It could also be because the following conditions may have evaluated to true,
if (!wasdel && !isrt) {
agno = XFS_FSB_TO_AGNO(mp, del.br_startblock);
if (prev_agno != NULLAGNUMBER && prev_agno > agno)
break;
prev_agno = agno;
}
i.e. the fs block being unmapped belongs to an AG whose agno is less than that
of the previous fs block that was successfully unmapped.
I can't seem to recreate this bug. I tried it with 64k directory block size
with both 4k and 1k fs block sizes. I will keep trying.
However I hit another call trace with directory block size > fs block size,
------------[ cut here ]------------
WARNING: CPU: 2 PID: 6136 at fs/xfs/libxfs/xfs_bmap.c:717 xfs_bmap_extents_to_btree+0x520/0x5d0
Modules linked in:
CPU: 2 PID: 6136 Comm: fsstress Tainted: G W 5.12.0-rc2-chandan #7
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1 04/01/2014
RIP: 0010:xfs_bmap_extents_to_btree+0x520/0x5d0
Code: 5f fb ff ff 89 0c 24 48 c7 c2 1d 14 ac b2 b9 9d 02 00 00 31 ff 48 c7 c6 bf 14 ac b2 e8 1e 70 ac 00 44 8b 14 24 e9 55 ff ff ff <0f> 0b c7 44 24 0c e4 ff ff ff e9 0f ff ff ff b9 0e 03 00 00 48 c7
RSP: 0018:ffffb43d4939f5c0 EFLAGS: 00010246
RAX: ffffffffffffffff RBX: ffffa011c8887048 RCX: 0000000000000de5
RDX: 00000000ffffffff RSI: 0000000000000000 RDI: ffffa010e29c9000
RBP: ffffa010e2e00000 R08: ffffffffb180e81a R09: 0000000000000116
R10: 0000000000000000 R11: 0000000000000000 R12: ffffa011c8887000
R13: 0000000000000000 R14: ffffa010e11ed7e0 R15: ffffa011c8468100
FS: 00007efd9458fb80(0000) GS:ffffa011f7c80000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007efd9458d000 CR3: 00000001fff6c000 CR4: 00000000000006e0
Call Trace:
? __cond_resched+0x16/0x40
? __kmalloc_track_caller+0x6d/0x260
xfs_bmap_add_extent_hole_real+0x747/0x960
xfs_bmapi_allocate+0x380/0x410
xfs_bmapi_write+0x507/0x9e0
xfs_da_grow_inode_int+0x1cd/0x330
? _xfs_trans_bjoin+0x72/0x110
xfs_dir2_grow_inode+0x62/0x110
? xfs_trans_log_inode+0xce/0x2d0
xfs_dir2_sf_to_block+0x103/0x940
? xfs_dir2_sf_check+0x8c/0x210
? xfs_da_compname+0x19/0x30
? xfs_dir2_sf_lookup+0xd0/0x3d0
xfs_dir2_sf_addname+0x10d/0x910
xfs_dir_createname+0x1ad/0x210
? xfs_trans_log_inode+0xce/0x2d0
xfs_rename+0x803/0xbb0
? avc_has_perm_noaudit+0x83/0x100
xfs_vn_rename+0xdb/0x150
vfs_rename+0x691/0xa90
do_renameat2+0x393/0x540
__x64_sys_renameat2+0x4b/0x60
do_syscall_64+0x33/0x40
entry_SYSCALL_64_after_hwframe+0x44/0xae
RIP: 0033:0x7efd945e8e9f
Code: 44 00 00 48 8b 15 f1 5f 16 00 f7 d8 64 89 02 b8 ff ff ff ff c3 66 0f 1f 44 00 00 49 89 ca 45 85 c0 74 40 b8 3c 01 00 00 0f 05 <48> 3d 00 f0 ff ff 77 39 41 89 c0 83 f8 ff 74 09 44 89 c0 c3 0f 1f
RSP: 002b:00007fff55bbfee8 EFLAGS: 00000202 ORIG_RAX: 000000000000013c
RAX: ffffffffffffffda RBX: 0000563e5f50654f RCX: 00007efd945e8e9f
RDX: 00000000ffffff9c RSI: 0000563e5ff284e0 RDI: 00000000ffffff9c
RBP: 00007fff55bc0140 R08: 0000000000000004 R09: feff7efdff2f3562
R10: 0000563e5ff286c0 R11: 0000000000000202 R12: 0000563e5f4fb630
R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
---[ end trace 6d859f8beaa17680 ]---
>
> OH CRAP, I just realized that xfs_dir2_shrink_inode only calls
> xfs_bunmapi once, which means that if the directory block it's removing
> is a multi-fsb block, it will remove the last extent map. It then trips
> the assertion, having left the rest of the directory block still mapped.
>
> This is also what's going on when xfs_inactive_symlink_rmt trips the
> same ASSERT(done), because the symlink remote block can span multiple
> (two?) fs blocks but we only ever call xfs_bunmapi once.
>
> So, no, there's nothing wrong with this test, but it _did_ shake loose
> a couple of XFS bugs. Congratulations!
>
> So... who wants to tackle this one? This isn't trivial to clean up
> because you'll have to clean up all callers of xfs_dir2_shrink_inode to
> handle rolling of the transaction, and I bet the only way to fix this is
> to use deferred bunmap items to make sure the unmap always completes.
I will work on both the above listed call traces. Thanks for reporting the bug
and also the follow up analysis.
--
chandan
next prev parent reply other threads:[~2021-03-23 5:29 UTC|newest]
Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-09 5:01 [PATCH V6 00/13] xfs: Tests to verify inode fork extent count overflow detection Chandan Babu R
2021-03-09 5:01 ` [PATCH V6 01/13] _check_xfs_filesystem: sync fs before running scrub Chandan Babu R
2021-03-09 5:03 ` Darrick J. Wong
2021-03-10 6:12 ` Allison Henderson
2021-03-09 5:01 ` [PATCH V6 02/13] common/xfs: Add a helper to get an inode fork's extent count Chandan Babu R
2021-03-09 5:04 ` Darrick J. Wong
2021-03-10 6:12 ` Allison Henderson
2021-03-09 5:01 ` [PATCH V6 03/13] common/xfs: Add helper to obtain fsxattr field value Chandan Babu R
2021-03-09 5:04 ` Darrick J. Wong
2021-03-10 6:13 ` Allison Henderson
2021-03-10 19:54 ` Darrick J. Wong
2021-03-11 2:54 ` Chandan Babu R
2021-03-11 8:52 ` [PATCH V6.1] " Chandan Babu R
2021-03-11 18:36 ` Allison Henderson
2021-03-09 5:01 ` [PATCH V6 04/13] xfs: Check for extent overflow when trivally adding a new extent Chandan Babu R
2021-03-10 19:54 ` Allison Henderson
2021-03-09 5:01 ` [PATCH V6 05/13] xfs: Check for extent overflow when growing realtime bitmap/summary inodes Chandan Babu R
2021-03-10 19:55 ` Allison Henderson
2021-03-22 17:56 ` Darrick J. Wong
2021-03-23 15:51 ` Chandan Babu R
2021-03-23 20:57 ` Darrick J. Wong
2021-03-24 10:46 ` Chandan Babu R
2021-03-24 14:17 ` Chandan Babu R
2021-03-09 5:01 ` [PATCH V6 06/13] xfs: Check for extent overflow when punching a hole Chandan Babu R
2021-03-10 19:55 ` Allison Henderson
2021-03-09 5:01 ` [PATCH V6 07/13] xfs: Check for extent overflow when adding/removing xattrs Chandan Babu R
2021-03-10 19:55 ` Allison Henderson
2021-03-09 5:01 ` [PATCH V6 08/13] xfs: Check for extent overflow when adding/removing dir entries Chandan Babu R
2021-03-10 22:41 ` Allison Henderson
2021-03-09 5:01 ` [PATCH V6 09/13] xfs: Check for extent overflow when writing to unwritten extent Chandan Babu R
2021-03-10 22:41 ` Allison Henderson
2021-03-09 5:01 ` [PATCH V6 10/13] xfs: Check for extent overflow when moving extent from cow to data fork Chandan Babu R
2021-03-10 22:41 ` Allison Henderson
2021-03-09 5:01 ` [PATCH V6 11/13] xfs: Check for extent overflow when remapping an extent Chandan Babu R
2021-03-10 23:49 ` Allison Henderson
2021-03-09 5:01 ` [PATCH V6 12/13] xfs: Check for extent overflow when swapping extents Chandan Babu R
2021-03-10 23:49 ` Allison Henderson
2021-03-09 5:01 ` [PATCH V6 13/13] xfs: Stress test with bmap_alloc_minlen_extent error tag enabled Chandan Babu R
2021-03-10 23:49 ` Allison Henderson
2021-03-22 18:54 ` Darrick J. Wong
2021-03-23 5:28 ` Chandan Babu R [this message]
2021-03-23 11:27 ` Chandan Babu R
2021-03-26 4:05 ` Chandan Babu R
2021-03-26 4:21 ` Darrick J. Wong
2021-04-15 9:33 ` Chandan Babu R
2021-04-17 0:26 ` Darrick J. Wong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=877dlyqw0k.fsf@garuda \
--to=chandanrlinux@gmail.com \
--cc=djwong@kernel.org \
--cc=fstests@vger.kernel.org \
--cc=linux-xfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.