Skip to contentSkip to conversation

CGroups: misc cleanups to write_string patchset

PATCH In review Paul Menage 2 participants 2 messages Jun 27, 2008 cgroup

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 CGroups: misc cleanups to write_string patchset

CGroups: misc cleanups to write_string patchset

This patch contains cleanups suggested by reviewers for the recent write_string() patchset:

- pair cgroup_lock_live_group() with cgroup_unlock() in cgroup.c for clarity, rather than directly unlocking cgroup_mutex.

- make the return type of cgroup_lock_live_group() a bool

- use a #define'd constant for the local buffer size in read/write functions

---
include/linux/cgroup.h | 4 ++--
kernel/cgroup.c | 21 ++++++++++++---------

2 files changed, 14 insertions(+), 11 deletions(-)
include/linux/cgroup.h +0 -0
Index: cws2-2.6.26-rc5-mm3/include/linux/cgroup.h
===================================================================
@@ -21,11 +21,13 @@

struct cgroupfs_root;
struct cgroup_subsys;
struct inode;
+struct cgroup;

extern int cgroup_init_early(void);
extern int cgroup_init(void);
extern void cgroup_init_smp(void);
extern void cgroup_lock(void);
+extern bool cgroup_lock_live_group(struct cgroup *cgrp); extern void cgroup_unlock(void);
extern void cgroup_fork(struct task_struct *p); extern void cgroup_fork_callbacks(struct task_struct *p); @@ -295,8 +297,6 @@ int cgroup_add_files(struct cgroup *cgrp

int cgroup_is_removed(const struct cgroup *cgrp);

-int cgroup_lock_live_group(struct cgroup *cgrp);
-
int cgroup_path(const struct cgroup *cgrp, char *buf, int buflen);

int cgroup_task_count(const struct cgroup *cgrp);

kernel/cgroup.c +3 -3
Index: cws2-2.6.26-rc5-mm3/kernel/cgroup.c
===================================================================
@@ -1331,10 +1331,10 @@ enum cgroup_filetype {
 * cgroup_lock_live_group - take cgroup_mutex and check that cgrp is alive.
 * @cgrp: the cgroup to be checked for liveness
 *
- * Returns true (with lock held) on success, or false (with no lock
- * held) on failure.
+ * On success, returns true; the lock should be later released with
+ * cgroup_unlock(). On failure returns false with no lock held.
 */
-int cgroup_lock_live_group(struct cgroup *cgrp)
+bool cgroup_lock_live_group(struct cgroup *cgrp)
{
	mutex_lock(&cgroup_mutex);
	if (cgroup_is_removed(cgrp)) {
@@ -1351,7 +1351,7 @@ static int cgroup_release_agent_write(st
	if (!cgroup_lock_live_group(cgrp))
		return -ENODEV;
	strcpy(cgrp->root->release_agent_path, buffer);
-	mutex_unlock(&cgroup_mutex);
+	cgroup_unlock();
	return 0;
}

@@ -1362,16 +1362,19 @@ static int cgroup_release_agent_show(str

		return -ENODEV;
	seq_puts(seq, cgrp->root->release_agent_path);
	seq_putc(seq, '\n');
-	mutex_unlock(&cgroup_mutex);
+	cgroup_unlock();
	return 0;
}

+/* A buffer size big enough for numbers or short strings */ +#define CGROUP_LOCAL_BUFFER_SIZE 64 +
static ssize_t cgroup_write_X64(struct cgroup *cgrp, struct cftype *cft,
struct file *file,
const char __user *userbuf, size_t nbytes, loff_t *unused_ppos)
{
- char buffer[64];

+	char buffer[CGROUP_LOCAL_BUFFER_SIZE];
	int retval = 0;
	char *end;

@@ -1405,7 +1408,7 @@ static ssize_t cgroup_write_string(struc
const char __user *userbuf, size_t nbytes, loff_t *unused_ppos)
{
- char local_buffer[64];

+	char local_buffer[CGROUP_LOCAL_BUFFER_SIZE];
	int retval = 0;
	size_t max_bytes = cft->max_write_len;
	char *buffer = local_buffer;

@@ -1459,7 +1462,7 @@ static ssize_t cgroup_read_u64(struct cg
char __user *buf, size_t nbytes, loff_t *ppos)
{
- char tmp[64];

+	char tmp[CGROUP_LOCAL_BUFFER_SIZE];
	u64 val = cft->read_u64(cgrp, cft);
	int len = sprintf(tmp, "%llu\n", (unsigned long long) val);

@@ -1471,7 +1474,7 @@ static ssize_t cgroup_read_s64(struct cg
char __user *buf, size_t nbytes, loff_t *ppos)
{
- char tmp[64];

+	char tmp[CGROUP_LOCAL_BUFFER_SIZE];
	s64 val = cft->read_s64(cgrp, cft);
	int len = sprintf(tmp, "%lld\n", (long long) val);
Acked-by on v1 1/1
#1 lore raw
Quoted 5 lines · Paul Menage

CGroups: misc cleanups to write_string patchset

This patch contains cleanups suggested by reviewers for the recent write_string() patchset:

Quoted 8 lines

- pair cgroup_lock_live_group() with cgroup_unlock() in cgroup.c for clarity, rather than directly unlocking cgroup_mutex.

- make the return type of cgroup_lock_live_group() a bool

- use a #define'd constant for the local buffer size in read/write functions

The use of cgroup_unlock() really is much more readable.

thanks,
-serge

Quoted 107 lines

---
include/linux/cgroup.h | 4 ++--
kernel/cgroup.c | 21 ++++++++++++---------

2 files changed, 14 insertions(+), 11 deletions(-)

Index: cws2-2.6.26-rc5-mm3/include/linux/cgroup.h
===================================================================

include/linux/cgroup.h +2 -2
@@ -21,11 +21,13 @@
 struct cgroupfs_root;
 struct cgroup_subsys;
 struct inode;
+struct cgroup;
 
 extern int cgroup_init_early(void);
 extern int cgroup_init(void);
 extern void cgroup_init_smp(void);
 extern void cgroup_lock(void);
+extern bool cgroup_lock_live_group(struct cgroup *cgrp);
 extern void cgroup_unlock(void);
 extern void cgroup_fork(struct task_struct *p);
 extern void cgroup_fork_callbacks(struct task_struct *p);
@@ -295,8 +297,6 @@ int cgroup_add_files(struct cgroup *cgrp
 
 int cgroup_is_removed(const struct cgroup *cgrp);
 
-int cgroup_lock_live_group(struct cgroup *cgrp);
-
 int cgroup_path(const struct cgroup *cgrp, char *buf, int buflen);
 
 int cgroup_task_count(const struct cgroup *cgrp);

Index: cws2-2.6.26-rc5-mm3/kernel/cgroup.c
===================================================================

kernel/cgroup.c +11 -9
@@ -1331,10 +1331,10 @@ enum cgroup_filetype {
 * cgroup_lock_live_group - take cgroup_mutex and check that cgrp is alive.
 * @cgrp: the cgroup to be checked for liveness
 *
- * Returns true (with lock held) on success, or false (with no lock
- * held) on failure.
+ * On success, returns true; the lock should be later released with
+ * cgroup_unlock(). On failure returns false with no lock held.
 */
-int cgroup_lock_live_group(struct cgroup *cgrp)
+bool cgroup_lock_live_group(struct cgroup *cgrp)
 {
 	mutex_lock(&cgroup_mutex);
 	if (cgroup_is_removed(cgrp)) {
@@ -1351,7 +1351,7 @@ static int cgroup_release_agent_write(st
 	if (!cgroup_lock_live_group(cgrp))
 		return -ENODEV;
 	strcpy(cgrp->root->release_agent_path, buffer);
-	mutex_unlock(&cgroup_mutex);
+	cgroup_unlock();
 	return 0;
 }
 
@@ -1362,16 +1362,19 @@ static int cgroup_release_agent_show(str
 		return -ENODEV;
 	seq_puts(seq, cgrp->root->release_agent_path);
 	seq_putc(seq, '\n');
-	mutex_unlock(&cgroup_mutex);
+	cgroup_unlock();
 	return 0;
 }
 
+/* A buffer size big enough for numbers or short strings */
+#define CGROUP_LOCAL_BUFFER_SIZE 64 +
 static ssize_t cgroup_write_X64(struct cgroup *cgrp, struct cftype *cft,
 				struct file *file,
 				const char __user *userbuf,
 				size_t nbytes, loff_t *unused_ppos)
 {
-	char buffer[64];
+	char buffer[CGROUP_LOCAL_BUFFER_SIZE];
 	int retval = 0;
 	char *end;
 
@@ -1405,7 +1408,7 @@ static ssize_t cgroup_write_string(struc
 				   const char __user *userbuf,
 				   size_t nbytes, loff_t *unused_ppos)
 {
-	char local_buffer[64];
+	char local_buffer[CGROUP_LOCAL_BUFFER_SIZE];
 	int retval = 0;
 	size_t max_bytes = cft->max_write_len;
 	char *buffer = local_buffer;
@@ -1459,7 +1462,7 @@ static ssize_t cgroup_read_u64(struct cg
 			       char __user *buf, size_t nbytes,
 			       loff_t *ppos)
 {
-	char tmp[64];
+	char tmp[CGROUP_LOCAL_BUFFER_SIZE];
 	u64 val = cft->read_u64(cgrp, cft);
 	int len = sprintf(tmp, "%llu\n", (unsigned long long) val);
 
@@ -1471,7 +1474,7 @@ static ssize_t cgroup_read_s64(struct cg
 			       char __user *buf, size_t nbytes,
 			       loff_t *ppos)
 {
-	char tmp[64];
+	char tmp[CGROUP_LOCAL_BUFFER_SIZE];
 	s64 val = cft->read_s64(cgrp, cft);
 	int len = sprintf(tmp, "%lld\n", (long long) val);