linux-kernel - Re: [PATCH 3/8] cgroup: use cgroup_lock_live_group(parent) in cgroup

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: <20121101150532.GA5065@dhcp22.suse.cz>
Date:	Thu, 1 Nov 2012 16:05:32 +0100
From:	Michal Hocko <mhocko@...e.cz>
To:	Tejun Heo <tj@...nel.org>
Cc:	lizefan@...wei.com, hannes@...xchg.org, bsingharora@...il.com,
	kamezawa.hiroyu@...fujitsu.com,
	containers@...ts.linux-foundation.org, cgroups@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/8] cgroup: use cgroup_lock_live_group(parent) in
 cgroup_create()

On Thu 01-11-12 07:52:24, Tejun Heo wrote:
> Hey, Michal.
> 
> On Thu, Nov 1, 2012 at 2:16 AM, Michal Hocko <mhocko@...e.cz> wrote:
> > I am not sure I understand. What does deactivate_super has to do with
> > the above suggestion? cgroup_lock_live_group will take the cgroup_mutex
> > on the success or frees the previously allocated&unused memory. The
> > only thing I was suggesting is to do cgroup_lock_live_group first and
> > allocate the cgroup only if it doesn't fail.
> 
> It complicates updates to the error exit path.

Still don't get it, sorry. What prevents us to do:
static long cgroup_create(struct cgroup *parent, struct dentry *dentry,
                             umode_t mode)
{
        struct cgroup *cgrp;
        struct cgroupfs_root *root = parent->root;
        int err = 0;
        struct cgroup_subsys *ss;
        struct super_block *sb = root->sb;

	if (!cgroup_lock_live_group(parent))
		return -ENODEV;

        cgrp = kzalloc(sizeof(*cgrp), GFP_KERNEL);
        if (!cgrp)
                return -ENOMEM;

> You end up having to update a lot more and it's not like grabbing lock
> first is substantially better in any way, so why bother?

Yes the allocation can sleep if we are short on memory so this can
potentially take long which is not entirely nice but a pointless
allocation is not nice either. Anyway I am asking because I am trying to
understand what is the motivation behind and your explanation about the
error exit path doesn't make much sense to me. So I am either missing
something or we are talking about two different things.
-- 
Michal Hocko
SUSE Labs
--
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/