* [PATCH] use for_each_cpu in oprofile code
@ 2004-09-04 17:44 Anton Blanchard
2004-09-04 17:46 ` [PATCH] fix oprofile vfree warning on error Anton Blanchard
0 siblings, 1 reply; 6+ messages in thread
From: Anton Blanchard @ 2004-09-04 17:44 UTC (permalink / raw)
To: akpm; +Cc: levon, phil.el, linux-kernel
Replace open coded versions with for_each_cpu()/for_each_online_cpu().
Signed-off-by: Anton Blanchard <anton@samba.org>
===== cpu_buffer.c 1.11 vs edited =====
--- 1.11/drivers/oprofile/cpu_buffer.c Fri Aug 27 16:42:56 2004
+++ edited/cpu_buffer.c Sun Sep 5 02:18:01 2004
@@ -36,11 +36,8 @@
{
int i;
- for (i = 0; i < NR_CPUS; ++i) {
- if (!cpu_online(i))
- continue;
+ for_each_online_cpu(i)
vfree(cpu_buffer[i].buffer);
- }
}
@@ -50,12 +47,9 @@
unsigned long buffer_size = fs_cpu_buffer_size;
- for (i = 0; i < NR_CPUS; ++i) {
+ for_each_online_cpu(i) {
struct oprofile_cpu_buffer * b = &cpu_buffer[i];
- if (!cpu_online(i))
- continue;
-
b->buffer = vmalloc(sizeof(struct op_sample) * buffer_size);
if (!b->buffer)
goto fail;
@@ -94,12 +88,9 @@
timers_enabled = 1;
- for (i = 0; i < NR_CPUS; ++i) {
+ for_each_online_cpu(i) {
struct oprofile_cpu_buffer * b = &cpu_buffer[i];
- if (!cpu_online(i))
- continue;
-
add_timer_on(&b->timer, i);
}
}
@@ -111,11 +102,8 @@
timers_enabled = 0;
- for (i = 0; i < NR_CPUS; ++i) {
+ for_each_online_cpu(i) {
struct oprofile_cpu_buffer * b = &cpu_buffer[i];
-
- if (!cpu_online(i))
- continue;
del_timer_sync(&b->timer);
}
===== oprofile_stats.c 1.8 vs edited =====
--- 1.8/drivers/oprofile/oprofile_stats.c Fri Aug 27 16:42:56 2004
+++ edited/oprofile_stats.c Sun Sep 5 02:15:49 2004
@@ -22,10 +22,7 @@
struct oprofile_cpu_buffer * cpu_buf;
int i;
- for (i = 0; i < NR_CPUS; ++i) {
- if (!cpu_possible(i))
- continue;
-
+ for_each_cpu(i) {
cpu_buf = &cpu_buffer[i];
cpu_buf->sample_received = 0;
cpu_buf->sample_lost_overflow = 0;
@@ -49,10 +46,7 @@
if (!dir)
return;
- for (i = 0; i < NR_CPUS; ++i) {
- if (!cpu_possible(i))
- continue;
-
+ for_each_cpu(i) {
cpu_buf = &cpu_buffer[i];
snprintf(buf, 10, "cpu%d", i);
cpudir = oprofilefs_mkdir(sb, dir, buf);
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] fix oprofile vfree warning on error 2004-09-04 17:44 [PATCH] use for_each_cpu in oprofile code Anton Blanchard @ 2004-09-04 17:46 ` Anton Blanchard 2004-09-04 17:57 ` [PATCH] Speed up oprofile buffer drain code Anton Blanchard 2004-09-05 14:32 ` [PATCH] fix oprofile vfree warning on error John Levon 0 siblings, 2 replies; 6+ messages in thread From: Anton Blanchard @ 2004-09-04 17:46 UTC (permalink / raw) To: akpm; +Cc: levon, phil.el, linux-kernel On error we can call __free_cpu_buffers with only some buffers allocated. I was getting a bunch of vfree warnings when I hit it, we should check before calling vfree. Signed-off-by: Anton Blanchard <anton@samba.org> diff -puN drivers/oprofile/cpu_buffer.c~oprofile_vfree_fix drivers/oprofile/cpu_buffer.c --- linux-2.5/drivers/oprofile/cpu_buffer.c~oprofile_vfree_fix 2004-09-05 02:37:38.223212931 +1000 +++ linux-2.5-anton/drivers/oprofile/cpu_buffer.c 2004-09-05 02:37:43.608142109 +1000 @@ -36,8 +36,10 @@ static void __free_cpu_buffers(int num) { int i; - for_each_online_cpu(i) - vfree(cpu_buffer[i].buffer); + for_each_online_cpu(i) { + if (cpu_buffer[i].buffer) + vfree(cpu_buffer[i].buffer); + } } _ ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] Speed up oprofile buffer drain code 2004-09-04 17:46 ` [PATCH] fix oprofile vfree warning on error Anton Blanchard @ 2004-09-04 17:57 ` Anton Blanchard 2004-09-05 18:26 ` John Levon 2004-09-05 14:32 ` [PATCH] fix oprofile vfree warning on error John Levon 1 sibling, 1 reply; 6+ messages in thread From: Anton Blanchard @ 2004-09-04 17:57 UTC (permalink / raw) To: akpm; +Cc: levon, phil.el, linux-kernel Hi, I noticed a large machine was doing about 400,000 context switches per second when oprofile was enabled. Upon closer inspection it looks like we were rearming the buffer sync timer without modifying the expire time. Now that we have schedule_delayed_work_on I believe we can remove the timer completely. Each cpu should be offset by 1 jiffy so they dont all fire at the same time. I bumped DEFAULT_TIMER_EXPIRE from 2 to 10 times a second to be sure we reap cpu buffers. With the following patch the same large machine gets about 4000 context switches per second. John does this look OK to you? The bump of DEFAULT_TIMER_EXPIRE was pretty arbitrary, I was seeing dropped samples before but that could be due to the timer storm before I converted it to delayed work. Signed-off-by: Anton Blanchard <anton@samba.org> diff -puN drivers/oprofile/cpu_buffer.c~oprofile_optimise drivers/oprofile/cpu_buffer.c --- gr_work/drivers/oprofile/cpu_buffer.c~oprofile_optimise 2004-09-04 12:10:18.658740785 -0500 +++ gr_work-anton/drivers/oprofile/cpu_buffer.c 2004-09-04 12:10:18.673738407 -0500 @@ -28,8 +28,8 @@ struct oprofile_cpu_buffer cpu_buffer[NR_CPUS] __cacheline_aligned; static void wq_sync_buffer(void *); -static void timer_ping(unsigned long data); -#define DEFAULT_TIMER_EXPIRE (HZ / 2) + +#define DEFAULT_TIMER_EXPIRE (HZ / 10) int timers_enabled; static void __free_cpu_buffers(int num) @@ -64,10 +64,6 @@ int alloc_cpu_buffers(void) b->sample_received = 0; b->sample_lost_overflow = 0; b->cpu = i; - init_timer(&b->timer); - b->timer.function = timer_ping; - b->timer.data = i; - b->timer.expires = jiffies + DEFAULT_TIMER_EXPIRE; INIT_WORK(&b->work, wq_sync_buffer, b); } return 0; @@ -93,7 +89,11 @@ void start_cpu_timers(void) for_each_online_cpu(i) { struct oprofile_cpu_buffer * b = &cpu_buffer[i]; - add_timer_on(&b->timer, i); + /* + * Spread the work by 1 jiffy per cpu so they dont all + * fire at once. + */ + schedule_delayed_work_on(i, &b->work, DEFAULT_TIMER_EXPIRE + i); } } @@ -107,7 +107,7 @@ void end_cpu_timers(void) for_each_online_cpu(i) { struct oprofile_cpu_buffer * b = &cpu_buffer[i]; - del_timer_sync(&b->timer); + cancel_delayed_work(&b->work); } flush_scheduled_work(); @@ -203,7 +203,13 @@ void cpu_buffer_reset(struct oprofile_cp } -/* FIXME: not guaranteed to be on our CPU */ +/* + * This serves to avoid cpu buffer overflow, and makes sure + * the task mortuary progresses + * + * By using schedule_delayed_work_on and then schedule_delayed_work + * we guarantee this will stay on the correct cpu + */ static void wq_sync_buffer(void * data) { struct oprofile_cpu_buffer * b = (struct oprofile_cpu_buffer *)data; @@ -213,24 +219,7 @@ static void wq_sync_buffer(void * data) } sync_buffer(b->cpu); - /* don't re-add the timer if we're shutting down */ - if (timers_enabled) { - del_timer_sync(&b->timer); - add_timer_on(&b->timer, b->cpu); - } -} - - -/* This serves to avoid cpu buffer overflow, and makes sure - * the task mortuary progresses - */ -static void timer_ping(unsigned long data) -{ - struct oprofile_cpu_buffer * b = &cpu_buffer[data]; - if (b->cpu != smp_processor_id()) { - printk("Timer on CPU%d, prefer CPU%d\n", - smp_processor_id(), b->cpu); - } - schedule_work(&b->work); - /* work will re-enable our timer */ + /* don't re-add the work if we're shutting down */ + if (timers_enabled) + schedule_delayed_work(&b->work, DEFAULT_TIMER_EXPIRE); } diff -puN drivers/oprofile/cpu_buffer.h~oprofile_optimise drivers/oprofile/cpu_buffer.h --- gr_work/drivers/oprofile/cpu_buffer.h~oprofile_optimise 2004-09-04 12:10:18.662740151 -0500 +++ gr_work-anton/drivers/oprofile/cpu_buffer.h 2004-09-04 12:10:18.674738249 -0500 @@ -12,7 +12,6 @@ #include <linux/types.h> #include <linux/spinlock.h> -#include <linux/timer.h> #include <linux/workqueue.h> #include <linux/cache.h> @@ -42,7 +41,6 @@ struct oprofile_cpu_buffer { unsigned long sample_received; unsigned long sample_lost_overflow; int cpu; - struct timer_list timer; struct work_struct work; } ____cacheline_aligned; _ ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Speed up oprofile buffer drain code 2004-09-04 17:57 ` [PATCH] Speed up oprofile buffer drain code Anton Blanchard @ 2004-09-05 18:26 ` John Levon 0 siblings, 0 replies; 6+ messages in thread From: John Levon @ 2004-09-05 18:26 UTC (permalink / raw) To: Anton Blanchard; +Cc: akpm, phil.el, linux-kernel On Sun, Sep 05, 2004 at 03:57:37AM +1000, Anton Blanchard wrote: > John does this look OK to you? Could we please rename timers_enable/start_cpu_timers()/end_cpu_timers() now there's no timer? Other than that looks great. thanks john ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] fix oprofile vfree warning on error 2004-09-04 17:46 ` [PATCH] fix oprofile vfree warning on error Anton Blanchard 2004-09-04 17:57 ` [PATCH] Speed up oprofile buffer drain code Anton Blanchard @ 2004-09-05 14:32 ` John Levon 2004-09-06 8:51 ` Anton Blanchard 1 sibling, 1 reply; 6+ messages in thread From: John Levon @ 2004-09-05 14:32 UTC (permalink / raw) To: Anton Blanchard; +Cc: akpm, phil.el, linux-kernel On Sun, Sep 05, 2004 at 03:46:42AM +1000, Anton Blanchard wrote: > On error we can call __free_cpu_buffers with only some buffers > allocated. I was getting a bunch of vfree warnings when I hit it, we > should check before calling vfree. Why does vfree() differ from free() / kfree() in not accepting NULL ? This seems like an interface wart. cheers john > - for_each_online_cpu(i) > - vfree(cpu_buffer[i].buffer); > + for_each_online_cpu(i) { > + if (cpu_buffer[i].buffer) > + vfree(cpu_buffer[i].buffer); > + } ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] fix oprofile vfree warning on error 2004-09-05 14:32 ` [PATCH] fix oprofile vfree warning on error John Levon @ 2004-09-06 8:51 ` Anton Blanchard 0 siblings, 0 replies; 6+ messages in thread From: Anton Blanchard @ 2004-09-06 8:51 UTC (permalink / raw) To: John Levon; +Cc: akpm, phil.el, linux-kernel > Why does vfree() differ from free() / kfree() in not accepting NULL ? > This seems like an interface wart. No idea, it does seem wrong to have this difference between kfree and vfree. Anton ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2004-09-06 8:52 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2004-09-04 17:44 [PATCH] use for_each_cpu in oprofile code Anton Blanchard 2004-09-04 17:46 ` [PATCH] fix oprofile vfree warning on error Anton Blanchard 2004-09-04 17:57 ` [PATCH] Speed up oprofile buffer drain code Anton Blanchard 2004-09-05 18:26 ` John Levon 2004-09-05 14:32 ` [PATCH] fix oprofile vfree warning on error John Levon 2004-09-06 8:51 ` Anton Blanchard
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox