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

Reply via email to