Re: [PATCH v1 3/3] cgroup: add lock guard support for others

[Jemmy Wong <jemmywong512@gmail.com>]

Hi Peter, Dam, Ingo,

Do you have any recommendations to eliminate this false positive, given that sparse recognizes scoped_guard but not guard?

Looks like false positives. Some of these warnings were already present 
due to asymmetric lock/unlock behavior in a specific function,

while others were introduced by the use of guard, 
which can be resolved by replacing it with scoped_guard, 
but I don’t like this solution which could add additional indentation.

——

Previous warnings - caused by asymmetric lock/unlock:

kernel/cgroup/cgroup.c:3161:9: sparse: warning: context imbalance in 'cgroup_lock_and_drain_offline' - wrong count at exit
kernel/cgroup/cgroup.c:6530:12: sparse: warning: context imbalance in 'cgroup_css_set_fork' - wrong count at exit
kernel/cgroup/cgroup.c:6646:9: sparse: warning: context imbalance in 'cgroup_css_set_put_fork' - wrong count at exit
kernel/cgroup/cgroup.c:6666:5: sparse: warning: context imbalance in 'cgroup_can_fork' - wrong count at exit
kernel/cgroup/cgroup.c:6715:9: sparse: warning: context imbalance in 'cgroup_cancel_fork' - unexpected unlock

New warnings - caused by guard:

kernel/cgroup/cgroup.c:345:13: sparse: sparse: context imbalance in 'cgroup_idr_replace' - wrong count at exit
kernel/cgroup/cgroup.c:351:13: sparse: sparse: context imbalance in 'cgroup_idr_remove' - wrong count at exit
kernel/cgroup/cgroup.c:626:5: sparse: sparse: context imbalance in 'cgroup_task_count' - wrong count at exit
kernel/cgroup/cgroup.c:2225:9: sparse: sparse: context imbalance in 'cgroup_do_get_tree' - different lock contexts for basic block
kernel/cgroup/cgroup.c:2418:5: sparse: sparse: context imbalance in 'cgroup_path_ns' - wrong count at exit
kernel/cgroup/cgroup.c:2734:9: sparse: sparse: context imbalance in 'cgroup_migrate_finish' - wrong count at exit
kernel/cgroup/cgroup.c:4532:9: sparse: sparse: context imbalance in 'cgroup_file_notify' - wrong count at exit
kernel/cgroup/cgroup.c:4994:9: sparse: sparse: context imbalance in 'css_task_iter_start' - wrong count at exit
kernel/cgroup/cgroup.c:5027:9: sparse: sparse: context imbalance in 'css_task_iter_next' - wrong count at exit
kernel/cgroup/cgroup.c:5047:9: sparse: sparse: context imbalance in 'css_task_iter_end' - wrong count at exit

Best,
Jemmy

> On Jun 7, 2025, at 6:50 PM, kernel test robot <lkp@intel.com> wrote:
> 
> Hi Jemmy,
> 
> kernel test robot noticed the following build warnings:
> 
> [auto build test WARNING on tj-cgroup/for-next]
> [also build test WARNING on linus/master next-20250606]
> [cannot apply to v6.15]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Jemmy-Wong/cgroup-add-lock-guard-support-for-cgroup_muetx/20250607-002109
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git for-next
> patch link:    https://lore.kernel.org/r/20250606161841.44354-4-jemmywong512%40gmail.com
> patch subject: [PATCH v1 3/3] cgroup: add lock guard support for others
> config: sparc-randconfig-r121-20250607 (https://download.01.org/0day-ci/archive/20250607/202506071822.ERv4i34r-lkp@intel.com/config)
> compiler: sparc-linux-gcc (GCC) 8.5.0
> reproduce: (https://download.01.org/0day-ci/archive/20250607/202506071822.ERv4i34r-lkp@intel.com/reproduce)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202506071822.ERv4i34r-lkp@intel.com/
> 
> sparse warnings: (new ones prefixed by >>)
>>> kernel/cgroup/cgroup.c:6721:33: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected struct spinlock [usertype] *l @@     got struct spinlock [noderef] __rcu * @@
>   kernel/cgroup/cgroup.c:6721:33: sparse:     expected struct spinlock [usertype] *l
>   kernel/cgroup/cgroup.c:6721:33: sparse:     got struct spinlock [noderef] __rcu *
>   kernel/cgroup/cgroup.c:345:13: sparse: sparse: context imbalance in 'cgroup_idr_replace' - wrong count at exit
>   kernel/cgroup/cgroup.c:351:13: sparse: sparse: context imbalance in 'cgroup_idr_remove' - wrong count at exit
>   kernel/cgroup/cgroup.c:626:5: sparse: sparse: context imbalance in 'cgroup_task_count' - wrong count at exit
>   kernel/cgroup/cgroup.c:2225:9: sparse: sparse: context imbalance in 'cgroup_do_get_tree' - different lock contexts for basic block
>   kernel/cgroup/cgroup.c:2418:5: sparse: sparse: context imbalance in 'cgroup_path_ns' - wrong count at exit
>   kernel/cgroup/cgroup.c:2734:9: sparse: sparse: context imbalance in 'cgroup_migrate_finish' - wrong count at exit
>   kernel/cgroup/cgroup.c:3131:9: sparse: sparse: context imbalance in 'cgroup_lock_and_drain_offline' - wrong count at exit
>   kernel/cgroup/cgroup.c:4532:9: sparse: sparse: context imbalance in 'cgroup_file_notify' - wrong count at exit
>   kernel/cgroup/cgroup.c:4994:9: sparse: sparse: context imbalance in 'css_task_iter_start' - wrong count at exit
>   kernel/cgroup/cgroup.c:5027:9: sparse: sparse: context imbalance in 'css_task_iter_next' - wrong count at exit
>   kernel/cgroup/cgroup.c:5047:9: sparse: sparse: context imbalance in 'css_task_iter_end' - wrong count at exit
>   kernel/cgroup/cgroup.c:6485:12: sparse: sparse: context imbalance in 'cgroup_css_set_fork' - wrong count at exit
>   kernel/cgroup/cgroup.c:6601:9: sparse: sparse: context imbalance in 'cgroup_css_set_put_fork' - wrong count at exit
>   kernel/cgroup/cgroup.c:6621:5: sparse: sparse: context imbalance in 'cgroup_can_fork' - wrong count at exit
>   kernel/cgroup/cgroup.c:6670:9: sparse: sparse: context imbalance in 'cgroup_cancel_fork' - unexpected unlock
>   kernel/cgroup/cgroup.c:6813:9: sparse: sparse: context imbalance in 'cgroup_release' - different lock contexts for basic block
> 
> vim +6721 kernel/cgroup/cgroup.c
> 
>  6672 
>  6673 /**
>  6674 * cgroup_post_fork - finalize cgroup setup for the child process
>  6675 * @child: the child process
>  6676 * @kargs: the arguments passed to create the child process
>  6677 *
>  6678 * Attach the child process to its css_set calling the subsystem fork()
>  6679 * callbacks.
>  6680 */
>  6681 void cgroup_post_fork(struct task_struct *child,
>  6682      struct kernel_clone_args *kargs)
>  6683 __releases(&cgroup_threadgroup_rwsem) __releases(&cgroup_mutex)
>  6684 {
>  6685 unsigned int cgrp_kill_seq = 0;
>  6686 unsigned long cgrp_flags = 0;
>  6687 bool kill = false;
>  6688 struct cgroup_subsys *ss;
>  6689 struct css_set *cset;
>  6690 int i;
>  6691 
>  6692 cset = kargs->cset;
>  6693 kargs->cset = NULL;
>  6694 
>  6695 scoped_guard(spinlock_irq, &css_set_lock) {
>  6696 /* init tasks are special, only link regular threads */
>  6697 if (likely(child->pid)) {
>  6698 if (kargs->cgrp) {
>  6699 cgrp_flags = kargs->cgrp->flags;
>  6700 cgrp_kill_seq = kargs->cgrp->kill_seq;
>  6701 } else {
>  6702 cgrp_flags = cset->dfl_cgrp->flags;
>  6703 cgrp_kill_seq = cset->dfl_cgrp->kill_seq;
>  6704 }
>  6705 
>  6706 WARN_ON_ONCE(!list_empty(&child->cg_list));
>  6707 cset->nr_tasks++;
>  6708 css_set_move_task(child, NULL, cset, false);
>  6709 } else {
>  6710 put_css_set(cset);
>  6711 cset = NULL;
>  6712 }
>  6713 
>  6714 if (!(child->flags & PF_KTHREAD)) {
>  6715 if (unlikely(test_bit(CGRP_FREEZE, &cgrp_flags))) {
>  6716 /*
>  6717 * If the cgroup has to be frozen, the new task has
>  6718 * too. Let's set the JOBCTL_TRAP_FREEZE jobctl bit to
>  6719 * get the task into the frozen state.
>  6720 */
>> 6721 scoped_guard(spinlock, &child->sighand->siglock) {
>  6722 WARN_ON_ONCE(child->frozen);
>  6723 child->jobctl |= JOBCTL_TRAP_FREEZE;
>  6724 }
>  6725 
>  6726 /*
>  6727 * Calling cgroup_update_frozen() isn't required here,
>  6728 * because it will be called anyway a bit later from
>  6729 * do_freezer_trap(). So we avoid cgroup's transient
>  6730 * switch from the frozen state and back.
>  6731 */
>  6732 }
>  6733 
>  6734 /*
>  6735 * If the cgroup is to be killed notice it now and take the
>  6736 * child down right after we finished preparing it for
>  6737 * userspace.
>  6738 */
>  6739 kill = kargs->kill_seq != cgrp_kill_seq;
>  6740 }
>  6741 }
>  6742 /*
>  6743 * Call ss->fork().  This must happen after @child is linked on
>  6744 * css_set; otherwise, @child might change state between ->fork()
>  6745 * and addition to css_set.
>  6746 */
>  6747 do_each_subsys_mask(ss, i, have_fork_callback) {
>  6748 ss->fork(child);
>  6749 } while_each_subsys_mask();
>  6750 
>  6751 /* Make the new cset the root_cset of the new cgroup namespace. */
>  6752 if (kargs->flags & CLONE_NEWCGROUP) {
>  6753 struct css_set *rcset = child->nsproxy->cgroup_ns->root_cset;
>  6754 
>  6755 get_css_set(cset);
>  6756 child->nsproxy->cgroup_ns->root_cset = cset;
>  6757 put_css_set(rcset);
>  6758 }
>  6759 
>  6760 /* Cgroup has to be killed so take down child immediately. */
>  6761 if (unlikely(kill))
>  6762 do_send_sig_info(SIGKILL, SEND_SIG_NOINFO, child, PIDTYPE_TGID);
>  6763 
>  6764 cgroup_css_set_put_fork(kargs);
>  6765 }
>  6766 
> 
> -- 
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki