-Walign-mismatch in block/blk-mq.c #1328

nathanchance · 2021-03-10T18:23:54Z

block/blk-mq.c:630:39: warning: passing 8-byte aligned argument to 32-byte aligned parameter 2 of 'smp_call_function_single_async' may result in an unaligned pointer access [-Walign-mismatch]
                smp_call_function_single_async(cpu, &rq->csd);
                                                    ^
1 warning generated.

Reported upstream due to the history behind it: https://lore.kernel.org/r/20210310182307.zzcbi5w5jrmveld4@archlinux-ax161/

The text was updated successfully, but these errors were encountered:

nathanchance · 2021-03-12T00:20:01Z

Some links:

https://git.kernel.org/linus/966a967116e69
https://git.kernel.org/linus/4ccafe032005
https://lore.kernel.org/r/a9beb452-7344-9e2d-fc80-094d8f5a0394@kernel.dk/

My suggestion for a fix:

https://lore.kernel.org/r/20210310225240.4epj2mdmzt4vurr3@archlinux-ax161/

nickdesaulniers · 2021-04-01T19:15:44Z

@jcai19 also sent a patch for this: https://lore.kernel.org/lkml/20210330230249.709221-1-jiancai@google.com/

@groeck links to: http://crbug.com/1193732

nickdesaulniers · 2021-04-07T18:33:56Z

@davidbolvansky cc'ed me on https://reviews.llvm.org/D100037 which looks somewhat related to this.

groeck · 2021-04-07T18:50:19Z

On 4/7/21 11:34 AM, Nick Desaulniers wrote: @davidbolvansky <https://github.com/davidbolvansky> cc'ed me on https://reviews.llvm.org/D100037 which looks somewhat related to this.

I can't comment there, but from the context it appears that "was introduces" should be "was introduced", not "introduces". Guenter

…

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1328 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAH3SMGPLZ5ZWWAC4IGTNNTTHSQSRANCNFSM4Y6TLSVA>.

nathanchance · 2021-04-08T19:45:47Z

Patch sent to disable the warning for the one problematic translation unit: https://lore.kernel.org/r/20210408194458.501617-1-nathan@kernel.org/

jcai19 · 2021-04-08T19:48:28Z

cc @m-gupta

jcai19 · 2021-04-08T19:50:36Z

cc @cjdb

nickdesaulniers · 2021-04-09T20:49:25Z

If creating an unaligned pointer is UB...

6.3.2.3 Pointers 7 (N2176 ISO/IEC 9899:2017)
A pointer to an object type may be converted to a pointer to a different object type. If the resulting
pointer is not correctly aligned) for the referenced type, the behavior is undefined.

I wonder if we should be detecting whether the address of the pointer is at runtime a multiple of the larger aligned type and casting else memcpying the unaligned struct to an aligned-alloc'ed struct of the larger alignment, then passing a pointer to that, then freeing it.

How else do you solve such issues trying to use data that that's currently underaligned from what a caller expects, safely? Making a copy of the data is the only thing that comes to mind.

LLVM 13 adds a new warning, -Walign-mismatch, which has an instance in blk_mq_complete_send_ipi(): block/blk-mq.c:630:39: warning: passing 8-byte aligned argument to 32-byte aligned parameter 2 of 'smp_call_function_single_async' may result in an unaligned pointer access [-Walign-mismatch] smp_call_function_single_async(cpu, &rq->csd); ^ 1 warning generated. This is expected after commit 4ccafe0 ("block: unalign call_single_data in struct request"), which purposefully unaligned the structure to save space. Given that there is no real alignment requirement and there have been no reports of issues since that change, it should be safe to disable the warning for this one translation unit. Link: ClangBuiltLinux/linux#1328 Link: https://lore.kernel.org/r/20210310182307.zzcbi5w5jrmveld4@archlinux-ax161/ Link: https://lore.kernel.org/r/20210330230249.709221-1-jiancai@google.com/ Signed-off-by: Nathan Chancellor <nathan@kernel.org> Reviewed-by: Guenter Roeck <linux@roeck-us.net> Link: https://lore.kernel.org/r/20210408194458.501617-1-nathan@kernel.org Signed-off-by: Nathan Chancellor <nathan@kernel.org>

LLVM 13 adds a new warning, -Walign-mismatch, which has an instance in blk_mq_complete_send_ipi(): block/blk-mq.c:630:39: warning: passing 8-byte aligned argument to 32-byte aligned parameter 2 of 'smp_call_function_single_async' may result in an unaligned pointer access [-Walign-mismatch] smp_call_function_single_async(cpu, &rq->csd); ^ 1 warning generated. This is expected after commit 4ccafe0 ("block: unalign call_single_data in struct request"), which purposefully unaligned the structure to save space. Given that there is no real alignment requirement and there have been no reports of issues since that change, it should be safe to disable the warning for this one translation unit. Link: ClangBuiltLinux/linux#1328 Link: https://lore.kernel.org/r/20210310182307.zzcbi5w5jrmveld4@archlinux-ax161/ Link: https://lore.kernel.org/r/20210330230249.709221-1-jiancai@google.com/ Signed-off-by: Nathan Chancellor <nathan@kernel.org> Reviewed-by: Guenter Roeck <linux@roeck-us.net> (am from https://lore.kernel.org/patchwork/patch/1410076/) (also found at https://lore.kernel.org/r/20210408194458.501617-1-nathan@kernel.org) BUG=chromium:1193732 TEST=CQ Signed-off-by: Jian Cai <jiancai@google.com> Change-Id: Idcc50c393ee2df6d9b199cf0450527f1294197a7 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/2831384 Reviewed-by: Sean Paul <seanpaul@chromium.org> Reviewed-by: Guenter Roeck <groeck@chromium.org>

LLVM 13 adds a new warning, -Walign-mismatch, which has an instance in blk_mq_complete_send_ipi(): block/blk-mq.c:630:39: warning: passing 8-byte aligned argument to 32-byte aligned parameter 2 of 'smp_call_function_single_async' may result in an unaligned pointer access [-Walign-mismatch] smp_call_function_single_async(cpu, &rq->csd); ^ 1 warning generated. This is expected after commit 4ccafe0 ("block: unalign call_single_data in struct request"), which purposefully unaligned the structure to save space. Given that there is no real alignment requirement and there have been no reports of issues since that change, it should be safe to disable the warning for this one translation unit. Link: ClangBuiltLinux/linux#1328 Link: https://lore.kernel.org/r/20210310182307.zzcbi5w5jrmveld4@archlinux-ax161/ Link: https://lore.kernel.org/r/20210330230249.709221-1-jiancai@google.com/ Signed-off-by: Nathan Chancellor <nathan@kernel.org> Reviewed-by: Guenter Roeck <linux@roeck-us.net> (am from https://lore.kernel.org/patchwork/patch/1410076/) (also found at https://lore.kernel.org/r/20210408194458.501617-1-nathan@kernel.org) BUG=chromium:1193732 TEST=CQ Signed-off-by: Jian Cai <jiancai@google.com> Change-Id: I1f9d3e04fd417058e73c67843fd8ccee60b5e64e Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/2831385 Reviewed-by: Sean Paul <seanpaul@chromium.org> Reviewed-by: Guenter Roeck <groeck@chromium.org>

LLVM 13 adds a new warning, -Walign-mismatch, which has an instance in blk_mq_complete_send_ipi(): block/blk-mq.c:630:39: warning: passing 8-byte aligned argument to 32-byte aligned parameter 2 of 'smp_call_function_single_async' may result in an unaligned pointer access [-Walign-mismatch] smp_call_function_single_async(cpu, &rq->csd); ^ 1 warning generated. This is expected after commit 4ccafe0 ("block: unalign call_single_data in struct request"), which purposefully unaligned the structure to save space. Given that there is no real alignment requirement and there have been no reports of issues since that change, it should be safe to disable the warning for this one translation unit. Link: ClangBuiltLinux/linux#1328 Link: https://lore.kernel.org/r/20210310182307.zzcbi5w5jrmveld4@archlinux-ax161/ Link: https://lore.kernel.org/r/20210330230249.709221-1-jiancai@google.com/ Signed-off-by: Nathan Chancellor <nathan@kernel.org> Reviewed-by: Guenter Roeck <linux@roeck-us.net> Link: https://lore.kernel.org/r/20210408194458.501617-1-nathan@kernel.org Signed-off-by: Nathan Chancellor <nathan@kernel.org>

arndb · 2021-04-29T15:11:05Z

Posted a new patch: https://lore.kernel.org/lkml/20210429150940.3256656-1-arnd@kernel.org/

As of commit 966a967 ("smp: Avoid using two cache lines for struct call_single_data"), the smp code prefers 32-byte aligned call_single_data objects for performance reasons, but the block layer includes an instance of this structure in the main 'struct request' that is more senstive to size than to performance here, see 4ccafe0 ("block: unalign call_single_data in struct request"). The result is a violation of the calling conventions that clang correctly points out: block/blk-mq.c:630:39: warning: passing 8-byte aligned argument to 32-byte aligned parameter 2 of 'smp_call_function_single_async' may result in an unaligned pointer access [-Walign-mismatch] smp_call_function_single_async(cpu, &rq->csd); It does seem that the usage of the call_single_data without cache line alignment should still be allowed by the smp code, so just change the function prototype so it accepts both, but leave the default alignment unchanged for the other users. This seems better to me than adding a local hack to shut up an otherwise correct warning in the caller. Link: https://lore.kernel.org/linux-block/20210330230249.709221-1-jiancai@google.com/ Link: ClangBuiltLinux#1328 Cc: Jens Axboe <axboe@kernel.dk> Cc: Jian Cai <jiancai@google.com> Cc: Guenter Roeck <linux@roeck-us.net> Cc: Peter Zijlstra <peterz@infradead.org> Cc: "Huang, Ying" <ying.huang@intel.com> Cc: Borislav Petkov <bp@suse.de> Cc: Eric Dumazet <eric.dumazet@gmail.com> Cc: Juergen Gross <jgross@suse.com> Cc: Michael Ellerman <mpe@ellerman.id.au> Cc: Thomas Gleixner <tglx@linutronix.de> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

LLVM 13 adds a new warning, -Walign-mismatch, which has an instance in blk_mq_complete_send_ipi(): block/blk-mq.c:630:39: warning: passing 8-byte aligned argument to 32-byte aligned parameter 2 of 'smp_call_function_single_async' may result in an unaligned pointer access [-Walign-mismatch] smp_call_function_single_async(cpu, &rq->csd); ^ 1 warning generated. This is expected after commit 4ccafe0 ("block: unalign call_single_data in struct request"), which purposefully unaligned the structure to save space. Given that there is no real alignment requirement and there have been no reports of issues since that change, it should be safe to disable the warning for this one translation unit. Link: ClangBuiltLinux/linux#1328 Link: https://lore.kernel.org/r/20210310182307.zzcbi5w5jrmveld4@archlinux-ax161/ Link: https://lore.kernel.org/r/20210330230249.709221-1-jiancai@google.com/ Signed-off-by: Nathan Chancellor <nathan@kernel.org> Reviewed-by: Guenter Roeck <linux@roeck-us.net> Link: https://lore.kernel.org/r/20210408194458.501617-1-nathan@kernel.org Signed-off-by: Nathan Chancellor <nathan@kernel.org>

As of commit 966a967 ("smp: Avoid using two cache lines for struct call_single_data"), the smp code prefers 32-byte aligned call_single_data objects for performance reasons, but the block layer includes an instance of this structure in the main 'struct request' that is more senstive to size than to performance here, see 4ccafe0 ("block: unalign call_single_data in struct request"). The result is a violation of the calling conventions that clang correctly points out: block/blk-mq.c:630:39: warning: passing 8-byte aligned argument to 32-byte aligned parameter 2 of 'smp_call_function_single_async' may result in an unaligned pointer access [-Walign-mismatch] smp_call_function_single_async(cpu, &rq->csd); It does seem that the usage of the call_single_data without cache line alignment should still be allowed by the smp code, so just change the function prototype so it accepts both, but leave the default alignment unchanged for the other users. This seems better to me than adding a local hack to shut up an otherwise correct warning in the caller. Link: https://lore.kernel.org/linux-block/20210330230249.709221-1-jiancai@google.com/ Link: ClangBuiltLinux#1328 Acked-by: Jens Axboe <axboe@kernel.dk> Cc: Jian Cai <jiancai@google.com> Cc: Guenter Roeck <linux@roeck-us.net> Cc: Peter Zijlstra <peterz@infradead.org> Cc: "Huang, Ying" <ying.huang@intel.com> Cc: Borislav Petkov <bp@suse.de> Cc: Eric Dumazet <eric.dumazet@gmail.com> Cc: Juergen Gross <jgross@suse.com> Cc: Michael Ellerman <mpe@ellerman.id.au> Cc: Thomas Gleixner <tglx@linutronix.de> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

nickdesaulniers · 2021-05-06T16:38:09Z

accepted: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?h=locking/urgent&id=1139aeb1c521eb4a050920ce6c64c36c4f2a3ab7

(Will need to backport to stable)

LLVM 13 adds a new warning, -Walign-mismatch, which has an instance in blk_mq_complete_send_ipi(): block/blk-mq.c:630:39: warning: passing 8-byte aligned argument to 32-byte aligned parameter 2 of 'smp_call_function_single_async' may result in an unaligned pointer access [-Walign-mismatch] smp_call_function_single_async(cpu, &rq->csd); ^ 1 warning generated. This is expected after commit 4ccafe0 ("block: unalign call_single_data in struct request"), which purposefully unaligned the structure to save space. Given that there is no real alignment requirement and there have been no reports of issues since that change, it should be safe to disable the warning for this one translation unit. Link: ClangBuiltLinux/linux#1328 Link: https://lore.kernel.org/r/20210310182307.zzcbi5w5jrmveld4@archlinux-ax161/ Link: https://lore.kernel.org/r/20210330230249.709221-1-jiancai@google.com/ Signed-off-by: Nathan Chancellor <nathan@kernel.org> Reviewed-by: Guenter Roeck <linux@roeck-us.net> Link: https://lore.kernel.org/r/20210408194458.501617-1-nathan@kernel.org Signed-off-by: Nathan Chancellor <nathan@kernel.org>

nathanchance · 2021-05-09T22:08:52Z

Merged into mainline: https://git.kernel.org/linus/1139aeb1c521eb4a050920ce6c64c36c4f2a3ab7

I can do the backports tomorrow.

nathanchance · 2021-05-10T21:38:41Z

Backports submitted: https://lore.kernel.org/r/YJmneuxxFWIrqyWN@archlinux-ax161/

I expect these to be picked up and released within the next couple of weeks so I am closing this up for now.

LLVM 13 adds a new warning, -Walign-mismatch, which has an instance in blk_mq_complete_send_ipi(): block/blk-mq.c:630:39: warning: passing 8-byte aligned argument to 32-byte aligned parameter 2 of 'smp_call_function_single_async' may result in an unaligned pointer access [-Walign-mismatch] smp_call_function_single_async(cpu, &rq->csd); ^ 1 warning generated. This is expected after commit 4ccafe0 ("block: unalign call_single_data in struct request"), which purposefully unaligned the structure to save space. Given that there is no real alignment requirement and there have been no reports of issues since that change, it should be safe to disable the warning for this one translation unit. Link: ClangBuiltLinux/linux#1328 Link: https://lore.kernel.org/r/20210310182307.zzcbi5w5jrmveld4@archlinux-ax161/ Link: https://lore.kernel.org/r/20210330230249.709221-1-jiancai@google.com/ Signed-off-by: Nathan Chancellor <nathan@kernel.org> Reviewed-by: Guenter Roeck <linux@roeck-us.net> Link: https://lore.kernel.org/r/20210408194458.501617-1-nathan@kernel.org Signed-off-by: Nathan Chancellor <nathan@kernel.org>

nathanchance added [BUG] linux A bug that should be fixed in the mainline kernel. Reported upstream This bug was filed on LLVM’s issue tracker, Phabricator, or the kernel mailing list. -Walign-mismatch labels Mar 10, 2021

nathanchance added the [PATCH] Submitted A patch has been submitted for review label Apr 8, 2021

nathanchance mentioned this issue Apr 22, 2021

Section mismatch in reference from the function remove_pmd_table() to the function .meminit.text:split_kernel_mapping() #1064

Closed

nickdesaulniers assigned arndb Apr 29, 2021

nickdesaulniers added [PATCH] Accepted A submitted patch has been accepted upstream Needs Backport Should be backported to either linux-stable tree or latest llvm release branch. and removed [PATCH] Submitted A patch has been submitted for review labels May 6, 2021

nathanchance assigned nathanchance and unassigned arndb May 9, 2021

nathanchance added [FIXED][LINUX] 5.13 This bug was fixed in Linux 5.13 and removed [PATCH] Accepted A submitted patch has been accepted upstream labels May 9, 2021

nathanchance closed this as completed May 10, 2021

nathanchance removed the Needs Backport Should be backported to either linux-stable tree or latest llvm release branch. label May 10, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

-Walign-mismatch in block/blk-mq.c #1328

-Walign-mismatch in block/blk-mq.c #1328

nathanchance commented Mar 10, 2021

nathanchance commented Mar 12, 2021

nickdesaulniers commented Apr 1, 2021

nickdesaulniers commented Apr 7, 2021

groeck commented Apr 7, 2021 via email

nathanchance commented Apr 8, 2021

jcai19 commented Apr 8, 2021

jcai19 commented Apr 8, 2021

nickdesaulniers commented Apr 9, 2021

arndb commented Apr 29, 2021

nickdesaulniers commented May 6, 2021

nathanchance commented May 9, 2021

nathanchance commented May 10, 2021

-Walign-mismatch in block/blk-mq.c #1328

-Walign-mismatch in block/blk-mq.c #1328

Comments

nathanchance commented Mar 10, 2021

nathanchance commented Mar 12, 2021

nickdesaulniers commented Apr 1, 2021

nickdesaulniers commented Apr 7, 2021

groeck commented Apr 7, 2021 via email

nathanchance commented Apr 8, 2021

jcai19 commented Apr 8, 2021

jcai19 commented Apr 8, 2021

nickdesaulniers commented Apr 9, 2021

arndb commented Apr 29, 2021

nickdesaulniers commented May 6, 2021

nathanchance commented May 9, 2021

nathanchance commented May 10, 2021