Messages in this thread | | | From | Waiman Long <> | Subject | Re: [PATCH 2/5] cgroup/cpuset: Add new cpus.partition type with no load balancing | Date | Wed, 16 Jun 2021 22:57:18 -0400 |
| |
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
| |