From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mta1.migadu.com (out-20.mta1.migadu.com [95.215.58.20]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id B214623AB9D for ; Fri, 4 Sep 2026 01:55:23 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=95.215.58.20 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1788486925; cv=none; b=f9r11u3xLvBN/ZpTCUBjhQ1yZqT8DtoE18J8a8649YO/PBpuVXSUYrSUbYxZqX3R0XjsZFnO9za1OE928RAX5zPFNHn3OeBnnKCzNIjdByZp6aLzdwjlS/sXrrTn2USKPU7XYdspu3XrSfRvA4xwGqaDselkx41KftrfP51Ry9g= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1788486925; c=relaxed/simple; bh=DVuNyZ7EXHoYVxNEMvSHfqDkBER0yL8vohLELNvu600=; h=Message-ID:Date:MIME-Version:Subject:From:To:Cc:References: In-Reply-To:Content-Type; b=IOADTiPsdByhCK5zelVL3+4JOYJCh6FxlCvWjS2N8kws7ousWaNdM8BLFLIIJjkPt1TQY+L27HP3887WrDo/C0EPWr0ZfVIeLxvmEi4Kz1RcrMkPnRCKABQ0F+EeaMCipVE/tm39kg1rwrlTSXdRMHf02p3iK3NqSm75ke1GQ5c= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=GZjtowSh; arc=none smtp.client-ip=95.215.58.20 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="GZjtowSh" X-Envelope-To: linux-kselftest@vger.kernel.org DKIM-Signature: a=rsa-sha256; bh=DVuNyZ7EXHoYVxNEMvSHfqDkBER0yL8vohLELNvu600=; c=simple/simple; d=linux.dev; h=from:to:subject:date:message-id:mime-version:content-type; s=key1; t=1788486921; v=1; x=1789091721; b=GZjtowSh0OaeMmOsq55r5ox0KdPuhP5lQTcG27ZsH3dbw1vC3y7m9M8pnx/59djLdhNcu2BZ p1bjWdfCTgJwQtJrDJH4km41N0vPdOqZiLKDywfPU3E6jy2/Xs0sx8L5nwiPCyDbg9QLVlXLmg7 1igFmU2ehZrPejucCS0SpLo8= X-Envelope-To: linux-kselftest@vger.kernel.org Received: by smtp.migadu.com with ESMTPS id 86530bbb6701146c; Fri, 04 Sep 2026 01:55:20 +0000 X-Mizu-Trace-ID: 86530bbb6701146c X-Migadu-Flow: FLOW_OUT Message-ID: <7d1f1c95-5dad-4124-858c-7c8e3119f437@linux.dev> Date: Fri, 4 Sep 2026 09:55:14 +0800 Precedence: bulk X-Mailing-List: linux-kselftest@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v3 1/7] cgroup/cpuset: Factor out child partition validation From: Ridong Chen To: Waiman Long , Guopeng Zhang , cgroups@vger.kernel.org Cc: tj@kernel.org, hannes@cmpxchg.org, mkoutny@suse.com, shuah@kernel.org, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org, Guopeng Zhang References: <20260902102615.79189-1-guopeng.zhang@linux.dev> <20260902102615.79189-2-guopeng.zhang@linux.dev> <5842edcd-9283-4a87-afc9-50e51962368e@redhat.com> <5e0d1386-b094-4d4e-8569-e6b584f955c5@linux.dev> In-Reply-To: <5e0d1386-b094-4d4e-8569-e6b584f955c5@linux.dev> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 9/4/2026 9:25 AM, Ridong Chen wrote: > > > On 9/4/2026 2:33 AM, Waiman Long wrote: >> On 9/2/26 6:26 AM, Guopeng Zhang wrote: >>> From: Guopeng Zhang >>> >>> compute_partition_effective_cpumask() checks whether each valid child >>> partition remains covered by the parent exclusive CPU mask and whether it >>> would consume all remaining CPUs of a populated parent. >>> >>> Factor these two checks into child_partition_error() so the same rules can >>> be reused when evaluating a proposed parent configuration. This is a >>> preparatory refactoring with no intended functional change. >>> >>> Signed-off-by: Guopeng Zhang >>> --- >>>   kernel/cgroup/cpuset.c | 38 +++++++++++++++++++++++++++++--------- >>>   1 file changed, 29 insertions(+), 9 deletions(-) >>> >>> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c >>> index 8f24171b6055..6994dc75d940 100644 >>> --- a/kernel/cgroup/cpuset.c >>> +++ b/kernel/cgroup/cpuset.c >>> @@ -2085,6 +2085,26 @@ static int update_parent_effective_cpumask(struct >>> cpuset *cs, int cmd, >>>       return 0; >>>   } >>> +/* >>> + * Return the error that will invalidate a child partition under a proposed >>> + * parent partition configuration. >>> + */ >>> +static enum prs_errcode >>> +child_partition_error(struct cpuset *child, >>> +              const struct cpumask *partition_cpus, >>> +              const struct cpumask *remaining_cpus, >>> +              bool parent_populated) >> I think you should add some functional comments on what "partition_cpus" and >> "remaining_cpus" are supposed to be so that caller knows what to pass into >> this helper. > > Would it help to rename them to excpus and local_excpus? > local already implies "excluding children" (just like cgroup.stat.local), so I > think that makes the intent clearer. > Regarding the naming: child_partition_error is a bit odd — it sounds like it's validating a hierarchical child partition, but it's actually validating the partition itself (the child argument). The parent is only needed as context for the validation logic, not because we're checking a subordinate partition. I'd suggest renaming it to something like: ``` static enum prs_errcode cs_partition_error(struct cpuset *cs, ...) ``` That would better reflect what the function actually does. On a related note, the current partition validation logic is scattered across the code. I think we should consider consolidating it into a common set of helpers, perhaps like: ``` static enum prs_errcode validate_partition(struct cpuset *cs, struct cpuset *trialcs, struct cpuset *parent) { ... } static enum prs_errcode validate_local_partition(struct cpuset *cs, struct cpuset *trialcs, struct cpuset *parent) { // local partion specific check // call validate_partition } static enum prs_errcode validate_remote_partition(struct cpuset *cs, struct cpuset *trialcs, struct cpuset *parent) { // remote partion specific check // call validate_partition } ``` >>> +{ >>> +    if (!cpumask_subset(child->effective_xcpus, partition_cpus)) >>> +        return PERR_INVCPUS; >>> + >>> +    if (parent_populated && >>> +        cpumask_subset(remaining_cpus, child->effective_xcpus)) >>> +        return PERR_NOCPUS; >>> + >>> +    return PERR_NONE; >>> +} >>> + >>>   /** >>>    * compute_partition_effective_cpumask - compute effective_cpus for partition >>>    * @cs: partition root cpuset >>> @@ -2121,6 +2141,8 @@ static void compute_partition_effective_cpumask(struct >>> cpuset *cs, >>>       rcu_read_lock(); >>>       cpuset_for_each_child(child, css, cs) { >>> +        enum prs_errcode child_err; >>> + >>>           if (!is_partition_valid(child)) >>>               continue; >>> @@ -2129,15 +2151,13 @@ static void >>> compute_partition_effective_cpumask(struct cpuset *cs, >>>            * partition root. >>>            */ >>>           WARN_ON_ONCE(is_remote_partition(child)); >>> -        WRITE_ONCE(child->prs_err, 0); >>> -        if (!cpumask_subset(child->effective_xcpus, >>> -                    cs->effective_xcpus)) >>> -            WRITE_ONCE(child->prs_err, PERR_INVCPUS); >>> -        else if (populated && >>> -             cpumask_subset(new_ecpus, child->effective_xcpus)) >>> -            WRITE_ONCE(child->prs_err, PERR_NOCPUS); >>> - >>> -        if (child->prs_err) { >>> +        WRITE_ONCE(child->prs_err, PERR_NONE); >>> +        child_err = child_partition_error(child, cs->effective_xcpus, >>> +                          new_ecpus, populated); >>> +        if (child_err) >>> +            WRITE_ONCE(child->prs_err, child_err); >>> + >> >> You can ignore the inital PERR_NONE write and always write the child_err value >> into child->prs_err. >> >> Not big issue, just some nits. >> >> Cheers, >> Longman >> >>> +        if (child_err) { >>>               int old_prs = child->partition_root_state; >>>               /* >> > -- Best regards Ridong