netdev - Re: [PATCH v4 4/8] cgroup: Remove CGROUP_BUILTIN_SUBSYS

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]

Message-ID: <50517FFF.4030106@huawei.com>
Date:	Thu, 13 Sep 2012 14:41:03 +0800
From:	Li Zefan <lizefan@...wei.com>
To:	Daniel Wagner <wagi@...om.org>
CC:	<netdev@...r.kernel.org>, <cgroups@...r.kernel.org>,
	Daniel Wagner <daniel.wagner@...-carit.de>,
	Gao feng <gaofeng@...fujitsu.com>,
	Jamal Hadi Salim <jhs@...atatu.com>,
	John Fastabend <john.r.fastabend@...el.com>,
	Neil Horman <nhorman@...driver.com>
Subject: Re: [PATCH v4 4/8] cgroup: Remove CGROUP_BUILTIN_SUBSYS_COUNT

> @@ -1321,11 +1321,13 @@ static int parse_cgroupfs_options(char *data, struct cgroup_sb_opts *opts)
>  	 * take duplicate reference counts on a subsystem that's already used,
>  	 * but rebind_subsystems handles this case.
>  	 */
> -	for (i = CGROUP_BUILTIN_SUBSYS_COUNT; i < CGROUP_SUBSYS_COUNT; i++) {
> +	for (i = 0; i < CGROUP_SUBSYS_COUNT; i++) {
>  		unsigned long bit = 1UL << i;
>  
>  		if (!(bit & opts->subsys_mask))
>  			continue;
> +		if (!subsys[i]->module)
> +			continue;

This check is not necessary. If it's builtin, try_module_get() will just return 1, and
we're fine.

>  		if (!try_module_get(subsys[i]->module)) {
>  			module_pin_failed = true;
>  			break;
> @@ -1337,12 +1339,14 @@ static int parse_cgroupfs_options(char *data, struct cgroup_sb_opts *opts)
>  		 * raced with a module_delete call, and to the user this is
>  		 * essentially a "subsystem doesn't exist" case.
>  		 */
> -		for (i--; i >= CGROUP_BUILTIN_SUBSYS_COUNT; i--) {
> +		for (i--; i >= 0; i--) {
>  			/* drop refcounts only on the ones we took */
>  			unsigned long bit = 1UL << i;
>  
>  			if (!(bit & opts->subsys_mask))
>  				continue;
> +			if (!subsys[i]->module)
> +				continue;

ditto

>  			module_put(subsys[i]->module);
>  		}
>  		return -ENOENT;
> @@ -1354,11 +1358,13 @@ static int parse_cgroupfs_options(char *data, struct cgroup_sb_opts *opts)
>  static void drop_parsed_module_refcounts(unsigned long subsys_mask)
>  {
>  	int i;
> -	for (i = CGROUP_BUILTIN_SUBSYS_COUNT; i < CGROUP_SUBSYS_COUNT; i++) {
> +	for (i = 0; i < CGROUP_SUBSYS_COUNT; i++) {
>  		unsigned long bit = 1UL << i;
>  
>  		if (!(bit & subsys_mask))
>  			continue;
> +		if (!subsys[i]->module)
> +			continue;

ditto

>  		module_put(subsys[i]->module);
>  	}
>  }
> @@ -1437,6 +1443,7 @@ static void init_cgroup_housekeeping(struct cgroup *cgrp)
>  	INIT_LIST_HEAD(&cgrp->event_list);
>  	spin_lock_init(&cgrp->event_list_lock);
>  	simple_xattrs_init(&cgrp->xattrs);
> +	memset(cgrp->subsys, 0, sizeof(cgrp->subsys));

This seems an unrelated change, and is redundant. Am I missing something?

>  }
>  


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html