Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: -Walign-mismatch in block/blk-mq.c
To:     Nathan Chancellor <nathan@kernel.org>
Cc:     linux-block@vger.kernel.org, linux-kernel@vger.kernel.org,
        clang-built-linux@googlegroups.com
References: <20210310182307.zzcbi5w5jrmveld4@archlinux-ax161>
 <99cf90ea-81c0-e110-4815-dd1f7df36cb4@kernel.dk>
 <20210310203323.35w2q7tlnxe23ukg@Ryzen-9-3900X.localdomain>
 <e43dba61-8c74-757d-862d-99d23559cf50@kernel.dk>
 <20210310205250.hpe4wcgn4yh3rjqz@archlinux-ax161>
From:   Jens Axboe <axboe@kernel.dk>
Message-ID: <9834f7fc-f4d2-2230-7e1f-9b607ea782de@kernel.dk>
Date:   Wed, 10 Mar 2021 14:03:56 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.10.0
MIME-Version: 1.0
In-Reply-To: <20210310205250.hpe4wcgn4yh3rjqz@archlinux-ax161>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

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