Re: [for-4.17] automation: Do not use null scheduler for boot cpupools test

[Michal Orzel <michal.orzel@xxxxxxx>]

Replying to all,

On 21/10/2022 21:36, Stefano Stabellini wrote:
> 
> 
> On Fri, 21 Oct 2022, Andrew Cooper wrote:
>> On 21/10/2022 17:53, Michal Orzel wrote:
>>> Null scheduler is not enabled on non-debug Xen builds so the current
>>> test can lead to a failure on such jobs. We still want to test that we
>>> can assign the cpupool to a domU with a different scheduler than default
>>> one (credit2). Switch to credit as it is enabled by default.
>>>
>>> Fixes: 36e3f4158778 ("automation: Add a new job for testing boot time 
>>> cpupools on arm64")
>>> Signed-off-by: Michal Orzel <michal.orzel@xxxxxxx>
>>
>> /sigh - I'm sure I nacked that stupidity to begin with.  apparently not...
>>
>> It is totally bogus for CONFIG_DEBUG to influence logical chunks of
>> functionality like this.  The CI script is good in its current form.
>>
>> RTDS and ARINC should be default n.
>>
>> NULL is more tricky.  PV_SHIM is explicitly security supported, and has
>> been for years, so the "UNSUPPORTED" is bogus, whatever the default is.
>>
>> As NULL is explicitly tested in CI, it's clearly supported, and probably
>> ought to be on default.
>>
>>
>> Please instead fix Kconfig to not be broken.  That will be a far better
>> fix overall for people.
>>
>> As a more general note, tests which are using non-default pieces of
>> logic ought to activate them explicitly.
> 
> 
> I agree with you, but first let me clarify the word "supported".
> 
> 
> In Xen Project "supported" implies extra efforts to follow the security
> process and of course the security team should be on board with it. If
> we say "supported, non security supported" we don't need to follow the
> security process but still we sign up for backporting fixes to the
> stable tree. It is less extra effort but still some extra effort is
> involved.
> 
> So, this specific issue aside, I think that as we expand the testing
> capabilities of gitlab-ci, we'll have tests for things that are not
> necessarily neither "supported" nor "supported, non security supported".
> 
> 
> For the NULL scheduler, it is clearly important to many users so it
> would be valuable to move it to "supported, non security supported" and
> enabling it by default in the build. I don't recall if we still have any
> known outstanding issues with it. I think we need a separate email
> thread for that discussion and I would understand if the decision is not
> to change NULL support status for the 4.17 release (maybe for the 4.18
> release?).
> 
> 
> In any case, we don't need CONFIG_DEBUG to enable CONFIG_UNSUPPORTED. It
> is just that UNSUPPORTED and NULL don't get enabled by default in the
> non-DEBUG build. So to fix gitlab-ci, we can simply enable
> CONFIG_UNSUPPORTED explicitly for the builds where we need it
> (alpine-3.12-gcc-arm64-boot-cpupools).

Given that there are still diverging opinions \wrt making use of DEBUG
to influence enabling/disabling some functionalities in the code, I would
opt for modifying the CI job to explicitly specify the required config options,
just like I did for static-mem test. The necessary options to enable NULL are:
CONFIG_EXPERT=y
CONFIG_UNSUPPORTED=y
CONFIG_SCHED_NULL=y

This will fix the issue and allow us to continue with 4.17 release.
Given the outstanding issues reported by Julien, it would be challenging to
try to mark the NULL scheduler as supported, not security supported for this 
release.

Besides that, I think that Andrew still has a valid point. We seem to use DEBUG
only in Kconfig.debug (obvious choice) and sched/Kconfig. So this is not 
something
common to rely on DEBUG to enable logical functionalities (why did we make this 
exception for schedulers?).
Having said that, I think the discussion on whether to switch to default n
instead of default DEBUG or not is still valid and requires more people to give 
feedback.

~Michal