linux-kernel - Re: [PATCH 4/4] Allow cgroup hierarchies to be created with no bound subsystems

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 23 Jul 2009 16:32:17 +0800
From:	Li Zefan <lizf@...fujitsu.com>
To:	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>
CC:	Paul Menage <menage@...gle.com>, akpm@...ux-foundation.org,
	containers@...ts.linux-foundation.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 4/4] Allow cgroup hierarchies to be created with no bound
 subsystems

>> -	/* We can't have an empty hierarchy */
>> +
>> +	/* Can't specify "none" and some subsystems */
>> +	if (opts->subsys_bits && opts->none)
>> +		return -EINVAL;
> 
> Is this case never happens ?
> 
> 	BUG_ON(!opts->subsys_bits && !opts->none)
> 

It can happen:

  # mount -t cgroup -o none,memory

> 
>> +
>> +	/*
>> +	 * We either have to specify by name or by subsystems. (So all
>> +	 * empty hierarchies must have a name).
>> +	 */
>>  	if (!opts->subsys_bits && !opts->name)
>>  		return -EINVAL;
>>  
>> @@ -1129,6 +1124,31 @@ static void init_cgroup_root(struct cgroupfs_root *root)
>>  	init_cgroup_housekeeping(cgrp);
>>  }
>>  
>> +static bool init_root_id(struct cgroupfs_root *root)
>> +{
>> +	int ret = 0;
>> +
>> +	do {
>> +		if (!ida_pre_get(&hierarchy_ida, GFP_KERNEL))
>> +			return false;
>> +		spin_lock(&hierarchy_id_lock);
>> +		/* Try to allocate the next unused ID */
>> +		ret = ida_get_new_above(&hierarchy_ida, next_hierarchy_id,
>> +					&root->hierarchy_id);
> 
> Why new id should be greater than old one ?
> Just for avoiding reuse-id-too-soon ?
> 

Otherwise we can't identify each hierachy by it's id.


>> +		if (ret == -ENOSPC)
>> +			/* Try again starting from 0 */
>> +			ret = ida_get_new(&hierarchy_ida, &root->hierarchy_id);
>> +		if (!ret) {
>> +			next_hierarchy_id = root->hierarchy_id + 1;
>> +		} else if (ret != -EAGAIN) {
>> +			/* Can only get here if the 31-bit IDR is full ... */
>> +			BUG_ON(ret);
>> +		}
>> +		spin_unlock(&hierarchy_id_lock);
>> +	} while (ret);
>> +	return true;
>> +}
>> +

...

>> @@ -3910,8 +3950,8 @@ static int current_css_set_cg_links_read(struct cgroup *cont,
>>  			name = c->dentry->d_name.name;
>>  		else
>>  			name = "?";
>> -		seq_printf(seq, "Root %lu group %s\n",
>> -			   c->root->subsys_bits, name);
>> +		seq_printf(seq, "Root %d group %s\n",
>> +			   c->root->hierarchy_id, name);
>>  	}
> 
> I'm not sure but this "id" is worth to be printed ?
> 

It's a debug file, so I think at least it's not bad to print the id.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/