LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: Waiman Long <llong@redhat.com>
Cc: "Zefan Li" <lizefan.x@bytedance.com>,
	"Johannes Weiner" <hannes@cmpxchg.org>,
	"Jonathan Corbet" <corbet@lwn.net>,
	"Shuah Khan" <shuah@kernel.org>,
	cgroups@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-doc@vger.kernel.org, linux-kselftest@vger.kernel.org,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"Roman Gushchin" <guro@fb.com>, "Phil Auld" <pauld@redhat.com>,
	"Peter Zijlstra" <peterz@infradead.org>,
	"Juri Lelli" <juri.lelli@redhat.com>,
	"Frederic Weisbecker" <frederic@kernel.org>,
	"Marcelo Tosatti" <mtosatti@redhat.com>,
	"Michal Koutný" <mkoutny@suse.com>
Subject: Re: [PATCH v6 5/6] cgroup/cpuset: Update description of cpuset.cpus.partition in cgroup-v2.rst
Date: Tue, 24 Aug 2021 09:04:31 -1000	[thread overview]
Message-ID: <YSVCv0WjTzwPUWUN@slm.duckdns.org> (raw)
In-Reply-To: <95b72d36-32a9-8356-05b7-2829e4cc29ad@redhat.com>

Hello,

On Tue, Aug 24, 2021 at 01:35:33AM -0400, Waiman Long wrote:
> Sorry for the late reply as I was on vacation last week.

No worries. Hope you enjoyed the vacation. :)

> > All the above ultimately says is that "a new task cannot be moved to a
> > partition root with no effective cpu", but I don't understand why this would
> > be a separate rule. Shouldn't the partition just stop being a partition when
> > it doesn't have any exclusive cpu? What's the benefit of having multiple its
> > own failure mode?
>
> A partition with 0 cpu can be considered as a special partition type for
> spawning child partitions. This can be temporary as the cpus will be given
> back when a child partition is destroyed.

But it can also happen by cpus going offline while the partition is
populated, right? Am I correct in thinking that a partition without cpu is
valid if its subtree contains cpus and invalid otherwise? If that's the
case, it looks like the rules can be made significantly simpler. The parent
cgroups never have processes anyway, so a partition is valid if its subtree
contains cpus, invalid otherwise.

> > So, I think this definitely is a step in the right direction but still seems
> > to be neither here or there. Before, we pretended that we could police the
> > input when we couldn't. Now, we're changing the interface so that it
> > includes configuration failures as an integral part; however, we're still
> > policing some particular inputs while letting other inputs pass through and
> > trigger failures and why one is handled one way while the other differently
> > seems rather arbitrary.
> > 
> The cpu_exclusive and load_balance flags are attributes associated directly
> with the partition type. They are not affected by cpu availability or
> changing of cpu list. That is why they are kept even when the partition
> become invalid. If we have to remove them, it will be equivalent to changing
> partition back to member and we may not need an invalid partition type at
> all. Also, we will not be able to revert back to partition again when the
> cpus becomes available.

Oh, yeah, I'm not saying to lose those states. What I'm trying to say is
that the rules and failure modes seem a lot more complicated than they need
to be. If the configuration becomes invalid for whatever reason, transition
the partition into invalid state and report why. If the situation resolves
for whatever reason, transition it back to valid state. Shouldn't that work?

Thanks.

-- 
tejun

  reply	other threads:[~2021-08-24 19:04 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-14 20:57 [PATCH-cgroup v6 0/6] cgroup/cpuset: Add new cpuset partition type & empty effecitve cpus Waiman Long
2021-08-14 20:57 ` [PATCH v6 1/6] cgroup/cpuset: Properly transition to invalid partition Waiman Long
2021-08-14 20:57 ` [PATCH v6 2/6] cgroup/cpuset: Show invalid partition reason string Waiman Long
2021-08-14 20:57 ` [PATCH v6 3/6] cgroup/cpuset: Add a new isolated cpus.partition type Waiman Long
2021-08-14 20:57 ` [PATCH v6 4/6] cgroup/cpuset: Allow non-top parent partition to distribute out all CPUs Waiman Long
2021-08-14 20:57 ` [PATCH v6 5/6] cgroup/cpuset: Update description of cpuset.cpus.partition in cgroup-v2.rst Waiman Long
2021-08-16 17:08   ` Tejun Heo
2021-08-24  5:35     ` Waiman Long
2021-08-24 19:04       ` Tejun Heo [this message]
2021-08-25 19:21         ` Waiman Long
2021-08-25 19:24           ` Tejun Heo
2021-08-14 20:57 ` [PATCH v6 6/6] kselftest/cgroup: Add cpuset v2 partition root state test Waiman Long

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YSVCv0WjTzwPUWUN@slm.duckdns.org \
    --to=tj@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=cgroups@vger.kernel.org \
    --cc=corbet@lwn.net \
    --cc=frederic@kernel.org \
    --cc=guro@fb.com \
    --cc=hannes@cmpxchg.org \
    --cc=juri.lelli@redhat.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=lizefan.x@bytedance.com \
    --cc=llong@redhat.com \
    --cc=mkoutny@suse.com \
    --cc=mtosatti@redhat.com \
    --cc=pauld@redhat.com \
    --cc=peterz@infradead.org \
    --cc=shuah@kernel.org \
    --subject='Re: [PATCH v6 5/6] cgroup/cpuset: Update description of cpuset.cpus.partition in cgroup-v2.rst' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).