cgroups: move cgroup_event_listener.c to tools/cgroup

PATCH Applied Greg Thelen 5 participants 10 messages Jan 4, 2013 – Jan 8, 2013 2 messages not in the archive

Subsystems (2)

Documentation cgroup

lore

Share & tools

lore

Greg Thelen

author

2013-01-04 21:05 UTC #0 lore raw

PATCH 1/2 cgroups: move cgroup_event_listener.c to tools/cgroup

Move the cgroup_event_listener.c tool from Documentation into the new tools/cgroup directory.

This change involves wiring cgroup_event_listener.c into the tools/ make system so that is can be built with:

  $ make tools/cgroup

Signed-off-by Greg Thelen <[email protected]>

---
 Documentation/cgroups/00-INDEX                |    2 -
 Documentation/cgroups/cgroup_event_listener.c |  110 -------------------------
 Documentation/cgroups/memcg_test.txt          |    3 +-
 tools/Makefile                                |   19 ++--
 tools/cgroup/.gitignore                       |    1 +
 tools/cgroup/Makefile                         |   11 +++
 tools/cgroup/cgroup_event_listener.c          |  110 +++++++++++++++++++++++++
 7 files changed, 133 insertions(+), 123 deletions(-)
 delete mode 100644 Documentation/cgroups/cgroup_event_listener.c
 create mode 100644 tools/cgroup/.gitignore
 create mode 100644 tools/cgroup/Makefile
 create mode 100644 tools/cgroup/cgroup_event_listener.c

Documentation/cgroups/00-INDEX +0 -2

index f78b90a..f5635a0 100644
@@ -4,8 +4,6 @@ blkio-controller.txt
 	- Description for Block IO Controller, implementation and usage details.
 cgroups.txt
 	- Control Groups definition, implementation details, examples and API.
-cgroup_event_listener.c
-	- A user program for cgroup listener.
 cpuacct.txt
 	- CPU Accounting Controller; account CPU usage for groups of tasks.
 cpusets.txt

Documentation/cgroups/cgroup_event_listener.c deleted +0 ~~-110~~

110 lines not on this page · raw message

Documentation/cgroups/memcg_test.txt +1 -2

index fc8fa97..ce94a83 100644
@@ -399,8 +399,7 @@ Under below explanation, we assume CONFIG_MEM_RES_CTRL_SWAP=y.
 
  9.10 Memory thresholds
 	Memory controller implements memory thresholds using cgroups notification
-	API. You can use Documentation/cgroups/cgroup_event_listener.c to test
-	it.
+	API. You can use tools/cgroup/cgroup_event_listener.c to test it.
 
 	(Shell-A) Create cgroup and run event listener
 	# mkdir /cgroup/A

tools/Makefile +10 -9

index 1f9a529..fc57a28 100644
@@ -3,6 +3,7 @@ include scripts/Makefile.include
 help:
 	@echo 'Possible targets:'
 	@echo ''
+	@echo '  cgroup     - cgroup tools'
 	@echo '  cpupower   - a tool for all things x86 CPU power'
 	@echo '  firewire   - the userspace part of nosy, an IEEE-1394 traffic sniffer'
 	@echo '  lguest     - a minimal 32-bit x86 hypervisor'

@@ -33,7 +34,7 @@ help:
 cpupower: FORCE
 	$(call descend,power/$@)
 
-firewire lguest perf usb virtio vm: FORCE
+cgroup firewire lguest perf usb virtio vm: FORCE
 	$(call descend,$@)
 
 selftests: FORCE

@@ -45,7 +46,7 @@ turbostat x86_energy_perf_policy: FORCE
 cpupower_install:
 	$(call descend,power/$(@:_install=),install)
 
-firewire_install lguest_install perf_install usb_install virtio_install vm_install:
+cgroup_install firewire_install lguest_install perf_install usb_install virtio_install vm_install:
 	$(call descend,$(@:_install=),install)
 
 selftests_install:

@@ -54,14 +55,14 @@ selftests_install:
 turbostat_install x86_energy_perf_policy_install:
 	$(call descend,power/x86/$(@:_install=),install)
 
-install: cpupower_install firewire_install lguest_install perf_install \
-		selftests_install turbostat_install usb_install virtio_install \
-		vm_install x86_energy_perf_policy_install
+install: cgroup_install cpupower_install firewire_install lguest_install \
+		perf_install selftests_install turbostat_install usb_install \
+		virtio_install vm_install x86_energy_perf_policy_install
 
 cpupower_clean:
 	$(call descend,power/cpupower,clean)
 
-firewire_clean lguest_clean perf_clean usb_clean virtio_clean vm_clean:
+cgroup_clean firewire_clean lguest_clean perf_clean usb_clean virtio_clean vm_clean:
 	$(call descend,$(@:_clean=),clean)
 
 selftests_clean:

@@ -70,8 +71,8 @@ selftests_clean:
 turbostat_clean x86_energy_perf_policy_clean:
 	$(call descend,power/x86/$(@:_clean=),clean)
 
-clean: cpupower_clean firewire_clean lguest_clean perf_clean selftests_clean \
-		turbostat_clean usb_clean virtio_clean vm_clean \
-		x86_energy_perf_policy_clean
+clean: cgroup_clean cpupower_clean firewire_clean lguest_clean perf_clean \
+		selftests_clean turbostat_clean usb_clean virtio_clean \
+		vm_clean x86_energy_perf_policy_clean
 
 .PHONY: FORCE

tools/cgroup/.gitignore new file +1 -0

new file mode 100644
index 0000000..633cd9b
@@ -0,0 +1 @@
+cgroup_event_listener

tools/cgroup/Makefile new file +11 -0

new file mode 100644
index 0000000..b428619
@@ -0,0 +1,11 @@
+# Makefile for cgroup tools
+
+CC = $(CROSS_COMPILE)gcc
+CFLAGS = -Wall -Wextra
+
+all: cgroup_event_listener
+%: %.c
+	$(CC) $(CFLAGS) -o $@ $^
+
+clean:
+	$(RM) cgroup_event_listener

tools/cgroup/cgroup_event_listener.c new file +110 -0

110 lines not on this page · raw message

Signature · 1 line

1.7.7.3

Greg Thelen

author

2013-01-04 21:05 UTC #1 lore raw

PATCH 2/2 cgroups: fix cgroup_event_listener error handling

If the <absolute-path-to-control-file> command line parameter cannot be opened, then cgroup_event_listener prints an error message and tries to return an error. However, due to an uninitialized variable the return value was undefined.

With this patch such failures always return non-zero error.

Compiler warning found this:

  $ gcc -Wall -O2 cgroup_event_listener.c
  cgroup_event_listener.c: In function ‘main’:
  cgroup_event_listener.c:109:2: warning: ‘ret’ may be used uninitialized in this function [-Wuninitialized]

Signed-off-by Greg Thelen <[email protected]>

---
 tools/cgroup/cgroup_event_listener.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

tools/cgroup/cgroup_event_listener.c +1 -1

index 3e082f9..a70f00c 100644
@@ -35,7 +35,7 @@ int main(int argc, char **argv)
 	if (cfd == -1) {
 		fprintf(stderr, "Cannot open %s: %s\n", argv[1],
 				strerror(errno));
-		goto out;
+		return 1;
 	}
 
 	ret = snprintf(event_control_path, PATH_MAX, "%s/cgroup.event_control",

Signature · 1 line

1.7.7.3

Tejun Heo

maintainer

2013-01-07 17:44 UTC #2 lore raw

Re: [PATCH 2/2] cgroups: fix cgroup_event_listener error handling

Quoted 22 lines · Greg Thelen

If the <absolute-path-to-control-file> command line parameter cannot be opened, then cgroup_event_listener prints an error message and tries to return an error. However, due to an uninitialized variable the return value was undefined.
With this patch such failures always return non-zero error.
Compiler warning found this:
  $ gcc -Wall -O2 cgroup_event_listener.c
  cgroup_event_listener.c: In function ‘main’:
  cgroup_event_listener.c:109:2: warning: ‘ret’ may be used uninitialized in this function [-Wuninitialized]
Signed-off-by Greg Thelen <[email protected]>
---
 tools/cgroup/cgroup_event_listener.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
tools/cgroup/cgroup_event_listener.c +0 -0
index 3e082f9..a70f00c 100644

tools/cgroup/cgroup_event_listener.c int main(int argc, char **argv) · line 38

Quoted 5 lines

tools/cgroup/cgroup_event_listener.c +1 -1

@@ -35,7 +35,7 @@ int main(int argc, char **argv)
 	if (cfd == -1) {
 		fprintf(stderr, "Cannot open %s: %s\n", argv[1],
 				strerror(errno));
-		goto out;
+		return 1;

Hmm... so, event_control open failure path is broken the same way. Can you please fix it together? Please just remove the cleanup path.

Thanks.

Signature · 1 line

tejun

Greg Thelen

author

2013-01-07 19:50 UTC #3 lore raw

Re: [PATCH 2/2] cgroups: fix cgroup_event_listener error handling

Quoted 18 lines · Tejun Heo

Quoted 17 lines · Greg Thelen
If the <absolute-path-to-control-file> command line parameter cannot be opened, then cgroup_event_listener prints an error message and tries to return an error. However, due to an uninitialized variable the return value was undefined.
With this patch such failures always return non-zero error.
Compiler warning found this:
  $ gcc -Wall -O2 cgroup_event_listener.c
  cgroup_event_listener.c: In function ‘main’:
  cgroup_event_listener.c:109:2: warning: ‘ret’ may be used uninitialized in this function [-Wuninitialized]
Signed-off-by Greg Thelen <[email protected]>
---
 tools/cgroup/cgroup_event_listener.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Quoted 13 lines

tools/cgroup/cgroup_event_listener.c int main(int argc, char **argv) · line 38
Quoted 10 lines
tools/cgroup/cgroup_event_listener.c +1 -1
index 3e082f9..a70f00c 100644
@@ -35,7 +35,7 @@ int main(int argc, char **argv)
 	if (cfd == -1) {
 		fprintf(stderr, "Cannot open %s: %s\n", argv[1],
 				strerror(errno));
-		goto out;
+		return 1;
Hmm... so, event_control open failure path is broken the same way. Can you please fix it together? Please just remove the cleanup path.

Done. Patch below.

---8<---
cgroups: fix cgroup_event_listener error handling

The error handling in cgroup_event_listener.c did not correctly deal with either an error opening either <control_file> or cgroup.event_control. Due to an uninitialized variable the program exit code was undefined if either of these opens failed.

This patch simplifies and corrects cgroup_event_listener.c error handling by:
1. using err*() rather than printf(),exit()
2. depending on process exit to close open files

With this patch failures always return non-zero error.

Signed-off-by Greg Thelen <[email protected]>

---
 tools/cgroup/cgroup_event_listener.c |   72 ++++++++++-----------------------
 1 files changed, 22 insertions(+), 50 deletions(-)

tools/cgroup/cgroup_event_listener.c +22 ~~-50~~

index 3e082f9..4eb5507 100644
@@ -5,6 +5,7 @@
  */
 
 #include <assert.h>
+#include <err.h>
 #include <errno.h>
 #include <fcntl.h>
 #include <libgen.h>

@@ -15,7 +16,7 @@
 
 #include <sys/eventfd.h>
 
-#define USAGE_STR "Usage: cgroup_event_listener <path-to-control-file> <args>\n"
+#define USAGE_STR "Usage: cgroup_event_listener <path-to-control-file> <args>"
 
 int main(int argc, char **argv)
 {

@@ -26,49 +27,33 @@ int main(int argc, char **argv)
 	char line[LINE_MAX];
 	int ret;
 
-	if (argc != 3) {
-		fputs(USAGE_STR, stderr);
-		return 1;
-	}
+	if (argc != 3)
+		errx(1, "%s", USAGE_STR);
 
 	cfd = open(argv[1], O_RDONLY);
-	if (cfd == -1) {
-		fprintf(stderr, "Cannot open %s: %s\n", argv[1],
-				strerror(errno));
-		goto out;
-	}
+	if (cfd == -1)
+		err(1, "Cannot open %s", argv[1]);
 
 	ret = snprintf(event_control_path, PATH_MAX, "%s/cgroup.event_control",
 			dirname(argv[1]));
-	if (ret >= PATH_MAX) {
-		fputs("Path to cgroup.event_control is too long\n", stderr);
-		goto out;
-	}
+	if (ret >= PATH_MAX)
+		errx(1, "Path to cgroup.event_control is too long");
 
 	event_control = open(event_control_path, O_WRONLY);
-	if (event_control == -1) {
-		fprintf(stderr, "Cannot open %s: %s\n", event_control_path,
-				strerror(errno));
-		goto out;
-	}
+	if (event_control == -1)
+		err(1, "Cannot open %s", event_control_path);
 
 	efd = eventfd(0, 0);
-	if (efd == -1) {
-		perror("eventfd() failed");
-		goto out;
-	}
+	if (efd == -1)
+		err(1, "eventfd() failed");
 
 	ret = snprintf(line, LINE_MAX, "%d %d %s", efd, cfd, argv[2]);
-	if (ret >= LINE_MAX) {
-		fputs("Arguments string is too long\n", stderr);
-		goto out;
-	}
+	if (ret >= LINE_MAX)
+		errx(1, "Arguments string is too long");
 
 	ret = write(event_control, line, strlen(line) + 1);
-	if (ret == -1) {
-		perror("Cannot write to cgroup.event_control");
-		goto out;
-	}
+	if (ret == -1)
+		err(1, "Cannot write to cgroup.event_control");
 
 	while (1) {
 		uint64_t result;

@@ -77,34 +62,21 @@ int main(int argc, char **argv)
 		if (ret == -1) {
 			if (errno == EINTR)
 				continue;
-			perror("Cannot read from eventfd");
-			break;
+			err(1, "Cannot read from eventfd");
 		}
 		assert(ret == sizeof(result));
 
 		ret = access(event_control_path, W_OK);
 		if ((ret == -1) && (errno == ENOENT)) {
-				puts("The cgroup seems to have removed.");
-				ret = 0;
-				break;
-		}
-
-		if (ret == -1) {
-			perror("cgroup.event_control "
-					"is not accessible any more");
+			puts("The cgroup seems to have removed.");
 			break;
 		}
 
+		if (ret == -1)
+			err(1, "cgroup.event_control is not accessible any more");
+
 		printf("%s %s: crossed\n", argv[1], argv[2]);
 	}
 
-out:
-	if (efd >= 0)
-		close(efd);
-	if (event_control >= 0)
-		close(event_control);
-	if (cfd >= 0)
-		close(cfd);
-
-	return (ret != 0);
+	return 0;
 }

Signature · 6 lines

1.7.7.3

_______________________________________________
Containers mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/containers

Li Zefan

Acked-by

2013-01-08 06:28 UTC #4 lore raw

Re: [PATCH 2/2] cgroups: fix cgroup_event_listener error handling

Quoted 7 lines

cgroups: fix cgroup_event_listener error handling
The error handling in cgroup_event_listener.c did not correctly deal with either an error opening either <control_file> or cgroup.event_control. Due to an uninitialized variable the program exit code was undefined if either of these opens failed.

Quoted 8 lines

This patch simplifies and corrects cgroup_event_listener.c error handling by:
1. using err*() rather than printf(),exit()
2. depending on process exit to close open files
With this patch failures always return non-zero error.
Signed-off-by Greg Thelen <[email protected]>

Acked-by Li Zefan <[email protected]>

Quoted 3 lines

---
 tools/cgroup/cgroup_event_listener.c |   72 ++++++++++-----------------------
 1 files changed, 22 insertions(+), 50 deletions(-)

Tejun Heo

maintainer Applied

2013-01-08 18:08 UTC #5 lore raw

Re: [PATCH 2/2] cgroups: fix cgroup_event_listener error handling

Quoted 12 lines · Li Zefan

Quoted 12 lines
cgroups: fix cgroup_event_listener error handling
The error handling in cgroup_event_listener.c did not correctly deal with either an error opening either <control_file> or cgroup.event_control. Due to an uninitialized variable the program exit code was undefined if either of these opens failed.
This patch simplifies and corrects cgroup_event_listener.c error handling by:
1. using err*() rather than printf(),exit()
2. depending on process exit to close open files

Quoted 5 lines

Quoted 3 lines
With this patch failures always return non-zero error.
Signed-off-by Greg Thelen <[email protected]>
Acked-by Li Zefan <[email protected]>

Applied to cgroup/for-3.9.

Thanks.

Signature · 1 line

tejun

A message that is not in the archive (sent off-list or not yet received): 1357333518-28899-2-git-send-email-gthelen-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org

Tejun Heo

2013-01-07 17:44 UTC #7 lore raw

Re: [PATCH 2/2] cgroups: fix cgroup_event_listener error handling

Quoted 22 lines · Greg Thelen

If the <absolute-path-to-control-file> command line parameter cannot be opened, then cgroup_event_listener prints an error message and tries to return an error. However, due to an uninitialized variable the return value was undefined.
With this patch such failures always return non-zero error.
Compiler warning found this:
  $ gcc -Wall -O2 cgroup_event_listener.c
  cgroup_event_listener.c: In function ‘main’:
  cgroup_event_listener.c:109:2: warning: ‘ret’ may be used uninitialized in this function [-Wuninitialized]
Signed-off-by Greg Thelen <[email protected]>
---
 tools/cgroup/cgroup_event_listener.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
tools/cgroup/cgroup_event_listener.c +0 -0
index 3e082f9..a70f00c 100644

tools/cgroup/cgroup_event_listener.c int main(int argc, char **argv) · line 38

Quoted 5 lines

tools/cgroup/cgroup_event_listener.c +1 -1

@@ -35,7 +35,7 @@ int main(int argc, char **argv)
 	if (cfd == -1) {
 		fprintf(stderr, "Cannot open %s: %s\n", argv[1],
 				strerror(errno));
-		goto out;
+		return 1;

Hmm... so, event_control open failure path is broken the same way. Can you please fix it together? Please just remove the cleanup path.

Thanks.

Signature · 5 lines

tejun
_______________________________________________
Containers mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/containers

A message that is not in the archive (sent off-list or not yet received): 1357333518-28899-1-git-send-email-gthelen-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org

Greg Thelen

2013-01-04 21:05 UTC #9 lore raw

PATCH 2/2 cgroups: fix cgroup_event_listener error handling

With this patch such failures always return non-zero error.

Compiler warning found this:

  $ gcc -Wall -O2 cgroup_event_listener.c
  cgroup_event_listener.c: In function ‘main’:
  cgroup_event_listener.c:109:2: warning: ‘ret’ may be used uninitialized in this function [-Wuninitialized]

Signed-off-by Greg Thelen <[email protected]>

---
 tools/cgroup/cgroup_event_listener.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

tools/cgroup/cgroup_event_listener.c +1 -1

index 3e082f9..a70f00c 100644
@@ -35,7 +35,7 @@ int main(int argc, char **argv)
 	if (cfd == -1) {
 		fprintf(stderr, "Cannot open %s: %s\n", argv[1],
 				strerror(errno));
-		goto out;
+		return 1;
 	}
 
 	ret = snprintf(event_control_path, PATH_MAX, "%s/cgroup.event_control",

Signature · 6 lines

1.7.7.3

_______________________________________________
Containers mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/containers

Tejun Heo

Applied

2013-01-07 17:42 UTC #10 lore raw

In reply to a message not in the archive

Quoted 8 lines · Greg Thelen

Move the cgroup_event_listener.c tool from Documentation into the new tools/cgroup directory.
This change involves wiring cgroup_event_listener.c into the tools/ make system so that is can be built with:
  $ make tools/cgroup
Signed-off-by Greg Thelen <[email protected]>

Applied to cgroup/for-3.9.

Thanks.

Signature · 1 line

tejun

Tejun Heo

maintainer Applied

2013-01-07 17:42 UTC #11 lore raw

In reply to Greg Thelen

Quoted 8 lines · Greg Thelen

Move the cgroup_event_listener.c tool from Documentation into the new tools/cgroup directory.
This change involves wiring cgroup_event_listener.c into the tools/ make system so that is can be built with:
  $ make tools/cgroup
Signed-off-by Greg Thelen <[email protected]>

Applied to cgroup/for-3.9.

Thanks.

Signature · 1 line

tejun