[Devel] [PATCH vz10] ve: set proper VE state on ve_create() error

[Konstantin Khorenko]

On 27.08.2025 04:53, Pavel Tikhomirov wrote:
> 
> 
> On 8/25/25 20:41, Konstantin Khorenko wrote:
>> If we fail to alloc ve_struct, we'll crash here on setting VE state.
>>
>> Fixes: 666e40b308457 ("ve/cgroups: Drop lock when stopping workqueue to
>> avoid dead lock")
>> Signed-off-by: Konstantin Khorenko <khorenko at virtuozzo.com>
>>
>> Feature: ve: ve generic structures
>> ---
>>    kernel/ve/ve.c | 2 +-
>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/kernel/ve/ve.c b/kernel/ve/ve.c
>> index 146f7922d4856..1d7e04bb7f3fb 100644
>> --- a/kernel/ve/ve.c
>> +++ b/kernel/ve/ve.c
>> @@ -1028,9 +1028,9 @@ static struct cgroup_subsys_state *ve_create(struct cgroup_subsys_state *parent_
>>    err_log:
>>    	free_percpu(ve->sched_lat_ve.cur);
>>    err_lat:
>> +	ve_set_state(ve, VE_STATE_DEAD);
>>    	kmem_cache_free(ve_cachep, ve);
>>    err_ve:
>> -	ve_set_state(ve, VE_STATE_STOPPED);
> 
> Why do we change s/VE_STATE_STOPPED/VE_STATE_DEAD/ ? As far as I can see
> VE_STATE_DEAD is for destroyed cgroup only. I don't think this change is
> correct.

It's be more logical to see _DEAD state in the freed/not used memory and it corresponds to other cases 
when VE_STATE_DEAD is set:

     ve_start_container()
     err handling:
             ve_set_state(ve, VE_STATE_STOPPED);
             ve_drop_context(ve); // not freeing memory

     ve_exit_ns
             ve_set_state(ve, VE_STATE_STOPPED);
             put_ve(ve); /* from ve_start_container() */
             // no memory free here, only later in ve_destroy()

     ve_destroy
             ve_set_state(ve, VE_STATE_DEAD);
             kmem_cache_free(ve_cachep, ve); // freeing memory

     ve_create
     err handling:
             ve_set_state(ve, VE_STATE_DEAD); // logical to use _DEAD here
             kmem_cache_free(ve_cachep, ve);  // freeing memory as well


> Other thing, the label "err_ve" after this patch is only doing return,
> codding style dictates that we remove it and return directly instead of
> jumping to this label.

Yep, you are right, fixed in the v2, thank you.

>   >  If there is no cleanup needed then just return directly.
> 
> https://www.kernel.org/doc/html/v4.10/process/coding-style.html#centralized-exiting-of-functions
> 
>>    	return ERR_PTR(err);
>>    }