Re: [PATCH] block: fix request.queuelist usage in flush

Friedrich Weber
2024-06-07T15:14:02Z

On 07/06/2024 04:37, Chengming Zhou wrote:
> On 2024/6/6 16:44, Friedrich Weber wrote:
>> [...]
>>
>> I used mainline kernel v6.10-rc2 as base and applied:
>>
>> - "block: fix request.queuelist usage in flush"
>> - Your `list_del_init` addition from above
>>
>> and if I boot the Debian machine into this kernel, I do not get the
>> crash anymore.
>
> Good to hear. So can I merge these two diffs into one patch and add
> your Tested-by?

I applied your merged patch [1] on top of mainline v6.10-rc2 (c3f38fa61a):

- I cannot reproduce the crash from [0] anymore in the (virtual) machine
with root (on LVM) on software RAID1

- I cannot reproduce the `blk_mq_requeue_work` crash from this thread
anymore in the Debian VM with root on LVM. With cache=writeback for the
VM disk, I get the expected in-guest WARNING [2] on VM boot.

- No crashes on bare-metal either. If write caching is enabled, I get
the expected WARNING.

So this looks good to me:

Tested-by: Friedrich Weber <f.weber@proxmox.com>

We might backport the merged patch (minus the WARN, possibly) to our
downstream Proxmox VE kernel 6.9 to fix the software RAID crash [0] --
if I understand correctly, the merged patch should be safe for now until
dm is fixed.

Thanks a lot for your work on this!

Best,

Friedrich

[0]
https://lore.kernel.org/all/14b89dfb-505c-49f7-aebb-01c54451db40@proxmox.com/

[1]

diff --git a/block/blk-flush.c b/block/blk-flush.c
index c17cf8ed8113..3d72393a1710 100644
--- a/block/blk-flush.c
+++ b/block/blk-flush.c
@@ -185,7 +185,7 @@ static void blk_flush_complete_seq(struct request *rq,
/* queue for flush */
if (list_empty(pending))
fq->flush_pending_since = jiffies;
- list_move_tail(&rq->queuelist, pending);
+ list_add_tail(&rq->queuelist, pending);
break;

case REQ_FSEQ_DATA:
@@ -263,6 +263,7 @@ static enum rq_end_io_ret flush_end_io(struct
request *flush_rq,
unsigned int seq = blk_flush_cur_seq(rq);

BUG_ON(seq != REQ_FSEQ_PREFLUSH && seq != REQ_FSEQ_POSTFLUSH);
+ list_del_init(&rq->queuelist);
blk_flush_complete_seq(rq, fq, seq, error);
}

@@ -402,6 +403,12 @@ bool blk_insert_flush(struct request *rq)
unsigned int policy = blk_flush_policy(fflags, rq);
struct blk_flush_queue *fq = blk_get_flush_queue(q, rq->mq_ctx);

+ /*
+ * PREFLUSH | POSTFLUSH is a weird invalid format,
+ * need to fix in the upper layer, catch it here.
+ */
+ WARN_ON_ONCE(policy == (REQ_FSEQ_PREFLUSH | REQ_FSEQ_POSTFLUSH));
+
/* FLUSH/FUA request must never be merged */
WARN_ON_ONCE(rq->bio != rq->biotail);

[2]

[ 2.142204] ------------[ cut here ]------------
[ 2.142206] WARNING: CPU: 0 PID: 179 at block/blk-flush.c:410
blk_insert_flush+0xff/0x270
[ 2.142211] Modules linked in: efi_pstore(E) dmi_sysfs(E)
qemu_fw_cfg(E) ip_tables(E) x_tables(E) autofs4(E) hid_generic(E)
usbhid(E) hid(E) psmouse(E) bochs(E) drm_vram_helper(E)
drm_ttm_helper(E) ahci(E) ttm(E) i2c_piix4(E) uhci_hcd(E) ehci_hcd(E)
libahci(E) pata_acpi(E) floppy(E)
[ 2.142225] CPU: 0 PID: 179 Comm: jbd2/dm-0-8 Tainted: G E
6.10.0-rc2-nohardened-patch0607+ #41
[ 2.142226] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS rel-1.16.3-0-ga6ed6b701f0a-prebuilt.qemu.org 04/01/2014
[ 2.142227] RIP: 0010:blk_insert_flush+0xff/0x270
[ 2.142229] Code: cc cc cc cc a9 00 00 04 00 74 3d 44 89 e2 83 ca 01
4d 85 c0 75 69 a9 00 00 02 00 0f 84 15 01 00 00 45 85 e4 0f 85 59 01 00
00 <0f> 0b 48 39 ce 0f 85 44 01 00 00 25 ff ff f9 ff 41 bc 05 00 00 00
[ 2.142230] RSP: 0018:ffffa608c0303a30 EFLAGS: 00010246
[ 2.142231] RAX: 0000000000069801 RBX: ffff93b70dc89600 RCX:
ffff93b70dd7baf8
[ 2.142233] RDX: 0000000000000001 RSI: ffff93b70dd7baf8 RDI:
ffff93b70d545e00
[ 2.142233] RBP: ffffa608c0303a48 R08: 0000000000000000 R09:
0000000000000000
[ 2.142234] R10: 0000000000000000 R11: 0000000000000000 R12:
0000000000000000
[ 2.142235] R13: ffff93b70d127980 R14: 0000000000000000 R15:
ffff93b70dc89600
[ 2.142236] FS: 0000000000000000(0000) GS:ffff93b837c00000(0000)
knlGS:0000000000000000
[ 2.142237] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 2.142238] CR2: 00005d28ed347fb8 CR3: 000000010d62e000 CR4:
00000000000006f0
[ 2.142240] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
0000000000000000
[ 2.142240] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7:
0000000000000400
[ 2.142241] Call Trace:
[ 2.142244] <TASK>
[ 2.142248] ? show_regs+0x6c/0x80
[ 2.142251] ? __warn+0x88/0x140
[ 2.142253] ? blk_insert_flush+0xff/0x270
[ 2.142254] ? report_bug+0x182/0x1b0
[ 2.142256] ? handle_bug+0x46/0x90
[ 2.142258] ? exc_invalid_op+0x18/0x80
[ 2.142259] ? asm_exc_invalid_op+0x1b/0x20
[ 2.142261] ? blk_insert_flush+0xff/0x270
[ 2.142262] blk_mq_submit_bio+0x5c9/0x740
[ 2.142265] __submit_bio+0x6e/0x250
[ 2.142267] submit_bio_noacct_nocheck+0x1a3/0x3c0
[ 2.142269] submit_bio_noacct+0x1dc/0x650
[ 2.142271] submit_bio+0xb1/0x110
[ 2.142272] submit_bh_wbc+0x163/0x1a0
[ 2.142274] submit_bh+0x12/0x20
[ 2.142275] journal_submit_commit_record+0x1c5/0x250
[ 2.142278] jbd2_journal_commit_transaction+0x120d/0x1960
[ 2.142281] ? __schedule+0x408/0x15d0
[ 2.142284] kjournald2+0xaa/0x280
[ 2.142285] ? __pfx_autoremove_wake_function+0x10/0x10
[ 2.142288] ? __pfx_kjournald2+0x10/0x10
[ 2.142289] kthread+0xe4/0x110
[ 2.142291] ? __pfx_kthread+0x10/0x10
[ 2.142292] ret_from_fork+0x47/0x70
[ 2.142294] ? __pfx_kthread+0x10/0x10
[ 2.142295] ret_from_fork_asm+0x1a/0x30
[ 2.142298] </TASK>
[ 2.142298] ---[ end trace 0000000000000000 ]---