No subject

the smallest unused ID. I want to cycle through increasing IDs until
we hit the end of the range, and then start again at 0.

>
> and I think we should check EAGAIN:

OK, I've changed this to handle EAGAIN. I don't see any value in
handling ENOSPC though - we're not going to fill up a 31-bit IDR.

>>
>> -     if (!opts->subsys_bits)
>> +     if (!opts->subsys_bits && !opts->none)
>>               return ERR_PTR(-EINVAL);
>>
>
> Shouldn't we check this in parse_cgroupfs_options() instead?

No, becase at that point it's valid - we can request the root named X
without specifying what subsystems X is already mounted with. But if
we don't find a root named X, then we're creating a new root, and we
have to have specified a set of subsystems, or else explicitly "none".

>>
>> +static void cgroup_drop_root(struct cgroupfs_root *root)
>> +{
>> +     BUG_ON(!root->hierarchy_id);
>
> I think this BUG_ON() is redundant, if root->hierarchy_id == 0,
> we are freeing the dummy root, and kfree(root) should trigger
> kernel oops.

It's more obvious this way.

>> +             seq_printf(seq, "css_set %p\n", cg);
>> +             list_for_each_entry_safe(task, saved_task, &cg->tasks,
>> +                                      cg_list) {
>> +                     if (count++ > MAX_TASKS_SHOWN_PER_CSS) {
>
> actually this prints at most 26 tasks but not 25

Not really a big deal :-)

>
> and what's the concern to not print out all the tasks? the buffer
> limit of seqfile?

Yes.

>
>> +                             seq_puts(seq, "  ...\n");
>> +                             break;
>> +                     } else {
>> +                             seq_printf(seq, "  task %d\n", task->pid);
>
> I guess we should call task_pid_vnr(tsk) instead of accessing task->pid
> directly.

Changed.

Paul