From: Lai Jiangshan <laijs@cn.fujitsu.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: "Serge E. Hallyn" <serue@us.ibm.com>,
Paul Menage <menage@google.com>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Linux Containers <containers@lists.linux-foundation.org>
Subject: [PATCH -mm] devcgroup: remove spin_lock()
Date: Mon, 01 Sep 2008 17:18:34 +0800 [thread overview]
Message-ID: <48BBB36A.1030807@cn.fujitsu.com> (raw)
Since we introduced rcu for read side, spin_lock is used only for
update. But we always hold cgroup_lock() when update, so spin_lock()
is not need.
Additional cleanup:
1) include linux/rcupdate.h explicitly
2) remove unused variable cur_devcgroup in devcgroup_update_access()
Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com>
---
diff --git a/security/device_cgroup.c b/security/device_cgroup.c
index d5c15a7..5ba7870 100644
--- a/security/device_cgroup.c
+++ b/security/device_cgroup.c
@@ -1,5 +1,5 @@
/*
- * dev_cgroup.c - device cgroup subsystem
+ * device_cgroup.c - device cgroup subsystem
*
* Copyright 2007 IBM Corp
*/
@@ -10,6 +10,7 @@
#include <linux/list.h>
#include <linux/uaccess.h>
#include <linux/seq_file.h>
+#include <linux/rcupdate.h>
#define ACC_MKNOD 1
#define ACC_READ 2
@@ -22,18 +23,8 @@
/*
* whitelist locking rules:
- * cgroup_lock() cannot be taken under dev_cgroup->lock.
- * dev_cgroup->lock can be taken with or without cgroup_lock().
- *
- * modifications always require cgroup_lock
- * modifications to a list which is visible require the
- * dev_cgroup->lock *and* cgroup_lock()
- * walking the list requires dev_cgroup->lock or cgroup_lock().
- *
- * reasoning: dev_whitelist_copy() needs to kmalloc, so needs
- * a mutex, which the cgroup_lock() is. Since modifying
- * a visible list requires both locks, either lock can be
- * taken for walking the list.
+ * hold cgroup_lock() for update/read.
+ * hold rcu_read_lock() for read.
*/
struct dev_whitelist_item {
@@ -47,7 +38,6 @@ struct dev_whitelist_item {
struct dev_cgroup {
struct cgroup_subsys_state css;
struct list_head whitelist;
- spinlock_t lock;
};
static inline struct dev_cgroup *css_to_devcgroup(struct cgroup_subsys_state *s)
@@ -103,7 +93,6 @@ free_and_exit:
/* Stupid prototype - don't bother combining existing entries */
/*
* called under cgroup_lock()
- * since the list is visible to other tasks, we need the spinlock also
*/
static int dev_whitelist_add(struct dev_cgroup *dev_cgroup,
struct dev_whitelist_item *wh)
@@ -114,7 +103,6 @@ static int dev_whitelist_add(struct dev_cgroup *dev_cgroup,
if (!whcopy)
return -ENOMEM;
- spin_lock(&dev_cgroup->lock);
list_for_each_entry(walk, &dev_cgroup->whitelist, list) {
if (walk->type != wh->type)
continue;
@@ -130,7 +118,6 @@ static int dev_whitelist_add(struct dev_cgroup *dev_cgroup,
if (whcopy != NULL)
list_add_tail_rcu(&whcopy->list, &dev_cgroup->whitelist);
- spin_unlock(&dev_cgroup->lock);
return 0;
}
@@ -144,14 +131,12 @@ static void whitelist_item_free(struct rcu_head *rcu)
/*
* called under cgroup_lock()
- * since the list is visible to other tasks, we need the spinlock also
*/
static void dev_whitelist_rm(struct dev_cgroup *dev_cgroup,
struct dev_whitelist_item *wh)
{
struct dev_whitelist_item *walk, *tmp;
- spin_lock(&dev_cgroup->lock);
list_for_each_entry_safe(walk, tmp, &dev_cgroup->whitelist, list) {
if (walk->type == DEV_ALL)
goto remove;
@@ -169,7 +154,6 @@ remove:
call_rcu(&walk->rcu, whitelist_item_free);
}
}
- spin_unlock(&dev_cgroup->lock);
}
/*
@@ -209,7 +193,6 @@ static struct cgroup_subsys_state *devcgroup_create(struct cgroup_subsys *ss,
}
}
- spin_lock_init(&dev_cgroup->lock);
return &dev_cgroup->css;
}
@@ -325,15 +308,11 @@ static int parent_has_perm(struct dev_cgroup *childcg,
{
struct cgroup *pcg = childcg->css.cgroup->parent;
struct dev_cgroup *parent;
- int ret;
if (!pcg)
return 1;
parent = cgroup_to_devcgroup(pcg);
- spin_lock(&parent->lock);
- ret = may_access_whitelist(parent, wh);
- spin_unlock(&parent->lock);
- return ret;
+ return may_access_whitelist(parent, wh);
}
/*
@@ -352,7 +331,6 @@ static int parent_has_perm(struct dev_cgroup *childcg,
static int devcgroup_update_access(struct dev_cgroup *devcgroup,
int filetype, const char *buffer)
{
- struct dev_cgroup *cur_devcgroup;
const char *b;
char *endp;
int count;
@@ -361,8 +339,6 @@ static int devcgroup_update_access(struct dev_cgroup *devcgroup,
if (!capable(CAP_SYS_ADMIN))
return -EPERM;
- cur_devcgroup = task_devcgroup(current);
-
memset(&wh, 0, sizeof(wh));
b = buffer;
next reply other threads:[~2008-09-01 9:21 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-09-01 9:18 Lai Jiangshan [this message]
[not found] ` <48BBB36A.1030807-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
2008-09-02 14:56 ` [PATCH -mm] devcgroup: remove spin_lock() Serge E. Hallyn
2008-09-02 14:56 ` Serge E. Hallyn
-- strict thread matches above, loose matches on Subject: below --
2008-09-01 9:18 Lai Jiangshan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=48BBB36A.1030807@cn.fujitsu.com \
--to=laijs@cn.fujitsu.com \
--cc=akpm@linux-foundation.org \
--cc=containers@lists.linux-foundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=menage@google.com \
--cc=serue@us.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.