Re: [PATCH] cgroup: remove redundant NULL assignments in migration finish

[Michal Koutný]

Hello Qingye.

On Tue, Feb 24, 2026 at 10:36:23AM +0000, zhaoqingye <zhaoqingye@xxxxxxxxx> wrote:
> cgroup_migrate_finish() currently sets cset->mg_src_cgrp, cset->mg_dst_cgrp
> and cset->mg_dst_cset to NULL when cleaning mgctx->preloaded_dst_csets.
> 
> These assignments are redundant for the css_sets on
> mgctx->preloaded_dst_csets:
> 
> - There are only three places that modify the mg_* members of a css_set:
>   - cgroup_migrate_add_src(), which sets src_cset->mg_src_cgrp
>   - cgroup_migrate_prepare_dst(), which clears src_cset->mg_src_cgrp when
>     src_cset and dst_cset happen to be the same
>   - cgroup_migrate_finish(), which clears mg_src_cgrp for css_sets on
>     mgctx->preloaded_src_csets and mgctx->preloaded_dst_csets
> 
> - All three functions are invoked through the migration sequence:
>   cgroup_migrate_add_src() ->
>   cgroup_migrate_prepare_dst() ->
>   cgroup_migrate_add_task() ->
>   cgroup_migrate_execute() ->
>   cgroup_migrate_finish()
> 
>   All migration entry points (cgroup_attach_task(),
>   cgroup_update_dfl_csses() and cgroup_transfer_tasks()) hold
>   cgroup_mutex across the whole sequence: cgroup_mutex is acquired before
>   cgroup_migrate_add_src() and only released after cgroup_migrate_finish()
>   returns. This rules out concurrent updates to the mg_* members.
> 
> - During a single migration, a given css_set cannot be on both
>   mgctx->preloaded_src_csets and mgctx->preloaded_dst_csets at the same
>   time. For css_sets on mgctx->preloaded_dst_csets, mg_src_cgrp,
>   mg_dst_cgrp and mg_dst_cset are never assigned and therefore remain NULL
>   for the entire migration.

This is impressive exercise! However, from defensive programming PoV I'd
keep the cgroup_migrate_finish() as is. (Otherwise the text above would
need to be added to the preloaded_src_sets processing to explain the
disparity.)

> As a result, explicitly setting these fields to NULL again in
> cgroup_migrate_finish() for css_sets on mgctx->preloaded_dst_csets does not
> change any observable state. Removing the redundant assignments makes the
> migration state handling clearer without changing behavior.

Despite being redundant, they pair with some WARN_ONs around and the
runtime benefit here is negligible.

Thanks,
Michal

Attachment: signature.asc
Description: PGP signature