Re: [PATCH v2 01/39] xen/riscv: disable unnecessary configs

[Jan Beulich <jbeulich@xxxxxxxx>]

On 07.12.2023 10:22, Oleksii wrote:
> On Tue, 2023-12-05 at 16:38 +0100, Jan Beulich wrote:
>>> On 24.11.2023 11:30, Oleksii Kurochko wrote:
>>>>> The patch also fixes the build script as conf util expects
>>>>> to have each config on separate line.
>>>
>>> The approach doesn't really scale (it's already odd that you add
>>> the
>>> (apparently) same set four times. There's also zero justification
>>> for
>>> this kind of an adjustment (as per discussion elsewhere I don't
>>> think
>>> we should go this route, and hence arguments towards convincing me
>>> [and
>>> perhaps others] would be needed here).
> I agree that this may not be the best approach, but it seems like we
> don't have too many options to turn off a config for randconfig.
> 
> To be honest, in my opinion (IMO), randconfig should either be removed
> or allowed to fail until most of the functionality is ready. Otherwise,
> there should be a need to introduce HAS_* or depend on
> 'SUPPORTED_ARCHS' for each config, or introduce a lot of stubs.
> 
> Could you please suggest a better option?

As to dropping randconfig tests, I'd like to refer you to Andrew, who
is of the opinion that it was wrong to drop them for ppc. (I'm agreeing
with him when taking a theoretical perspective, but I'm not happy with
the practical consequences.)

As to a better approach: Instead of listing the same set of options
several times, can't there be a template config which is used to force
randconfig to not touch certain settings? In fact at least for non-
randconfig purposes I thought tiny64_defconfig / riscv64_defconfig
already serve kind of a similar purpose. Imo the EXTRA_*CONFIG overrides
are there for at most very few special case settings, not for purposes
like you use them here.

>>>>> --- a/automation/gitlab-ci/build.yaml
>>>>> +++ b/automation/gitlab-ci/build.yaml
>>>>> @@ -522,6 +522,38 @@ archlinux-current-gcc-riscv64:
>>>>>      CONTAINER: archlinux:current-riscv64
>>>>>      KBUILD_DEFCONFIG: tiny64_defconfig
>>>>>      HYPERVISOR_ONLY: y
>>>>> +    EXTRA_XEN_CONFIG:
>>>>> +      CONFIG_COVERAGE=n
>>>>> +      CONFIG_GRANT_TABLE=n
>>>>> +      CONFIG_SCHED_CREDIT=n
>>>>> +      CONFIG_SCHED_CREDIT2=n
>>>>> +      CONFIG_SCHED_RTDS=n
>>>>> +      CONFIG_SCHED_NULL=n
>>>>> +      CONFIG_SCHED_ARINC653=n
>>>>> +      CONFIG_TRACEBUFFER=n
>>>>> +      CONFIG_HYPFS=n
>>>>> +      CONFIG_GRANT_TABLE=n
>>>>> +      CONFIG_SPECULATIVE_HARDEN_ARRAY=n
>>>>> +      CONFIG_ARGO=n
>>>>> +      CONFIG_HYPFS_CONFIG=n
>>>>> +      CONFIG_CORE_PARKING=n
>>>>> +      CONFIG_DEBUG_TRACE=n
>>>>> +      CONFIG_IOREQ_SERVER=n
>>>>> +      CONFIG_CRASH_DEBUG=n
>>>>> +      CONFIG_KEXEC=n
>>>>> +      CONFIG_LIVEPATCH=n
>>>>> +      CONFIG_MEM_ACCESS=n
>>>>> +      CONFIG_NUMA=n
>>>>> +      CONFIG_PERF_COUNTERS=n
>>>>> +      CONFIG_HAS_PMAP=n
>>>>> +      CONFIG_TRACEBUFFER=n
>>>>> +      CONFIG_XENOPROF=n
>>>>> +      CONFIG_COMPAT=n
>>>>> +      CONFIG_COVERAGE=n
>>>>> +      CONFIG_UBSAN=n
>>>>> +      CONFIG_NEEDS_LIBELF=n
>>>>> +      CONFIG_XSM=n
>>>>> +
>>>>>  
>>>>>  archlinux-current-gcc-riscv64-debug:
>>>>>    extends: .gcc-riscv64-cross-build-debug
>>>
>>> I think I've said so elsewhere before: Please avoid introducing
>>> double
>>> blank lines, unless entirely unavoidable (for reasons I cannot
>>> think
>>> of).
> Sorry for that; I am not doing that on purpose. It's just a big patch
> series, and I missed that double blank. I will try to be more
> attentive.
> 
> Do you think it makes sense to add a script to perform basic code style
> checks, similar to what Linux has?

Such a script would be nice, but it doesn't exist and re-using existing
checkers has also proven controversial. There's actually an ongoing
discussion on this topic which you may want to follow.

Jan