Re: [PATCH v14 14/14] selftests/sgx: Add scripts for EPC cgroup testing

[Jarkko Sakkinen]

On Thu Jun 6, 2024 at 1:30 AM EEST, Huang, Kai wrote:
>
> >> Reorg:
> >>
> >> void sgx_cgroup_init(void)
> >> {
> >>     struct workqueue_struct *wq;
> >>
> >>     /* eagerly allocate the workqueue: */
> >>     wq = alloc_workqueue("sgx_cg_wq", wq_unbound | wq_freezable, 
> >> wq_unbound_max_active);
> >>     if (!wq) {
> >>         pr_warn("sgx_cg_wq creation failed\n");
> >>         return;
> > 
> > sgx_cgroup_try_charge() expects sgx_cg_wq, so it would break unless we 
> > check and return 0 which was the initially implemented in v12. But then 
> > Kai had some concern on that we expose all the interface files to allow 
> > user to set limits but we don't enforce. To keep it simple we settled 
> > down back to BUG_ON(). 
>
> [...]
>
> > This would only happen rarely and user can add 
> > command-line to disable SGX if s/he really wants to start kernel in this 
> > case, just can't do SGX.
>
> Just to be clear that I don't like BUG_ON() either.  It's inevitable you 
> will get attention because of using it.

Just then plain disable SGX if it fails to start.

Do not take down the whole system.

> This is a compromise that you don't want to reset "capacity" to 0 when 
> alloc_workqueue() fails.

BUG_ON() is not a "compromise".

> There are existing code where BUG_ON() is used during the kernel early 
> boot code when memory allocation fails (e.g., see cgroup_init_subsys()), 
> so it might be acceptable to use BUG_ON() here, but it's up to 
> maintainers to decide whether it is OK.

When it is not possible continue to run the system at all, and only
then.

Here it is possible. Without SGX.

BR, Jarkko