On 3/10/21 1:52 PM, Nathan Chancellor wrote: > On Wed, Mar 10, 2021 at 01:40:25PM -0700, Jens Axboe wrote: >> On 3/10/21 1:33 PM, Nathan Chancellor wrote: >>> On Wed, Mar 10, 2021 at 01:21:52PM -0700, Jens Axboe wrote: >>>> On 3/10/21 11:23 AM, Nathan Chancellor wrote: >>>>> Hi Jens, >>>>> >>>>> There is a new clang warning added in the development branch, >>>>> -Walign-mismatch, which shows an instance in block/blk-mq.c: >>>>> >>>>> 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. >>>>> >>>>> There appears to be some history here as I can see that this member was >>>>> purposefully unaligned in commit 4ccafe032005 ("block: unalign >>>>> call_single_data in struct request"). However, I later see a change in >>>>> commit 7c3fb70f0341 ("block: rearrange a few request fields for better >>>>> cache layout") that seems somewhat related. Is it possible to get back >>>>> the alignment by rearranging the structure again? This seems to be the >>>>> only solution for the warning aside from just outright disabling it, >>>>> which would be a shame since it seems like it could be useful for >>>>> architectures that cannot handle unaligned accesses well, unless I am >>>>> missing something obvious :) >>>> >>>> It should not be hard to ensure that alignment without re-introducing >>>> the bloat. Is there some background on why 32-byte alignment is >>>> required? >>>> >>> >>> This alignment requirement was introduced in commit 966a967116e6 ("smp: >>> Avoid using two cache lines for struct call_single_data") and it looks >>> like there was a thread between you and Peter Zijlstra that has some >>> more information on this: >>> https://lore.kernel.org/r/a9beb452-7344-9e2d-fc80-094d8f5a0394@xxxxxxxxx/ >> >> Ah now I remember - so it's not that it _needs_ to be 32-byte aligned, >> it's just a handy way to ensure that it doesn't straddle two cachelines. >> In fact, there's no real alignment concern, outside of performance >> reasons we don't want it touching two cachelines. >> >> So... what exactly is your concern? Just silencing that warning? Because > > Yes, dealing with the warning in some way is my only motivation. My > apologies, I should have led with that. I had assumed that this would > potentially be an issue due to the warning's text and that rearranging > the structure might allow the alignment to be added back but if there is > not actually a problem, then the warning should be silenced in some way. Right, that's what I was getting at, but I needed to page that context back in, it had long since been purged :-) > I am not sure if there is a preferred way to silence it (CFLAGS_... or > some of the __diag() infrastructure in include/linux/compiler_types.h). That's a good question, I'm not sure what the best approach here would be. Funnily enough, on my build, it just so happens to be 32-byte aligned anyway, but that's by mere chance. -- Jens Axboe