linux-kernel - Re: query: [PATCH 2/2] cgroup: Remove call to synchronize_rcu in cgroup_attach

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: <BANLkTikqvP90Etu=L24DPWbckrawM-6n=Q@mail.gmail.com>
Date:	Wed, 13 Apr 2011 15:16:55 +0200
From:	Paul Menage <menage@...gle.com>
To:	Mike Galbraith <efault@....de>
Cc:	Li Zefan <lizf@...fujitsu.com>,
	LKML <linux-kernel@...r.kernel.org>,
	Colin Cross <ccross@...roid.com>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Ingo Molnar <mingo@...e.hu>
Subject: Re: query: [PATCH 2/2] cgroup: Remove call to synchronize_rcu in cgroup_attach_task

On Wed, Apr 13, 2011 at 5:11 AM, Mike Galbraith <efault@....de> wrote:
> If the user _does_ that rmdir(), it's more or less back to square one.
> RCU grace periods should not impact userland, but if you try to do
> create/attach/detach/destroy, you run into the same bottleneck, as does
> any asynchronous GC, though that's not such a poke in the eye.  I tried
> a straight forward move to schedule_work(), and it seems to work just
> fine.  rmdir() no longer takes ~30ms on my box, but closer to 20us.

> +       /*
> +        * Release the subsystem state objects.
> +        */
> +       for_each_subsys(cgrp->root, ss)
> +               ss->destroy(ss, cgrp);
> +
> +       cgrp->root->number_of_cgroups--;
> +       mutex_unlock(&cgroup_mutex);
> +
> +       /*
> +        * Drop the active superblock reference that we took when we
> +        * created the cgroup
> +        */
> +       deactivate_super(cgrp->root->sb);
> +
> +       /*
> +        * if we're getting rid of the cgroup, refcount should ensure
> +        * that there are no pidlists left.
> +        */
> +       BUG_ON(!list_empty(&cgrp->pidlists));
> +
> +       kfree(cgrp);

We might want to punt this through RCU again, in case the subsystem
destroy() callbacks left anything around that was previously depending
on the RCU barrier.

Also, I'd be concerned that subsystems might get confused by the fact
that a new group called 'foo' could be created before the old 'foo'
has been cleaned up? (And do any subsystems rely on being able to
access the cgroup dentry up until the point when destroy() is called?

Paul
--
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/