Received-SPF: pass (google.com: domain of linux-kernel+bounces-168155-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <3beb08a6-0be7-4305-9ac5-2ccc9e6d02c5@linuxfoundation.org>
Date: Fri, 3 May 2024 13:55:00 -0600
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] selftests: default to host arch for LLVM builds
To: Valentin Obst <kernel@valentinobst.de>, jhubbard@nvidia.com
Cc: anders.roxell@linaro.org, bpoirier@nvidia.com, broonie@kernel.org,
 guillaume.tucker@collabora.com, linux-kernel@vger.kernel.org,
 linux-kselftest@vger.kernel.org, mpdesouza@suse.com, nathan@kernel.org,
 sashal@kernel.org, shuah@kernel.org, Shuah Khan <skhan@linuxfoundation.org>
References: <6d398f1d-6882-40fb-8ced-7fe6bee2aee7@nvidia.com>
 <20240428120806.19275-1-kernel@valentinobst.de>
Content-Language: en-US
From: Shuah Khan <skhan@linuxfoundation.org>
In-Reply-To: <20240428120806.19275-1-kernel@valentinobst.de>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

On 4/28/24 06:08, Valentin Obst wrote:
>>> Align the behavior for gcc and clang builds by interpreting unset
>>> `ARCH` and `CROSS_COMPILE` variables in `LLVM` builds as a sign that the
>>> user wants to build for the host architecture.
>>>
>>> This patch preserves the properties that setting the `ARCH` variable to an
>>> unknown value will trigger an error that complains about insufficient
>>> information, and that a set `CROSS_COMPILE` variable will override the
>>> target triple that is determined based on presence/absence of `ARCH`.
>>>
>>> When compiling with clang, i.e., `LLVM` is set, an unset `ARCH` variable in
>>> combination with an unset `CROSS_COMPILE` variable, i.e., compiling for
>>> the host architecture, leads to compilation failures since `lib.mk` can
>>> not determine the clang target triple. In this case, the following error
>>> message is displayed for each subsystem that does not set `ARCH` in its
>>> own Makefile before including `lib.mk` (lines wrapped at 75 chrs):
>>>
>>>     make[1]: Entering directory '/mnt/build/linux/tools/testing/selftests/
>>>      sysctl'
>>>     ../lib.mk:33: *** Specify CROSS_COMPILE or add '--target=' option to
>>>      lib.mk.  Stop.
>>>     make[1]: Leaving directory '/mnt/build/linux/tools/testing/selftests/
>>>      sysctl'
>>
>> Thanks for fixing this.
>>
>> And yes, the selftests "normal" (non-cross-compile) build is *broken*
>> right now, for clang. I didn't realize from the patch title that this is
>> actually a significant fix. Maybe we should change the subject line (patch
>> title) to something like:
>>
>>      [PATCH] selftests: fix the clang build: default to host arch for LLVM builds
> 
> Yes, I agree that the title should contain the word 'fix' somewhere. For
> me its okay if maintainers reword the title when applying the patch,
> alternatively I can send a v2. (Is it still a v2 if I change the title, or
> rather a new patch?).
> 
> Any thoughts on whether this also needs a 'Cc stable'? Its not quite
> clear to me if this fix meets the requirements. As above, no objections if
> maintainers should decide to add it.
> 
>>
>> ?
>>
>> Just a thought. The "Fixes:" tag covers it already, I realize.
>>
>> Anyway, this looks correct, and fixes that aspect of the build for me, so
>> either way, please feel free to add:
>>
>> Reviewed-by: John Hubbard <jhubbard@nvidia.com>
> 

Thanks for the patch. Applied to linux-kselftest next for Linux 6.10-rc1

thanks,
-- Shuah