Turning on -Wformat for Clang #346
Assignees
Labels
duplicate
This issue or pull request already exists
Comments
nathanchance
added
[BUG] linux
|
An internal bug was just filed for this this week.
…On Sat, Feb 2, 2019, 5:20 AM Nathan Chancellor ***@***.*** wrote:
***@***.***/
I am doing a set of builds to see if it picks up any additional warnings,
which I'll file separate issues for, then I'll review it upstream
(assigning myself for tracking).
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#346>, or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABvUX3DpOO-k0_sYxoAOiq7kPEtHW2lLks5vJSAYgaJpZM4afbYI>
.
|
Or not because... https://gist.github.com/nathanchance/b23edb45743ddfc1d2107d1ffdd46e2a Doesn't necessarily seem super bad (like the |
|
Patch "kbuild: LLVMLinux: Don't suppress format warnings" now in kbuild.git#kbuild. |
|
Well that's annoying... Guess I'll start fixing them. |
|
#378 is tracking this now. |
tpimh
added
duplicate
nathanchance
pushed a commit
that referenced
this issue
Dec 18, 2020
Internally, UBD treats each physical IO segment as a separate command to be submitted in the execution pipe. If the pipe returns a transient error after a few segments have already been written, UBD will tell the block layer to requeue the request, but there is no way to reclaim the segments already submitted. When a new attempt to dispatch the request is done, those segments already submitted will get duplicated, causing the WARN_ON below in the best case, and potentially data corruption. In my system, running a UML instance with 2GB of RAM and a 50M UBD disk, I can reproduce the WARN_ON by simply running mkfs.fvat against the disk on a freshly booted system. There are a few ways to around this, like reducing the pressure on the pipe by reducing the queue depth, which almost eliminates the occurrence of the problem, increasing the pipe buffer size on the host system, or by limiting the request to one physical segment, which causes the block layer to submit way more requests to resolve a single operation. Instead, this patch modifies the format of a UBD command, such that all segments are sent through a single element in the communication pipe, turning the command submission atomic from the point of view of the block layer. The new format has a variable size, depending on the number of elements, and looks like this: +------------+-----------+-----------+------------ | cmd_header | segment 0 | segment 1 | segment ... +------------+-----------+-----------+------------ With this format, we push a pointer to cmd_header in the submission pipe. This has the advantage of reducing the memory footprint of executing a single request, since it allow us to merge some fields in the header. It is possible to reduce even further each segment memory footprint, by merging bitmap_words and cow_offset, for instance, but this is not the focus of this patch and is left as future work. One issue with the patch is that for a big number of segments, we now perform one big memory allocation instead of multiple small ones, but I wasn't able to trigger any real issues or -ENOMEM because of this change, that wouldn't be reproduced otherwise. This was tested using fio with the verify-crc32 option, and by running an ext4 filesystem over this UBD device. The original WARN_ON was: ------------[ cut here ]------------ WARNING: CPU: 0 PID: 0 at lib/refcount.c:28 refcount_warn_saturate+0x13f/0x141 refcount_t: underflow; use-after-free. Modules linked in: CPU: 0 PID: 0 Comm: swapper Not tainted 5.5.0-rc6-00002-g2a5bb2cf75c8 #346 Stack: 6084eed0 6063dc77 00000009 6084ef60 00000000 604b8d9f 6084eee0 6063dcbc 6084ef40 6006ab8d e013d780 1c00000000 Call Trace: [<600a0c1c>] ? printk+0x0/0x94 [<6004a888>] show_stack+0x13b/0x155 [<6063dc77>] ? dump_stack_print_info+0xdf/0xe8 [<604b8d9f>] ? refcount_warn_saturate+0x13f/0x141 [<6063dcbc>] dump_stack+0x2a/0x2c [<6006ab8d>] __warn+0x107/0x134 [<6008da6c>] ? wake_up_process+0x17/0x19 [<60487628>] ? blk_queue_max_discard_sectors+0x0/0xd [<6006b05f>] warn_slowpath_fmt+0xd1/0xdf [<6006af8e>] ? warn_slowpath_fmt+0x0/0xdf [<600acc14>] ? raw_read_seqcount_begin.constprop.0+0x0/0x15 [<600619ae>] ? os_nsecs+0x1d/0x2b [<604b8d9f>] refcount_warn_saturate+0x13f/0x141 [<6048bc8f>] refcount_sub_and_test.constprop.0+0x2f/0x37 [<6048c8de>] blk_mq_free_request+0xf1/0x10d [<6048ca06>] __blk_mq_end_request+0x10c/0x114 [<6005ac0f>] ubd_intr+0xb5/0x169 [<600a1a37>] __handle_irq_event_percpu+0x6b/0x17e [<600a1b70>] handle_irq_event_percpu+0x26/0x69 [<600a1bd9>] handle_irq_event+0x26/0x34 [<600a1bb3>] ? handle_irq_event+0x0/0x34 [<600a5186>] ? unmask_irq+0x0/0x37 [<600a57e6>] handle_edge_irq+0xbc/0xd6 [<600a131a>] generic_handle_irq+0x21/0x29 [<60048f6e>] do_IRQ+0x39/0x54 [...] ---[ end trace c6e7444e55386c0f ]--- Cc: Christopher Obbard <[email protected]> Reported-by: Martyn Welch <[email protected]> Signed-off-by: Gabriel Krisman Bertazi <[email protected]> Tested-by: Christopher Obbard <[email protected]> Acked-by: Anton Ivanov <[email protected]> Signed-off-by: Richard Weinberger <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
https://lore.kernel.org/lkml/[email protected]/
I am doing a set of builds to see if it picks up any additional warnings, which I'll file separate issues for, then I'll review it upstream (assigning myself for tracking).
The text was updated successfully, but these errors were encountered: