* [PATCH] blk-mq: fix alignment mismatch. @ 2021-03-30 23:02 Jian Cai 2021-03-30 23:12 ` Guenter Roeck 2021-03-30 23:29 ` Nathan Chancellor 0 siblings, 2 replies; 14+ messages in thread From: Jian Cai @ 2021-03-30 23:02 UTC (permalink / raw) Cc: cjdb, manojgupta, llozano, clang-built-linux, Jian Cai, Guenter Roeck, Jens Axboe, Nathan Chancellor, Nick Desaulniers, linux-block, linux-kernel This fixes the mismatch of alignments between csd and its use as an argument to smp_call_function_single_async, which causes build failure when -Walign-mismatch in Clang is used. Link: http://crrev.com/c/1193732 Suggested-by: Guenter Roeck <linux@roeck-us.net> Signed-off-by: Jian Cai <jiancai@google.com> --- include/linux/blkdev.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index bc6bc8383b43..3b92330d95ad 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -231,7 +231,7 @@ struct request { unsigned long deadline; union { - struct __call_single_data csd; + call_single_data_t csd; u64 fifo_time; }; -- 2.31.0.291.g576ba9dcdaf-goog ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] blk-mq: fix alignment mismatch. 2021-03-30 23:02 [PATCH] blk-mq: fix alignment mismatch Jian Cai @ 2021-03-30 23:12 ` Guenter Roeck 2021-03-30 23:29 ` Nathan Chancellor 1 sibling, 0 replies; 14+ messages in thread From: Guenter Roeck @ 2021-03-30 23:12 UTC (permalink / raw) To: Jian Cai Cc: cjdb, manojgupta, llozano, clang-built-linux, Jens Axboe, Nathan Chancellor, Nick Desaulniers, linux-block, linux-kernel On Tue, Mar 30, 2021 at 04:02:49PM -0700, Jian Cai wrote: > This fixes the mismatch of alignments between csd and its use as an > argument to smp_call_function_single_async, which causes build failure > when -Walign-mismatch in Clang is used. > > Link: > http://crrev.com/c/1193732 That link does not work. You probably meant http://crbug.com/1193732 You'll probably want drop the link and to copy the relevant information directly into the patch description. Generate the error with an upstream kernel and cut-and-paste the output here. For others, the observed error message is: block/blk-mq.c:622:44: error: passing 8-byte aligned argument to 32-byte aligned parameter 2 of 'smp_call_function_single_async' may result in an unaligned pointer access [-Werror,-Walign-mismatch] Thanks, Guenter > > Suggested-by: Guenter Roeck <linux@roeck-us.net> > Signed-off-by: Jian Cai <jiancai@google.com> > --- > include/linux/blkdev.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h > index bc6bc8383b43..3b92330d95ad 100644 > --- a/include/linux/blkdev.h > +++ b/include/linux/blkdev.h > @@ -231,7 +231,7 @@ struct request { > unsigned long deadline; > > union { > - struct __call_single_data csd; > + call_single_data_t csd; > u64 fifo_time; > }; > > -- > 2.31.0.291.g576ba9dcdaf-goog > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] blk-mq: fix alignment mismatch. 2021-03-30 23:02 [PATCH] blk-mq: fix alignment mismatch Jian Cai 2021-03-30 23:12 ` Guenter Roeck @ 2021-03-30 23:29 ` Nathan Chancellor 2021-03-31 0:26 ` Guenter Roeck 2021-03-31 10:38 ` [PATCH] blk-mq: fix alignment mismatch David Laight 1 sibling, 2 replies; 14+ messages in thread From: Nathan Chancellor @ 2021-03-30 23:29 UTC (permalink / raw) To: Jian Cai Cc: cjdb, manojgupta, llozano, clang-built-linux, Guenter Roeck, Jens Axboe, Nick Desaulniers, linux-block, linux-kernel Hi Jian, On Tue, Mar 30, 2021 at 04:02:49PM -0700, Jian Cai wrote: > This fixes the mismatch of alignments between csd and its use as an > argument to smp_call_function_single_async, which causes build failure > when -Walign-mismatch in Clang is used. > > Link: > http://crrev.com/c/1193732 > > Suggested-by: Guenter Roeck <linux@roeck-us.net> > Signed-off-by: Jian Cai <jiancai@google.com> Thanks for the patch. This is effectively a revert of commit 4ccafe032005 ("block: unalign call_single_data in struct request"), which I had brought up in this thread: https://lore.kernel.org/r/20210310182307.zzcbi5w5jrmveld4@archlinux-ax161/ This is obviously a correct fix, I am not just sure what the impact to 'struct request' will be. Cheers, Nathan > --- > include/linux/blkdev.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h > index bc6bc8383b43..3b92330d95ad 100644 > --- a/include/linux/blkdev.h > +++ b/include/linux/blkdev.h > @@ -231,7 +231,7 @@ struct request { > unsigned long deadline; > > union { > - struct __call_single_data csd; > + call_single_data_t csd; > u64 fifo_time; > }; > > -- > 2.31.0.291.g576ba9dcdaf-goog > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] blk-mq: fix alignment mismatch. 2021-03-30 23:29 ` Nathan Chancellor @ 2021-03-31 0:26 ` Guenter Roeck 2021-03-31 1:31 ` Jian Cai 2021-03-31 10:38 ` [PATCH] blk-mq: fix alignment mismatch David Laight 1 sibling, 1 reply; 14+ messages in thread From: Guenter Roeck @ 2021-03-31 0:26 UTC (permalink / raw) To: Nathan Chancellor, Jian Cai Cc: cjdb, manojgupta, llozano, clang-built-linux, Jens Axboe, Nick Desaulniers, linux-block, linux-kernel On 3/30/21 4:29 PM, Nathan Chancellor wrote: > Hi Jian, > > On Tue, Mar 30, 2021 at 04:02:49PM -0700, Jian Cai wrote: >> This fixes the mismatch of alignments between csd and its use as an >> argument to smp_call_function_single_async, which causes build failure >> when -Walign-mismatch in Clang is used. >> >> Link: >> http://crrev.com/c/1193732 >> >> Suggested-by: Guenter Roeck <linux@roeck-us.net> >> Signed-off-by: Jian Cai <jiancai@google.com> > > Thanks for the patch. This is effectively a revert of commit > 4ccafe032005 ("block: unalign call_single_data in struct request"), > which I had brought up in this thread: > > https://lore.kernel.org/r/20210310182307.zzcbi5w5jrmveld4@archlinux-ax161/ > > This is obviously a correct fix, I am not just sure what the impact to > 'struct request' will be. > As commit 4ccafe032005 states, it increases the request structure size. Given the exchange referenced above, I think we'll need to disable the warning in the block code. Thanks, Guenter > Cheers, > Nathan > >> --- >> include/linux/blkdev.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h >> index bc6bc8383b43..3b92330d95ad 100644 >> --- a/include/linux/blkdev.h >> +++ b/include/linux/blkdev.h >> @@ -231,7 +231,7 @@ struct request { >> unsigned long deadline; >> >> union { >> - struct __call_single_data csd; >> + call_single_data_t csd; >> u64 fifo_time; >> }; >> >> -- >> 2.31.0.291.g576ba9dcdaf-goog >> ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] blk-mq: fix alignment mismatch. 2021-03-31 0:26 ` Guenter Roeck @ 2021-03-31 1:31 ` Jian Cai 2021-03-31 21:27 ` Jian Cai 0 siblings, 1 reply; 14+ messages in thread From: Jian Cai @ 2021-03-31 1:31 UTC (permalink / raw) To: Guenter Roeck Cc: Nathan Chancellor, Christopher Di Bella, Manoj Gupta, Luis Lozano, clang-built-linux, Jens Axboe, Nick Desaulniers, linux-block, Linux Kernel Mailing List Thanks for all the information. I'll check for similar instances and send an updated version. On Tue, Mar 30, 2021 at 5:26 PM Guenter Roeck <linux@roeck-us.net> wrote: > > On 3/30/21 4:29 PM, Nathan Chancellor wrote: > > Hi Jian, > > > > On Tue, Mar 30, 2021 at 04:02:49PM -0700, Jian Cai wrote: > >> This fixes the mismatch of alignments between csd and its use as an > >> argument to smp_call_function_single_async, which causes build failure > >> when -Walign-mismatch in Clang is used. > >> > >> Link: > >> http://crrev.com/c/1193732 > >> > >> Suggested-by: Guenter Roeck <linux@roeck-us.net> > >> Signed-off-by: Jian Cai <jiancai@google.com> > > > > Thanks for the patch. This is effectively a revert of commit > > 4ccafe032005 ("block: unalign call_single_data in struct request"), > > which I had brought up in this thread: > > > > https://lore.kernel.org/r/20210310182307.zzcbi5w5jrmveld4@archlinux-ax161/ > > > > This is obviously a correct fix, I am not just sure what the impact to > > 'struct request' will be. > > > > As commit 4ccafe032005 states, it increases the request structure size. > Given the exchange referenced above, I think we'll need to disable > the warning in the block code. > > Thanks, > Guenter > > > Cheers, > > Nathan > > > >> --- > >> include/linux/blkdev.h | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h > >> index bc6bc8383b43..3b92330d95ad 100644 > >> --- a/include/linux/blkdev.h > >> +++ b/include/linux/blkdev.h > >> @@ -231,7 +231,7 @@ struct request { > >> unsigned long deadline; > >> > >> union { > >> - struct __call_single_data csd; > >> + call_single_data_t csd; > >> u64 fifo_time; > >> }; > >> > >> -- > >> 2.31.0.291.g576ba9dcdaf-goog > >> > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] blk-mq: fix alignment mismatch. 2021-03-31 1:31 ` Jian Cai @ 2021-03-31 21:27 ` Jian Cai 2021-03-31 21:58 ` Nathan Chancellor 0 siblings, 1 reply; 14+ messages in thread From: Jian Cai @ 2021-03-31 21:27 UTC (permalink / raw) To: Guenter Roeck Cc: Nathan Chancellor, Christopher Di Bella, Manoj Gupta, Luis Lozano, clang-built-linux, Jens Axboe, Nick Desaulniers, linux-block, Linux Kernel Mailing List Hi Nathan, I just realized you already proposed solutions for skipping the check in https://lore.kernel.org/linux-block/20210310225240.4epj2mdmzt4vurr3@archlinux-ax161/#t. Do you have any plans to send them for review? Thanks, Jian On Tue, Mar 30, 2021 at 6:31 PM Jian Cai <jiancai@google.com> wrote: > > Thanks for all the information. I'll check for similar instances and > send an updated version. > > > On Tue, Mar 30, 2021 at 5:26 PM Guenter Roeck <linux@roeck-us.net> wrote: > > > > On 3/30/21 4:29 PM, Nathan Chancellor wrote: > > > Hi Jian, > > > > > > On Tue, Mar 30, 2021 at 04:02:49PM -0700, Jian Cai wrote: > > >> This fixes the mismatch of alignments between csd and its use as an > > >> argument to smp_call_function_single_async, which causes build failure > > >> when -Walign-mismatch in Clang is used. > > >> > > >> Link: > > >> http://crrev.com/c/1193732 > > >> > > >> Suggested-by: Guenter Roeck <linux@roeck-us.net> > > >> Signed-off-by: Jian Cai <jiancai@google.com> > > > > > > Thanks for the patch. This is effectively a revert of commit > > > 4ccafe032005 ("block: unalign call_single_data in struct request"), > > > which I had brought up in this thread: > > > > > > https://lore.kernel.org/r/20210310182307.zzcbi5w5jrmveld4@archlinux-ax161/ > > > > > > This is obviously a correct fix, I am not just sure what the impact to > > > 'struct request' will be. > > > > > > > As commit 4ccafe032005 states, it increases the request structure size. > > Given the exchange referenced above, I think we'll need to disable > > the warning in the block code. > > > > Thanks, > > Guenter > > > > > Cheers, > > > Nathan > > > > > >> --- > > >> include/linux/blkdev.h | 2 +- > > >> 1 file changed, 1 insertion(+), 1 deletion(-) > > >> > > >> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h > > >> index bc6bc8383b43..3b92330d95ad 100644 > > >> --- a/include/linux/blkdev.h > > >> +++ b/include/linux/blkdev.h > > >> @@ -231,7 +231,7 @@ struct request { > > >> unsigned long deadline; > > >> > > >> union { > > >> - struct __call_single_data csd; > > >> + call_single_data_t csd; > > >> u64 fifo_time; > > >> }; > > >> > > >> -- > > >> 2.31.0.291.g576ba9dcdaf-goog > > >> > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] blk-mq: fix alignment mismatch. 2021-03-31 21:27 ` Jian Cai @ 2021-03-31 21:58 ` Nathan Chancellor 2021-03-31 22:06 ` Nick Desaulniers 0 siblings, 1 reply; 14+ messages in thread From: Nathan Chancellor @ 2021-03-31 21:58 UTC (permalink / raw) To: Jian Cai Cc: Guenter Roeck, Christopher Di Bella, Manoj Gupta, Luis Lozano, clang-built-linux, Jens Axboe, Nick Desaulniers, linux-block, Linux Kernel Mailing List Hi Jian, On Wed, Mar 31, 2021 at 02:27:03PM -0700, Jian Cai wrote: > Hi Nathan, > > I just realized you already proposed solutions for skipping the check > in https://lore.kernel.org/linux-block/20210310225240.4epj2mdmzt4vurr3@archlinux-ax161/#t. > Do you have any plans to send them for review? > > Thanks, > Jian I was hoping to gather some feedback on which option would be preferred by Jens and the other ClangBuiltLinux folks before I sent them along. I can send the first just to see what kind of feedback I can gather. Cheers, Nathan > On Tue, Mar 30, 2021 at 6:31 PM Jian Cai <jiancai@google.com> wrote: > > > > Thanks for all the information. I'll check for similar instances and > > send an updated version. > > > > > > On Tue, Mar 30, 2021 at 5:26 PM Guenter Roeck <linux@roeck-us.net> wrote: > > > > > > On 3/30/21 4:29 PM, Nathan Chancellor wrote: > > > > Hi Jian, > > > > > > > > On Tue, Mar 30, 2021 at 04:02:49PM -0700, Jian Cai wrote: > > > >> This fixes the mismatch of alignments between csd and its use as an > > > >> argument to smp_call_function_single_async, which causes build failure > > > >> when -Walign-mismatch in Clang is used. > > > >> > > > >> Link: > > > >> http://crrev.com/c/1193732 > > > >> > > > >> Suggested-by: Guenter Roeck <linux@roeck-us.net> > > > >> Signed-off-by: Jian Cai <jiancai@google.com> > > > > > > > > Thanks for the patch. This is effectively a revert of commit > > > > 4ccafe032005 ("block: unalign call_single_data in struct request"), > > > > which I had brought up in this thread: > > > > > > > > https://lore.kernel.org/r/20210310182307.zzcbi5w5jrmveld4@archlinux-ax161/ > > > > > > > > This is obviously a correct fix, I am not just sure what the impact to > > > > 'struct request' will be. > > > > > > > > > > As commit 4ccafe032005 states, it increases the request structure size. > > > Given the exchange referenced above, I think we'll need to disable > > > the warning in the block code. > > > > > > Thanks, > > > Guenter > > > > > > > Cheers, > > > > Nathan > > > > > > > >> --- > > > >> include/linux/blkdev.h | 2 +- > > > >> 1 file changed, 1 insertion(+), 1 deletion(-) > > > >> > > > >> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h > > > >> index bc6bc8383b43..3b92330d95ad 100644 > > > >> --- a/include/linux/blkdev.h > > > >> +++ b/include/linux/blkdev.h > > > >> @@ -231,7 +231,7 @@ struct request { > > > >> unsigned long deadline; > > > >> > > > >> union { > > > >> - struct __call_single_data csd; > > > >> + call_single_data_t csd; > > > >> u64 fifo_time; > > > >> }; > > > >> > > > >> -- > > > >> 2.31.0.291.g576ba9dcdaf-goog > > > >> > > > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] blk-mq: fix alignment mismatch. 2021-03-31 21:58 ` Nathan Chancellor @ 2021-03-31 22:06 ` Nick Desaulniers 2021-04-08 17:57 ` Jian Cai 0 siblings, 1 reply; 14+ messages in thread From: Nick Desaulniers @ 2021-03-31 22:06 UTC (permalink / raw) To: Nathan Chancellor, Jens Axboe Cc: Jian Cai, Guenter Roeck, Christopher Di Bella, Manoj Gupta, Luis Lozano, clang-built-linux, linux-block, Linux Kernel Mailing List On Wed, Mar 31, 2021 at 2:58 PM Nathan Chancellor <nathan@kernel.org> wrote: > > On Wed, Mar 31, 2021 at 02:27:03PM -0700, Jian Cai wrote: > > > > I just realized you already proposed solutions for skipping the check > > in https://lore.kernel.org/linux-block/20210310225240.4epj2mdmzt4vurr3@archlinux-ax161/#t. > > Do you have any plans to send them for review? > > I was hoping to gather some feedback on which option would be preferred > by Jens and the other ClangBuiltLinux folks before I sent them along. I > can send the first just to see what kind of feedback I can gather. Either approach is fine by me. The smaller might be easier to get accepted into stable. The larger approach will probably become more useful in the future (having the diag infra work properly with clang). I think the ball is kind of in Jens' court to decide. Would doing both be appropriate, Jens? Have the smaller patch tagged for stable disabling it for the whole file, then another commit on top not tagged for stable that adds the diag infra, and a third on top replacing the file level warning disablement with local diags to isolate this down to one case? It's a fair but small amount of churn IMO; but if Jens is not opposed it seems fine? -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] blk-mq: fix alignment mismatch. 2021-03-31 22:06 ` Nick Desaulniers @ 2021-04-08 17:57 ` Jian Cai 2021-04-08 18:12 ` Nathan Chancellor 0 siblings, 1 reply; 14+ messages in thread From: Jian Cai @ 2021-04-08 17:57 UTC (permalink / raw) To: Nick Desaulniers Cc: Nathan Chancellor, Jens Axboe, Guenter Roeck, Christopher Di Bella, Manoj Gupta, Luis Lozano, clang-built-linux, linux-block, Linux Kernel Mailing List So this issue is blocking the LLVM upgrading on ChromeOS. Nathan, do you mind sending out the smaller patch like Nick suggested just to see what feedback we could get? I could send it for you if you are busy, and please let me know what tags I should use in that case. Thanks, Jian On Wed, Mar 31, 2021 at 3:06 PM Nick Desaulniers <ndesaulniers@google.com> wrote: > > On Wed, Mar 31, 2021 at 2:58 PM Nathan Chancellor <nathan@kernel.org> wrote: > > > > On Wed, Mar 31, 2021 at 02:27:03PM -0700, Jian Cai wrote: > > > > > > I just realized you already proposed solutions for skipping the check > > > in https://lore.kernel.org/linux-block/20210310225240.4epj2mdmzt4vurr3@archlinux-ax161/#t. > > > Do you have any plans to send them for review? > > > > I was hoping to gather some feedback on which option would be preferred > > by Jens and the other ClangBuiltLinux folks before I sent them along. I > > can send the first just to see what kind of feedback I can gather. > > Either approach is fine by me. The smaller might be easier to get > accepted into stable. The larger approach will probably become more > useful in the future (having the diag infra work properly with clang). > I think the ball is kind of in Jens' court to decide. Would doing > both be appropriate, Jens? Have the smaller patch tagged for stable > disabling it for the whole file, then another commit on top not tagged > for stable that adds the diag infra, and a third on top replacing the > file level warning disablement with local diags to isolate this down > to one case? It's a fair but small amount of churn IMO; but if Jens > is not opposed it seems fine? > -- > Thanks, > ~Nick Desaulniers ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] blk-mq: fix alignment mismatch. 2021-04-08 17:57 ` Jian Cai @ 2021-04-08 18:12 ` Nathan Chancellor 2021-04-08 18:42 ` Jian Cai 2021-04-08 19:44 ` [PATCH] block: Disable -Walign-mismatch for blk-mq.c Nathan Chancellor 0 siblings, 2 replies; 14+ messages in thread From: Nathan Chancellor @ 2021-04-08 18:12 UTC (permalink / raw) To: Jian Cai Cc: Nick Desaulniers, Jens Axboe, Guenter Roeck, Christopher Di Bella, Manoj Gupta, Luis Lozano, clang-built-linux, linux-block, Linux Kernel Mailing List Hi Jian, On Thu, Apr 08, 2021 at 10:57:54AM -0700, Jian Cai wrote: > So this issue is blocking the LLVM upgrading on ChromeOS. Nathan, do > you mind sending out the smaller patch like Nick suggested just to see > what feedback we could get? I could send it for you if you are busy, > and please let me know what tags I should use in that case. > > Thanks, > Jian I will go ahead and send the smaller patch at some point today. For what it's worth, Nick brought up https://reviews.llvm.org/D100037, which may be relevant here. Cheers, Nathan > On Wed, Mar 31, 2021 at 3:06 PM Nick Desaulniers > <ndesaulniers@google.com> wrote: > > > > On Wed, Mar 31, 2021 at 2:58 PM Nathan Chancellor <nathan@kernel.org> wrote: > > > > > > On Wed, Mar 31, 2021 at 02:27:03PM -0700, Jian Cai wrote: > > > > > > > > I just realized you already proposed solutions for skipping the check > > > > in https://lore.kernel.org/linux-block/20210310225240.4epj2mdmzt4vurr3@archlinux-ax161/#t. > > > > Do you have any plans to send them for review? > > > > > > I was hoping to gather some feedback on which option would be preferred > > > by Jens and the other ClangBuiltLinux folks before I sent them along. I > > > can send the first just to see what kind of feedback I can gather. > > > > Either approach is fine by me. The smaller might be easier to get > > accepted into stable. The larger approach will probably become more > > useful in the future (having the diag infra work properly with clang). > > I think the ball is kind of in Jens' court to decide. Would doing > > both be appropriate, Jens? Have the smaller patch tagged for stable > > disabling it for the whole file, then another commit on top not tagged > > for stable that adds the diag infra, and a third on top replacing the > > file level warning disablement with local diags to isolate this down > > to one case? It's a fair but small amount of churn IMO; but if Jens > > is not opposed it seems fine? > > -- > > Thanks, > > ~Nick Desaulniers ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] blk-mq: fix alignment mismatch. 2021-04-08 18:12 ` Nathan Chancellor @ 2021-04-08 18:42 ` Jian Cai 2021-04-08 19:44 ` [PATCH] block: Disable -Walign-mismatch for blk-mq.c Nathan Chancellor 1 sibling, 0 replies; 14+ messages in thread From: Jian Cai @ 2021-04-08 18:42 UTC (permalink / raw) To: Nathan Chancellor Cc: Nick Desaulniers, Jens Axboe, Guenter Roeck, Christopher Di Bella, Manoj Gupta, Luis Lozano, clang-built-linux, linux-block, Linux Kernel Mailing List Sounds good! Thanks for the help and the link. On Thu, Apr 8, 2021 at 11:12 AM Nathan Chancellor <nathan@kernel.org> wrote: > > Hi Jian, > > On Thu, Apr 08, 2021 at 10:57:54AM -0700, Jian Cai wrote: > > So this issue is blocking the LLVM upgrading on ChromeOS. Nathan, do > > you mind sending out the smaller patch like Nick suggested just to see > > what feedback we could get? I could send it for you if you are busy, > > and please let me know what tags I should use in that case. > > > > Thanks, > > Jian > > I will go ahead and send the smaller patch at some point today. > > For what it's worth, Nick brought up https://reviews.llvm.org/D100037, > which may be relevant here. > > Cheers, > Nathan > > > On Wed, Mar 31, 2021 at 3:06 PM Nick Desaulniers > > <ndesaulniers@google.com> wrote: > > > > > > On Wed, Mar 31, 2021 at 2:58 PM Nathan Chancellor <nathan@kernel.org> wrote: > > > > > > > > On Wed, Mar 31, 2021 at 02:27:03PM -0700, Jian Cai wrote: > > > > > > > > > > I just realized you already proposed solutions for skipping the check > > > > > in https://lore.kernel.org/linux-block/20210310225240.4epj2mdmzt4vurr3@archlinux-ax161/#t. > > > > > Do you have any plans to send them for review? > > > > > > > > I was hoping to gather some feedback on which option would be preferred > > > > by Jens and the other ClangBuiltLinux folks before I sent them along. I > > > > can send the first just to see what kind of feedback I can gather. > > > > > > Either approach is fine by me. The smaller might be easier to get > > > accepted into stable. The larger approach will probably become more > > > useful in the future (having the diag infra work properly with clang). > > > I think the ball is kind of in Jens' court to decide. Would doing > > > both be appropriate, Jens? Have the smaller patch tagged for stable > > > disabling it for the whole file, then another commit on top not tagged > > > for stable that adds the diag infra, and a third on top replacing the > > > file level warning disablement with local diags to isolate this down > > > to one case? It's a fair but small amount of churn IMO; but if Jens > > > is not opposed it seems fine? > > > -- > > > Thanks, > > > ~Nick Desaulniers ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] block: Disable -Walign-mismatch for blk-mq.c 2021-04-08 18:12 ` Nathan Chancellor 2021-04-08 18:42 ` Jian Cai @ 2021-04-08 19:44 ` Nathan Chancellor 2021-04-08 19:52 ` Guenter Roeck 1 sibling, 1 reply; 14+ messages in thread From: Nathan Chancellor @ 2021-04-08 19:44 UTC (permalink / raw) To: Jens Axboe Cc: Nick Desaulniers, Jian Cai, Guenter Roeck, Christopher Di Bella, Manoj Gupta, Luis Lozano, linux-block, linux-kernel, clang-built-linux, Nathan Chancellor 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 4ccafe032005 ("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: https://github.com/ClangBuiltLinux/linux/issues/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> --- block/Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/block/Makefile b/block/Makefile index 8d841f5f986f..d69ac0bd8e61 100644 --- a/block/Makefile +++ b/block/Makefile @@ -10,6 +10,7 @@ obj-$(CONFIG_BLOCK) := bio.o elevator.o blk-core.o blk-sysfs.o \ blk-mq-sysfs.o blk-mq-cpumap.o blk-mq-sched.o ioctl.o \ genhd.o ioprio.o badblocks.o partitions/ blk-rq-qos.o +CFLAGS_blk-mq.o := $(call cc-disable-warning, align-mismatch) obj-$(CONFIG_BOUNCE) += bounce.o obj-$(CONFIG_BLK_SCSI_REQUEST) += scsi_ioctl.o obj-$(CONFIG_BLK_DEV_BSG) += bsg.o base-commit: e49d033bddf5b565044e2abe4241353959bc9120 -- 2.31.1.189.g2e36527f23 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] block: Disable -Walign-mismatch for blk-mq.c 2021-04-08 19:44 ` [PATCH] block: Disable -Walign-mismatch for blk-mq.c Nathan Chancellor @ 2021-04-08 19:52 ` Guenter Roeck 0 siblings, 0 replies; 14+ messages in thread From: Guenter Roeck @ 2021-04-08 19:52 UTC (permalink / raw) To: Nathan Chancellor, Jens Axboe Cc: Nick Desaulniers, Jian Cai, Christopher Di Bella, Manoj Gupta, Luis Lozano, linux-block, linux-kernel, clang-built-linux On 4/8/21 12:44 PM, Nathan Chancellor wrote: > 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 4ccafe032005 ("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: https://github.com/ClangBuiltLinux/linux/issues/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> > --- > block/Makefile | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/block/Makefile b/block/Makefile > index 8d841f5f986f..d69ac0bd8e61 100644 > --- a/block/Makefile > +++ b/block/Makefile > @@ -10,6 +10,7 @@ obj-$(CONFIG_BLOCK) := bio.o elevator.o blk-core.o blk-sysfs.o \ > blk-mq-sysfs.o blk-mq-cpumap.o blk-mq-sched.o ioctl.o \ > genhd.o ioprio.o badblocks.o partitions/ blk-rq-qos.o > > +CFLAGS_blk-mq.o := $(call cc-disable-warning, align-mismatch) > obj-$(CONFIG_BOUNCE) += bounce.o > obj-$(CONFIG_BLK_SCSI_REQUEST) += scsi_ioctl.o > obj-$(CONFIG_BLK_DEV_BSG) += bsg.o > > base-commit: e49d033bddf5b565044e2abe4241353959bc9120 > ^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH] blk-mq: fix alignment mismatch. 2021-03-30 23:29 ` Nathan Chancellor 2021-03-31 0:26 ` Guenter Roeck @ 2021-03-31 10:38 ` David Laight 1 sibling, 0 replies; 14+ messages in thread From: David Laight @ 2021-03-31 10:38 UTC (permalink / raw) To: 'Nathan Chancellor', Jian Cai Cc: cjdb, manojgupta, llozano, clang-built-linux, Guenter Roeck, Jens Axboe, Nick Desaulniers, linux-block, linux-kernel From: Nathan Chancellor > Sent: 31 March 2021 00:30 > > Hi Jian, > > On Tue, Mar 30, 2021 at 04:02:49PM -0700, Jian Cai wrote: > > This fixes the mismatch of alignments between csd and its use as an > > argument to smp_call_function_single_async, which causes build failure > > when -Walign-mismatch in Clang is used. > > > > Link: > > http://crrev.com/c/1193732 > > > > Suggested-by: Guenter Roeck <linux@roeck-us.net> > > Signed-off-by: Jian Cai <jiancai@google.com> > > Thanks for the patch. This is effectively a revert of commit > 4ccafe032005 ("block: unalign call_single_data in struct request"), > which I had brought up in this thread: > > https://lore.kernel.org/r/20210310182307.zzcbi5w5jrmveld4@archlinux-ax161/ > > This is obviously a correct fix, I am not just sure what the impact to > 'struct request' will be. If the structure is allocated on-stack then aligning it requires the compiler generate the rather horrid 'double stack frame' for the function. Possibly the unaligned 'struct' should be used by all the code except for a few places where it makes sense to allocate an aligned item? David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales) ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2021-04-08 19:52 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-03-30 23:02 [PATCH] blk-mq: fix alignment mismatch Jian Cai 2021-03-30 23:12 ` Guenter Roeck 2021-03-30 23:29 ` Nathan Chancellor 2021-03-31 0:26 ` Guenter Roeck 2021-03-31 1:31 ` Jian Cai 2021-03-31 21:27 ` Jian Cai 2021-03-31 21:58 ` Nathan Chancellor 2021-03-31 22:06 ` Nick Desaulniers 2021-04-08 17:57 ` Jian Cai 2021-04-08 18:12 ` Nathan Chancellor 2021-04-08 18:42 ` Jian Cai 2021-04-08 19:44 ` [PATCH] block: Disable -Walign-mismatch for blk-mq.c Nathan Chancellor 2021-04-08 19:52 ` Guenter Roeck 2021-03-31 10:38 ` [PATCH] blk-mq: fix alignment mismatch David Laight
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.