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