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

["Huang, Kai" <kai.huang@intel.com>]

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

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

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.

[...]

>> With "--strict" flag I also catched these:
>>
>> CHECK: spinlock_t definition without comment
>> #1308: FILE: arch/x86/kernel/cpu/sgx/sgx.h:122:
>> +    spinlock_t lock;
>>
> Yes I had a comment but Kai thought it was too obvious and I can't think 
> of a better one that's not obvious so I removed:
> 
> https://lore.kernel.org/linux-sgx/9ffb02a3344807f2c173fe8c7cb000cd6c7843b6.camel@intel.com/
>
To be clear, my reply was really about the comment itself isn't useful, 
but didn't say we shouldn't use a comment here.

> 
>> CHECK: multiple assignments should be avoided
>> #444: FILE: kernel/cgroup/misc.c:450:
>> +        parent_cg = cg = &root_cg;
>>
> 
> This was also suggested by Kai a few versions back:
> https://lore.kernel.org/linux-sgx/8f08f0b0f2b04b90d7cdb7b628f16f9080687c43.camel@intel.com/
> 

I didn't know checkpatch complains this.  Feel free to revert back as it 
is trivial to me.