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 for Android: free password hash cracker in your pocket

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

Message-ID: <20121101151556.GB5065@dhcp22.suse.cz>
Date:	Thu, 1 Nov 2012 16:15:56 +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 16:05:32, Michal Hocko wrote:
> 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;

this needs to drop the lock of course but it doesn't make it much more
complicated...
 
> > 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/