On Wed, Mar 10, 2021 at 01:40:25PM -0700, Jens Axboe wrote:

On 3/10/21 1:33 PM, Nathan Chancellor wrote:

quoted

On Wed, Mar 10, 2021 at 01:21:52PM -0700, Jens Axboe wrote:

quoted

On 3/10/21 11:23 AM, Nathan Chancellor wrote:

quoted

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@kernel.dk/ (local)

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.

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).

there doesn't seem to be an issue with just having it wherever in struct
request.

Cheers,
Nathan