[PATCH 0/9] cgroup v1 deprecation warnings

[PATCH 0/9] cgroup v1 deprecation warnings

[Michal Koutný]

Memory controller had begun to print warning messages when using some
attributes that do no have a counterpart in its cgroup v2
implementation. This is informative to users who run (unwittingly) on v1
or to distros that run v1 (they can learn about such users or prepare
for disabling v1 configs).

I consider the deprecated files in three categories:
  - RE) replacement exists,
  - DN) dropped as non-ideal concept (e.g. non-hierarchical resources),
  - NE) not evaluated (yet).

For RE, I added the replacement into the warning message, DN have only a
plain deprecation message and I marked the commits with NE as RFC.
Also I'd be happy if you would point out some forgotten knobs that'd
deserve similar warnings.
At the end are some cleanup patches I encountered en route.

Michal Koutný (9):
  cgroup/cpuset-v1: Add deprecation warnings to sched_load_balance and
    memory_pressure_enabled
  cgroup/cpuset-v1: Add deprecation warnings to memory_spread_page and
    memory_spread_slab
  cgroup/blkio: Add deprecation warnings to reset_stats
  cgroup: Print warning when /proc/cgroups is read on v2-only system
  RFC cgroup/cpuset-v1: Add deprecation warnings to mem_exclusive and
    mem_hardwall
  RFC cgroup/cpuset-v1: Add deprecation warnings to memory_migrate
  RFC cgroup/cpuset-v1: Add deprecation warnings to
    sched_relax_domain_level
  cgroup: Update file naming comment
  blk-cgroup: Simplify policy files registration

 block/blk-cgroup.c              |  8 ++++++--
 block/blk-ioprio.c              | 23 +++++++----------------
 include/linux/cgroup-defs.h     |  5 ++---
 include/linux/cgroup.h          |  1 +
 kernel/cgroup/cgroup-internal.h |  1 +
 kernel/cgroup/cgroup-v1.c       |  7 +++++++
 kernel/cgroup/cgroup.c          |  4 ++--
 kernel/cgroup/cpuset-v1.c       |  8 ++++++++
 8 files changed, 34 insertions(+), 23 deletions(-)


base-commit: 76544811c850a1f4c055aa182b513b7a843868ea
-- 
2.48.1

[PATCH 1/9] cgroup/cpuset-v1: Add deprecation warnings to sched_load_balance and memory_pressure_enabled

[Michal Koutný]

These two v1 feature have analogues in cgroup v2.

Signed-off-by: Michal Koutný <mkoutny@suse.com>
---
 kernel/cgroup/cpuset-v1.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/cgroup/cpuset-v1.c b/kernel/cgroup/cpuset-v1.c
index 25c1d7b77e2f2..3e81ac76578c7 100644
--- a/kernel/cgroup/cpuset-v1.c
+++ b/kernel/cgroup/cpuset-v1.c
@@ -430,12 +430,14 @@ static int cpuset_write_u64(struct cgroup_subsys_state *css, struct cftype *cft,
 		retval = cpuset_update_flag(CS_MEM_HARDWALL, cs, val);
 		break;
 	case FILE_SCHED_LOAD_BALANCE:
+		pr_warn_once("cpuset.%s is deprecated, use cpus.partition instead\n", cft->name);
 		retval = cpuset_update_flag(CS_SCHED_LOAD_BALANCE, cs, val);
 		break;
 	case FILE_MEMORY_MIGRATE:
 		retval = cpuset_update_flag(CS_MEMORY_MIGRATE, cs, val);
 		break;
 	case FILE_MEMORY_PRESSURE_ENABLED:
+		pr_warn_once("cpuset.%s is deprecated, use memory.pressure instead\n", cft->name);
 		cpuset_memory_pressure_enabled = !!val;
 		break;
 	case FILE_SPREAD_PAGE:
-- 
2.48.1

[PATCH 2/9] cgroup/cpuset-v1: Add deprecation warnings to memory_spread_page and memory_spread_slab

[Michal Koutný]

There is MPOL_INTERLEAVE for user explicit allocations.
Deprecate spreading of allocations that users carry out unwittingly.

Signed-off-by: Michal Koutný <mkoutny@suse.com>
---
 kernel/cgroup/cpuset-v1.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/cgroup/cpuset-v1.c b/kernel/cgroup/cpuset-v1.c
index 3e81ac76578c7..9aae6dabb0b56 100644
--- a/kernel/cgroup/cpuset-v1.c
+++ b/kernel/cgroup/cpuset-v1.c
@@ -441,9 +441,11 @@ static int cpuset_write_u64(struct cgroup_subsys_state *css, struct cftype *cft,
 		cpuset_memory_pressure_enabled = !!val;
 		break;
 	case FILE_SPREAD_PAGE:
+		pr_warn_once("cpuset.%s is deprecated\n", cft->name);
 		retval = cpuset_update_flag(CS_SPREAD_PAGE, cs, val);
 		break;
 	case FILE_SPREAD_SLAB:
+		pr_warn_once("cpuset.%s is deprecated\n", cft->name);
 		retval = cpuset_update_flag(CS_SPREAD_SLAB, cs, val);
 		break;
 	default:
-- 
2.48.1

[PATCH 3/9] cgroup/blkio: Add deprecation warnings to reset_stats

[Michal Koutný]

It is difficult to sync with stat updaters, stats are (should be)
monotonic so users can calculate differences from a reference.

Signed-off-by: Michal Koutný <mkoutny@suse.com>
---
 block/blk-cgroup.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index 9ed93d91d754a..b77219dd8b061 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -659,6 +659,7 @@ static int blkcg_reset_stats(struct cgroup_subsys_state *css,
 	struct blkcg_gq *blkg;
 	int i;
 
+	pr_warn_once("blkio.%s is deprecated\n", cftype->name);
 	mutex_lock(&blkcg_pol_mutex);
 	spin_lock_irq(&blkcg->lock);
 
-- 
2.48.1

[PATCH 4/9] cgroup: Print warning when /proc/cgroups is read on v2-only system

[Michal Koutný]

As a followup to commits 6c2920926b10e ("cgroup: replace
unified-hierarchy.txt with a proper cgroup v2 documentation") and
ab03125268679 ("cgroup: Show # of subsystem CSSes in cgroup.stat"),
add a runtime message to users who read status of controllers in
/proc/cgroups on v2-only system. The detection is based on a)
no controllers are attached to v1, b) default hierarchy is mounted (the
latter is for setups that neven mount v2 but read /proc/cgroups upon
boot when controllers default to v2).

Signed-off-by: Michal Koutný <mkoutny@suse.com>
---
 kernel/cgroup/cgroup-internal.h | 1 +
 kernel/cgroup/cgroup-v1.c       | 7 +++++++
 kernel/cgroup/cgroup.c          | 2 +-
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/kernel/cgroup/cgroup-internal.h b/kernel/cgroup/cgroup-internal.h
index c964dd7ff967a..95ab39e1ec8f0 100644
--- a/kernel/cgroup/cgroup-internal.h
+++ b/kernel/cgroup/cgroup-internal.h
@@ -168,6 +168,7 @@ struct cgroup_mgctx {
 
 extern struct cgroup_subsys *cgroup_subsys[];
 extern struct list_head cgroup_roots;
+extern bool cgrp_dfl_visible;
 
 /* iterate across the hierarchies */
 #define for_each_root(root)						\
diff --git a/kernel/cgroup/cgroup-v1.c b/kernel/cgroup/cgroup-v1.c
index e28d5f0d20ed0..5c59b01024019 100644
--- a/kernel/cgroup/cgroup-v1.c
+++ b/kernel/cgroup/cgroup-v1.c
@@ -673,6 +673,7 @@ struct cftype cgroup1_base_files[] = {
 int proc_cgroupstats_show(struct seq_file *m, void *v)
 {
 	struct cgroup_subsys *ss;
+	bool cgrp_v1_visible = false;
 	int i;
 
 	seq_puts(m, "#subsys_name\thierarchy\tnum_cgroups\tenabled\n");
@@ -684,12 +685,18 @@ int proc_cgroupstats_show(struct seq_file *m, void *v)
 	for_each_subsys(ss, i) {
 		if (cgroup1_subsys_absent(ss))
 			continue;
+		cgrp_v1_visible |= ss->root != &cgrp_dfl_root;
+
 		seq_printf(m, "%s\t%d\t%d\t%d\n",
 			   ss->legacy_name, ss->root->hierarchy_id,
 			   atomic_read(&ss->root->nr_cgrps),
 			   cgroup_ssid_enabled(i));
 	}
 
+	if (cgrp_dfl_visible && !cgrp_v1_visible)
+		pr_warn_once("/proc/cgroups lists only v1 controllers, use cgroup.controllers of root cgroup for v2 info\n");
+
+
 	return 0;
 }
 
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index afc665b7b1fe5..3a5af0fc544a6 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -171,7 +171,7 @@ EXPORT_SYMBOL_GPL(cgrp_dfl_root);
  * The default hierarchy always exists but is hidden until mounted for the
  * first time.  This is for backward compatibility.
  */
-static bool cgrp_dfl_visible;
+bool cgrp_dfl_visible;
 
 /* some controllers are not supported in the default hierarchy */
 static u16 cgrp_dfl_inhibit_ss_mask;
-- 
2.48.1

[PATCH 5/9] RFC cgroup/cpuset-v1: Add deprecation warnings to mem_exclusive and mem_hardwall

[Michal Koutný]

The concept of exclusive memory affinity may require complex approaches
like with cpuset v2 cpu partitions. There is so far no implementation in
cpuset v2.
Specific kernel memory affinity may cause unintended (global)
bottlenecks like kmem limits.

Cc: Paul Menage <menage@google.com>
Cc: Paul Jackson <pj@sgi.com>
Signed-off-by: Michal Koutný <mkoutny@suse.com>
---
 kernel/cgroup/cpuset-v1.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/cgroup/cpuset-v1.c b/kernel/cgroup/cpuset-v1.c
index 9aae6dabb0b56..05d3622ea41e5 100644
--- a/kernel/cgroup/cpuset-v1.c
+++ b/kernel/cgroup/cpuset-v1.c
@@ -424,9 +424,11 @@ static int cpuset_write_u64(struct cgroup_subsys_state *css, struct cftype *cft,
 		retval = cpuset_update_flag(CS_CPU_EXCLUSIVE, cs, val);
 		break;
 	case FILE_MEM_EXCLUSIVE:
+		pr_warn_once("cpuset.%s is deprecated\n", cft->name);
 		retval = cpuset_update_flag(CS_MEM_EXCLUSIVE, cs, val);
 		break;
 	case FILE_MEM_HARDWALL:
+		pr_warn_once("cpuset.%s is deprecated\n", cft->name);
 		retval = cpuset_update_flag(CS_MEM_HARDWALL, cs, val);
 		break;
 	case FILE_SCHED_LOAD_BALANCE:
-- 
2.48.1

[PATCH 6/9] RFC cgroup/cpuset-v1: Add deprecation warnings to memory_migrate

[Michal Koutný]

Memory migration (between cgroups) was given up in v2 due to performance
reasons of its implementation. Migration between NUMA nodes within one
memcg may still make sense to modify affinity at runtime though.

Cc: Paul Jackson <pj@sgi.com>
Signed-off-by: Michal Koutný <mkoutny@suse.com>
---
 kernel/cgroup/cpuset-v1.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/cgroup/cpuset-v1.c b/kernel/cgroup/cpuset-v1.c
index 05d3622ea41e5..6155d890f10a4 100644
--- a/kernel/cgroup/cpuset-v1.c
+++ b/kernel/cgroup/cpuset-v1.c
@@ -436,6 +436,7 @@ static int cpuset_write_u64(struct cgroup_subsys_state *css, struct cftype *cft,
 		retval = cpuset_update_flag(CS_SCHED_LOAD_BALANCE, cs, val);
 		break;
 	case FILE_MEMORY_MIGRATE:
+		pr_warn_once("cpuset.%s is deprecated\n", cft->name);
 		retval = cpuset_update_flag(CS_MEMORY_MIGRATE, cs, val);
 		break;
 	case FILE_MEMORY_PRESSURE_ENABLED:
-- 
2.48.1

[PATCH 7/9] RFC cgroup/cpuset-v1: Add deprecation warnings to sched_relax_domain_level

[Michal Koutný]

This is not a properly hierarchical resource, it might be better
implemented based on a sched_attr.

Cc: Hidetoshi Seto <seto.hidetoshi@jp.fujitsu.com>
Signed-off-by: Michal Koutný <mkoutny@suse.com>
---
 kernel/cgroup/cpuset-v1.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/cgroup/cpuset-v1.c b/kernel/cgroup/cpuset-v1.c
index 6155d890f10a4..ada6fcdffe0b5 100644
--- a/kernel/cgroup/cpuset-v1.c
+++ b/kernel/cgroup/cpuset-v1.c
@@ -175,6 +175,7 @@ static int cpuset_write_s64(struct cgroup_subsys_state *css, struct cftype *cft,
 
 	switch (type) {
 	case FILE_SCHED_RELAX_DOMAIN_LEVEL:
+		pr_warn_once("cpuset.%s is deprecated\n", cft->name);
 		retval = update_relax_domain_level(cs, val);
 		break;
 	default:
-- 
2.48.1

[PATCH 8/9] cgroup: Update file naming comment

[Michal Koutný]

This changed long time ago in commit 8d7e6fb0a1db9 ("cgroup: update
cgroup name handling").

Signed-off-by: Michal Koutný <mkoutny@suse.com>
---
 include/linux/cgroup-defs.h | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h
index 17960a1e858db..561a9022ec100 100644
--- a/include/linux/cgroup-defs.h
+++ b/include/linux/cgroup-defs.h
@@ -619,9 +619,8 @@ struct cgroup_root {
  */
 struct cftype {
 	/*
-	 * By convention, the name should begin with the name of the
-	 * subsystem, followed by a period.  Zero length string indicates
-	 * end of cftype array.
+	 * name of the subsystem is prepended in cgroup_file_name().
+	 * Zero length string indicates end of cftype array.
 	 */
 	char name[MAX_CFTYPE_NAME];
 	unsigned long private;
-- 
2.48.1

[PATCH 9/9] blk-cgroup: Simplify policy files registration

[Michal Koutný]

Use one set of files when there is no difference between default and
legacy files, similar to regular subsys files registration. No
functional change.

Signed-off-by: Michal Koutný <mkoutny@suse.com>
---
 block/blk-cgroup.c     |  7 +++++--
 block/blk-ioprio.c     | 23 +++++++----------------
 include/linux/cgroup.h |  1 +
 kernel/cgroup/cgroup.c |  2 +-
 4 files changed, 14 insertions(+), 19 deletions(-)

diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index b77219dd8b061..db6adc8a7ff41 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -1771,12 +1771,15 @@ int blkcg_policy_register(struct blkcg_policy *pol)
 	mutex_unlock(&blkcg_pol_mutex);
 
 	/* everything is in place, add intf files for the new policy */
-	if (pol->dfl_cftypes)
+	if (pol->dfl_cftypes == pol->legacy_cftypes) {
+		WARN_ON(cgroup_add_cftypes(&io_cgrp_subsys,
+					   pol->dfl_cftypes));
+	} else {
 		WARN_ON(cgroup_add_dfl_cftypes(&io_cgrp_subsys,
 					       pol->dfl_cftypes));
-	if (pol->legacy_cftypes)
 		WARN_ON(cgroup_add_legacy_cftypes(&io_cgrp_subsys,
 						  pol->legacy_cftypes));
+	}
 	mutex_unlock(&blkcg_pol_register_mutex);
 	return 0;
 
diff --git a/block/blk-ioprio.c b/block/blk-ioprio.c
index 8fff7ccc0ac73..13659dc15c3ff 100644
--- a/block/blk-ioprio.c
+++ b/block/blk-ioprio.c
@@ -113,27 +113,18 @@ static void ioprio_free_cpd(struct blkcg_policy_data *cpd)
 	kfree(blkcg);
 }
 
-#define IOPRIO_ATTRS						\
-	{							\
-		.name		= "prio.class",			\
-		.seq_show	= ioprio_show_prio_policy,	\
-		.write		= ioprio_set_prio_policy,	\
-	},							\
-	{ } /* sentinel */
-
-/* cgroup v2 attributes */
 static struct cftype ioprio_files[] = {
-	IOPRIO_ATTRS
-};
-
-/* cgroup v1 attributes */
-static struct cftype ioprio_legacy_files[] = {
-	IOPRIO_ATTRS
+	{
+		.name		= "prio.class",
+		.seq_show	= ioprio_show_prio_policy,
+		.write		= ioprio_set_prio_policy,
+	},
+	{ } /* sentinel */
 };
 
 static struct blkcg_policy ioprio_policy = {
 	.dfl_cftypes	= ioprio_files,
-	.legacy_cftypes = ioprio_legacy_files,
+	.legacy_cftypes = ioprio_files,
 
 	.cpd_alloc_fn	= ioprio_alloc_cpd,
 	.cpd_free_fn	= ioprio_free_cpd,
diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
index f8ef47f8a634d..8e7415c64ed1d 100644
--- a/include/linux/cgroup.h
+++ b/include/linux/cgroup.h
@@ -113,6 +113,7 @@ int cgroup_transfer_tasks(struct cgroup *to, struct cgroup *from);
 
 int cgroup_add_dfl_cftypes(struct cgroup_subsys *ss, struct cftype *cfts);
 int cgroup_add_legacy_cftypes(struct cgroup_subsys *ss, struct cftype *cfts);
+int cgroup_add_cftypes(struct cgroup_subsys *ss, struct cftype *cfts);
 int cgroup_rm_cftypes(struct cftype *cfts);
 void cgroup_file_notify(struct cgroup_file *cfile);
 void cgroup_file_show(struct cgroup_file *cfile, bool show);
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 3a5af0fc544a6..e93b0563a8964 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -4447,7 +4447,7 @@ int cgroup_rm_cftypes(struct cftype *cfts)
  * function currently returns 0 as long as @cfts registration is successful
  * even if some file creation attempts on existing cgroups fail.
  */
-static int cgroup_add_cftypes(struct cgroup_subsys *ss, struct cftype *cfts)
+int cgroup_add_cftypes(struct cgroup_subsys *ss, struct cftype *cfts)
 {
 	int ret;
 
-- 
2.48.1

Re: [PATCH 1/9] cgroup/cpuset-v1: Add deprecation warnings to sched_load_balance and memory_pressure_enabled

[Waiman Long]

On 3/4/25 10:37 AM, Michal Koutný wrote:
> These two v1 feature have analogues in cgroup v2.
>
> Signed-off-by: Michal Koutný <mkoutny@suse.com>
> ---
>   kernel/cgroup/cpuset-v1.c | 2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/kernel/cgroup/cpuset-v1.c b/kernel/cgroup/cpuset-v1.c
> index 25c1d7b77e2f2..3e81ac76578c7 100644
> --- a/kernel/cgroup/cpuset-v1.c
> +++ b/kernel/cgroup/cpuset-v1.c
> @@ -430,12 +430,14 @@ static int cpuset_write_u64(struct cgroup_subsys_state *css, struct cftype *cft,
>   		retval = cpuset_update_flag(CS_MEM_HARDWALL, cs, val);
>   		break;
>   	case FILE_SCHED_LOAD_BALANCE:
> +		pr_warn_once("cpuset.%s is deprecated, use cpus.partition instead\n", cft->name);
Should you use the full name "cpuset.cpus.partition" instead?
>   		retval = cpuset_update_flag(CS_SCHED_LOAD_BALANCE, cs, val);
>   		break;
>   	case FILE_MEMORY_MIGRATE:
>   		retval = cpuset_update_flag(CS_MEMORY_MIGRATE, cs, val);
>   		break;
>   	case FILE_MEMORY_PRESSURE_ENABLED:
> +		pr_warn_once("cpuset.%s is deprecated, use memory.pressure instead\n", cft->name);

memory.pressure depends on CONFIG_PSI, so it may not available in some 
cases. My suggestion is to add a "if available" suffix.

I do have some concern with the use of pr_warn*() because some users may 
attempt to use the panic_on_warn command line option.

Cheers,
Longman

Re: [PATCH 1/9] cgroup/cpuset-v1: Add deprecation warnings to sched_load_balance and memory_pressure_enabled

[Tejun Heo]

On Tue, Mar 04, 2025 at 11:19:00AM -0500, Waiman Long wrote:
...
> I do have some concern with the use of pr_warn*() because some users may
> attempt to use the panic_on_warn command line option.

Yeah, let's print these as info.

Thanks.

-- 
tejun

Re: [PATCH 4/9] cgroup: Print warning when /proc/cgroups is read on v2-only system

[Tejun Heo]

On Tue, Mar 04, 2025 at 04:37:56PM +0100, Michal Koutný wrote:
> As a followup to commits 6c2920926b10e ("cgroup: replace
> unified-hierarchy.txt with a proper cgroup v2 documentation") and
> ab03125268679 ("cgroup: Show # of subsystem CSSes in cgroup.stat"),
> add a runtime message to users who read status of controllers in
> /proc/cgroups on v2-only system. The detection is based on a)
> no controllers are attached to v1, b) default hierarchy is mounted (the
> latter is for setups that neven mount v2 but read /proc/cgroups upon
> boot when controllers default to v2).

I'm hoping that we could deprecate /proc/cgroups entirely. Maybe we can just
warn whenever the file is accessed?

Thanks.

-- 
tejun

Re: [PATCH 1/9] cgroup/cpuset-v1: Add deprecation warnings to sched_load_balance and memory_pressure_enabled

[Michal Koutný]

[-- Attachment #1: Type: text/plain, Size: 530 bytes --]

On Tue, Mar 04, 2025 at 06:52:41AM -1000, Tejun Heo <tj@kernel.org> wrote:
> On Tue, Mar 04, 2025 at 11:19:00AM -0500, Waiman Long wrote:
> ...
> > I do have some concern with the use of pr_warn*() because some users may
> > attempt to use the panic_on_warn command line option.
> 
> Yeah, let's print these as info.

The intention is _not_ to cause panic by any of this this.
Note the difference between WARN() and pr_warn() (only the former
panics).
Warn level has precedent in mm/memcontrol-v1.c already.

Michal

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/9] cgroup/cpuset-v1: Add deprecation warnings to sched_load_balance and memory_pressure_enabled

[Waiman Long]

On 3/4/25 12:10 PM, Michal Koutný wrote:
> On Tue, Mar 04, 2025 at 06:52:41AM -1000, Tejun Heo <tj@kernel.org> wrote:
>> On Tue, Mar 04, 2025 at 11:19:00AM -0500, Waiman Long wrote:
>> ...
>>> I do have some concern with the use of pr_warn*() because some users may
>>> attempt to use the panic_on_warn command line option.
>> Yeah, let's print these as info.
> The intention is _not_ to cause panic by any of this this.
> Note the difference between WARN() and pr_warn() (only the former
> panics).
> Warn level has precedent in mm/memcontrol-v1.c already.

I think you are right. The pr_warn() function should not cause a panic. 
I have the misconception that pr_warn() will be affected by 
panic_on_warn before. In that case, I have no objection to use pr_warn().

Thanks,
Longman

Re: [PATCH 1/9] cgroup/cpuset-v1: Add deprecation warnings to sched_load_balance and memory_pressure_enabled

[Tejun Heo]

On Tue, Mar 04, 2025 at 12:33:32PM -0500, Waiman Long wrote:
> 
> On 3/4/25 12:10 PM, Michal Koutný wrote:
> > On Tue, Mar 04, 2025 at 06:52:41AM -1000, Tejun Heo <tj@kernel.org> wrote:
> > > On Tue, Mar 04, 2025 at 11:19:00AM -0500, Waiman Long wrote:
> > > ...
> > > > I do have some concern with the use of pr_warn*() because some users may
> > > > attempt to use the panic_on_warn command line option.
> > > Yeah, let's print these as info.
> > The intention is _not_ to cause panic by any of this this.
> > Note the difference between WARN() and pr_warn() (only the former
> > panics).
> > Warn level has precedent in mm/memcontrol-v1.c already.
> 
> I think you are right. The pr_warn() function should not cause a panic. I
> have the misconception that pr_warn() will be affected by panic_on_warn
> before. In that case, I have no objection to use pr_warn().

I'm apprehensive about adding warning messages which may be triggered
consistently without anything end users can do about them. I think that
deprecation messages, unless such deprecation is immediate and would have
direct consequences on how the system can be used, should be informational.

Thanks.

-- 
tejun

Re: [PATCH 1/9] cgroup/cpuset-v1: Add deprecation warnings to sched_load_balance and memory_pressure_enabled

[Michal Koutný]

[-- Attachment #1: Type: text/plain, Size: 735 bytes --]

On Tue, Mar 04, 2025 at 08:04:06AM -1000, Tejun Heo <tj@kernel.org> wrote:
> I'm apprehensive about adding warning messages which may be triggered
> consistently without anything end users can do about them.

That means you'd distinguish RE (replacement exists) vs DN (dropped as
non-ideal) categories?


> I think that deprecation messages, unless such deprecation is
> immediate and would have direct consequences on how the system can be
> used, should be informational.

I could subscribe to that if there weren't so many other places to
evaluate:
  $ git grep -i "pr_warn.*deprec" torvalds/master --  | wc -l
  62
  $ git grep -i "pr_info.*deprec" torvalds/master --  | wc -l
  2

So is the disctinction worth the hassle?

Michal

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 4/9] cgroup: Print warning when /proc/cgroups is read on v2-only system

[Michal Koutný]

[-- Attachment #1: Type: text/plain, Size: 503 bytes --]

On Tue, Mar 04, 2025 at 06:55:02AM -1000, Tejun Heo <tj@kernel.org> wrote:
> I'm hoping that we could deprecate /proc/cgroups entirely. Maybe we can just
> warn whenever the file is accessed?

I added the guard with legacy systems (i.e. make this backportable) in
mind which start with no cgroupfs mounted at all and until they decide
to continue either v1 or v2 way, looking at /proc/cgroups is fine.
It should warn users that look at /proc/cgroups in cases when it bears
no valid information.

Michal

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 4/9] cgroup: Print warning when /proc/cgroups is read on v2-only system

[Waiman Long]

On 3/5/25 5:17 AM, Michal Koutný wrote:
> On Tue, Mar 04, 2025 at 06:55:02AM -1000, Tejun Heo <tj@kernel.org> wrote:
>> I'm hoping that we could deprecate /proc/cgroups entirely. Maybe we can just
>> warn whenever the file is accessed?
> I added the guard with legacy systems (i.e. make this backportable) in
> mind which start with no cgroupfs mounted at all and until they decide
> to continue either v1 or v2 way, looking at /proc/cgroups is fine.
> It should warn users that look at /proc/cgroups in cases when it bears
> no valid information.

I like the idea of warning users about using /etc/cgroups if no v1 
filesystem is mounted. It makes sense to make it backportable to older 
kernel. We can always add a follow-up patch later to always warn about it.

Acked-by: Waiman Long <longman@redhat.com>

Re: [PATCH 1/9] cgroup/cpuset-v1: Add deprecation warnings to sched_load_balance and memory_pressure_enabled

[Tejun Heo]

Hello,

On Wed, Mar 05, 2025 at 11:12:21AM +0100, Michal Koutný wrote:
> On Tue, Mar 04, 2025 at 08:04:06AM -1000, Tejun Heo <tj@kernel.org> wrote:
> > I'm apprehensive about adding warning messages which may be triggered
> > consistently without anything end users can do about them.
> 
> That means you'd distinguish RE (replacement exists) vs DN (dropped as
> non-ideal) categories?

I don't think I am. I'm just concerned about emitting warn messages on every
boot without users being able to do anything about them.

> > I think that deprecation messages, unless such deprecation is
> > immediate and would have direct consequences on how the system can be
> > used, should be informational.
> 
> I could subscribe to that if there weren't so many other places to
> evaluate:
>   $ git grep -i "pr_warn.*deprec" torvalds/master --  | wc -l
>   62
>   $ git grep -i "pr_info.*deprec" torvalds/master --  | wc -l
>   2
> 
> So is the disctinction worth the hassle?

Well, not all deprecations are the same. If users are stuck on cgroup1, they
can be really stuck - there can be a tall stack of software with
dependencies that users can't do much about, at least not immediately. We
will deprecate cgroup1 but this is going to be a long stretched out process
at the end of which we should be fairly comfortable in stating that there
aren't major users left which are stuck on cgroup1.

It's almost certain that that future won't arrive in, say, three years. Five
years may be too ambitious too but let's say that at that point we are
relatively sure that most platforms have moved on (but there may still be
users on older versions of those platforms). Maybe it'd make sense to
increase the deprecation warning temperature by then to warn and drain
existing users and maybe after a few years we'd actually be able to drop
cgroup1 support.

So, I don't want to be emitting warnings on every boot for the good part of
a decade on every boot for those users. Doing so feels silly and annoying to
me. Let's inform that it's coming down the pipeline but I personally don't
want to be warned by something that's close to a decade out.

Thanks.

-- 
tejun