Re: [PATCH 2/5] cgroup/cpuset: Add new cpus.partition type with no load balancing

On 6/16/21 4:47 PM, Tejun Heo wrote:

Hello,

Generally looks fine to me.

On Thu, Jun 03, 2021 at 05:24:13PM -0400, Waiman Long wrote:

...

@@ -1984,12 +1987,31 @@ static int update_prstate(struct cpuset *cs, int val) goto out; err = update_parent_subparts_cpumask(cs, partcmd_enable,

• 				     NULL, &tmp);
  

• 				     NULL, &tmpmask);
  

• if (err) { update_flag(CS_CPU_EXCLUSIVE, cs, 0); goto out;

• } else if (new_prs == PRS_ENABLED_NOLB) {
  

• 	/*
  

• 	 * Disable the load balance flag should not return an
  

                              ^ing

and "else if" after "if (err) goto out" block is weird. The two conditions don't need to be tied together.

Yes, the else part is redundant in this case. Will remove it.

...

@@ -2518,6 +2547,9 @@ static int sched_partition_show(struct seq_file *seq, void *v) case PRS_ENABLED: seq_puts(seq, "root\n"); break;

• case PRS_ENABLED_NOLB:

• seq_puts(seq, "root-nolb\n");
  

• break;
  

  case PRS_DISABLED: seq_puts(seq, "member\n"); break;

@@ -2544,6 +2576,8 @@ static ssize_t sched_partition_write(struct kernfs_open_file *of, char *buf, val = PRS_ENABLED; else if (!strcmp(buf, "member")) val = PRS_DISABLED;

• else if (!strcmp(buf, "root-nolb"))

• val = PRS_ENABLED_NOLB;
  

  else return -EINVAL;

I wonder whether there's a better name than "root-nolb" because nolb isn't the most readable and we are using space as the delimiter for other names. Would something like "isolated" work?

Right. "isolated" is a better name and it corresponds better with the isolcpus kernel command line option. Will change the name.

Thanks, Longman