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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 24 Oct 2022, Michal Orzel wrote:
> 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.

Yes, sounds good


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

Yeah