Re: [PATCH 1/3] xfs: pass alloc flags through to xfs_extent_busy_flush()

Wengang Wang
2023-06-15T23:09:52Z

When mounting the problematic metadump with the patches, I see the following reported.

For more information about troubleshooting your instance using a console connection, see the documentation: https://docs.cloud.oracle.com/en-us/iaas/Content/Compute/References/serialconsole.htm#four
=================================================
[ 67.212496] loop: module loaded
[ 67.214732] loop0: detected capacity change from 0 to 629137408
[ 67.247542] XFS (loop0): Deprecated V4 format (crc=0) will not be supported after September 2030.
[ 67.249257] XFS (loop0): Mounting V4 Filesystem af755a98-5f62-421d-aa81-2db7bffd2c40
[ 72.241546] XFS (loop0): Starting recovery (logdev: internal)
[ 92.218256] XFS (loop0): Internal error ltbno + ltlen > bno at line 1957 of file fs/xfs/libxfs/xfs_alloc.c. Caller xfs_free_ag_extent+0x3f6/0x870 [xfs]
[ 92.249802] CPU: 1 PID: 4201 Comm: mount Not tainted 6.4.0-rc6 #8
[ 92.250869] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.5.1 06/16/2021
[ 92.252301] Call Trace:
[ 92.252784] <TASK>
[ 92.253166] dump_stack_lvl+0x4c/0x70
[ 92.253866] dump_stack+0x14/0x20
[ 92.254475] xfs_corruption_error+0x94/0xa0 [xfs]
[ 92.255571] ? xfs_free_ag_extent+0x3f6/0x870 [xfs]
[ 92.256595] xfs_free_ag_extent+0x428/0x870 [xfs]
[ 92.257609] ? xfs_free_ag_extent+0x3f6/0x870 [xfs]
[ 92.258638] __xfs_free_extent+0x95/0x170 [xfs]
[ 92.259618] xfs_trans_free_extent+0x97/0x1c0 [xfs]
[ 92.260672] xfs_extent_free_finish_item+0x18/0x50 [xfs]
[ 92.261823] xfs_defer_finish_noroll+0x18e/0x6c0 [xfs]
[ 92.262892] __xfs_trans_commit+0x241/0x360 [xfs]
[ 92.263900] ? kfree+0x7c/0x120
[ 92.264506] xfs_trans_commit+0x14/0x20 [xfs]
[ 92.265506] xlog_recover_process_intents.isra.28+0x1db/0x2d0 [xfs]
[ 92.266803] xlog_recover_finish+0x35/0x300 [xfs]
[ 92.267848] ? __queue_delayed_work+0x5e/0x90
[ 92.268634] xfs_log_mount_finish+0xe8/0x170 [xfs]
[ 92.269677] xfs_mountfs+0x51a/0x900 [xfs]
[ 92.270608] xfs_fs_fill_super+0x4df/0x980 [xfs]
[ 92.271642] get_tree_bdev+0x1a3/0x280
[ 92.272287] ? __pfx_xfs_fs_fill_super+0x10/0x10 [xfs]
[ 92.273415] xfs_fs_get_tree+0x19/0x20 [xfs]
[ 92.274374] vfs_get_tree+0x2b/0xd0
[ 92.275006] ? capable+0x1d/0x30
[ 92.275599] path_mount+0x709/0xa40
[ 92.276202] ? putname+0x5d/0x70
[ 92.276808] do_mount+0x84/0xa0
[ 92.277364] __x64_sys_mount+0xd1/0xf0
[ 92.278107] do_syscall_64+0x3b/0x90
[ 92.278779] entry_SYSCALL_64_after_hwframe+0x72/0xdc
[ 92.279703] RIP: 0033:0x7fdc8683a26e
[ 92.280801] Code: 48 8b 0d 1d 4c 38 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 49 89 ca b8 a5 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d ea 4b 38 00 f7 d8 64 89 01 48
[ 92.284752] RSP: 002b:00007ffc213711d8 EFLAGS: 00000246 ORIG_RAX: 00000000000000a5
[ 92.286460] RAX: ffffffffffffffda RBX: 0000561748023b40 RCX: 00007fdc8683a26e
[ 92.288048] RDX: 0000561748032f70 RSI: 0000561748025a20 RDI: 000056174802b010
[ 92.289665] RBP: 00007fdc87e58184 R08: 0000000000000000 R09: 0000000000000000
[ 92.291315] R10: 00000000c0ed0000 R11: 0000000000000246 R12: 0000000000000000
[ 92.292898] R13: 00000000c0ed0000 R14: 000056174802b010 R15: 0000561748032f70
[ 92.294505] </TASK>
[ 92.295351] XFS (loop0): Corruption detected. Unmount and run xfs_repair
[ 92.297073] XFS (loop0): Corruption of in-memory data (0x8) detected at xfs_defer_finish_noroll+0x406/0x6c0 [xfs] (fs/xfs/libxfs/xfs_defer.c:575). Shutting down filesystem.
[ 92.300686] XFS (loop0): Please unmount the filesystem and rectify the problem(s)
[ 92.302372] XFS (loop0): Failed to recover intents
[ 92.303571] XFS (loop0): Ending recovery (logdev: internal)

I think that’s because that the same EFI record was going to be freed again
by xfs_extent_free_finish_item() after it already got freed by xfs_efi_item_recover().
I was trying to fix above issue in my previous patch by checking the intent
log item’s lsn and avoid running iop_recover() in xlog_recover_process_intents().

Now I am thinking if we can pass a flag, say XFS_EFI_PROCESSED, from
xfs_efi_item_recover() after it processed that record to the xfs_efi_log_item
memory structure somehow. In xfs_extent_free_finish_item(), we skip to process
that xfs_efi_log_item on seeing XFS_EFI_PROCESSED and return OK. By that
we can avoid the double free.

thanks,
wengang

> On Jun 15, 2023, at 3:31 PM, Wengang Wang <wen.gang.wang@oracle.com> wrote:
>
>
>
>> On Jun 15, 2023, at 3:14 PM, Dave Chinner <david@fromorbit.com> wrote:
>>
>> On Thu, Jun 15, 2023 at 09:57:18PM +0000, Wengang Wang wrote:
>>> Hi Dave,
>>>
>>> I got this error when applying this patch to the newest code:
>>> (I tried on both MacOS and OL8, it’s the same result)
>>>
>>> $ patch -p1 <~/tmp/Dave1.txt
>>> patching file 'fs/xfs/libxfs/xfs_alloc.c'
>>> patch: **** malformed patch at line 27: */
>>
>> Whoa. I haven't use patch like this for a decade. :)
>>
>> The way all the cool kids do this now is apply the entire series to
>> directly to a git tree branch with b4:
>>
>> $ b4 am -o - 20230615014201.3171380-2-david@fromorbit.com | git am
>>
>> (b4 shazam merges the b4 am and git am operations into the one
>> command, IIRC, but that trick isn't automatic for me yet :)
>>> Looking at the patch to see the line numbers:
>>>
>>> 22 diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
>>> 23 index c20fe99405d8..11bd0a1756a1 100644
>>> 24 --- a/fs/xfs/libxfs/xfs_alloc.c
>>> 25 +++ b/fs/xfs/libxfs/xfs_alloc.c
>>> 26 @@ -1536,7 +1536,8 @@ xfs_alloc_ag_vextent_lastblock(
>>> 27 */
>>> 28 STATIC int
>>> 29 xfs_alloc_ag_vextent_near(
>>
>> Yup, however you saved the patch to a file stripped the leading
>> spaces from all the lines in the patch.
>>
>> If you look at the raw email on lore it has the correct leading
>> spaces in the patch.
>>
>> https://lore.kernel.org/linux-xfs/20230615014201.3171380-2-david@fromorbit.com/raw
>>
>> These sorts of patching problems go away when you use tools like b4
>> to pull the patches directly from the mailing list...
>>
>>>> On Jun 14, 2023, at 6:41 PM, Dave Chinner <david@fromorbit.com> wrote:
>>>>
>>>> From: Dave Chinner <dchinner@redhat.com>
>>>>
>>>> To avoid blocking in xfs_extent_busy_flush() when freeing extents
>>>> and the only busy extents are held by the current transaction, we
>>>> need to pass the XFS_ALLOC_FLAG_FREEING flag context all the way
>>>> into xfs_extent_busy_flush().
>>>>
>>>> Signed-off-by: Dave Chinner <dchinner@redhat.com>
>>>> ---
>>>> fs/xfs/libxfs/xfs_alloc.c | 96 +++++++++++++++++++++------------------
>>>> fs/xfs/libxfs/xfs_alloc.h | 2 +-
>>>> fs/xfs/xfs_extent_busy.c | 3 +-
>>>> fs/xfs/xfs_extent_busy.h | 2 +-
>>>> 4 files changed, 56 insertions(+), 47 deletions(-)
>>>>
>>>> diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
>>>> index c20fe99405d8..11bd0a1756a1 100644
>>>> --- a/fs/xfs/libxfs/xfs_alloc.c
>>>> +++ b/fs/xfs/libxfs/xfs_alloc.c
>>>> @@ -1536,7 +1536,8 @@ xfs_alloc_ag_vextent_lastblock(
>>>> */
>>>> STATIC int
>>>> xfs_alloc_ag_vextent_near(
>>>> - struct xfs_alloc_arg *args)
>>>> + struct xfs_alloc_arg *args,
>>>> + uint32_t alloc_flags)
>>>> {
>>>> struct xfs_alloc_cur acur = {};
>>>> int error; /* error code */
>>
>> This indicates the problem is likely to be your mail program,
>> because the quoting it has done here has completely mangled all the
>> whitespace in the patch....
>>
>
> Right, the .txt was not raw.
> Now I got the raw formats and they applied to upstream master. Let me run log recover with my metadump, will report result later.
>
> thanks,
> wengang
>
>> Cheers,
>>
>> Dave.
>> --
>> Dave Chinner
>> david@fromorbit.com