Re: [PATCH 4/5] oprofile: Remove exit function for timer mode

Robert Richter
2011-10-19T16:35:34Z

On 18.10.11 02:13:47, Ingo Molnar wrote:
>
> * Robert Richter <robert.richter@amd.com> wrote:
>
> > /* failed */
> > - if (timer_mode)
> > - oprofile_timer_exit();
> > - else
> > + if (!timer_mode)
> > oprofile_arch_exit();
>
> > - if (timer_mode)
> > - oprofile_timer_exit();
> > - else
> > + if (!timer_mode)
> > oprofile_arch_exit();
>
> ... and the ugliness didnt improve much here either.
>
> Any reason why oprofile_arch_exit() couldnt check timer_mode?

The two fixes are the smallest possible solution for stable kernels.
... and they don't look nice.

Moving the check to oprofile_arch_exit() would mean to make timer_mode
global and to touch all oprofile_arch_exit() functions for all archs.
But we could get rid of timer_mode if the exit function would be part
of struct oprofile_operations. For this we would have to touch the
code for all architectures too, and I actually want to avoid this. But
it would be a nice solution.

Anyway, I have some improvements in patch #5 that make the code looks a
bit better. See below for a preview, will post a v2 patch set after
testing. As said, I could remove variable timer_mode completely by
moving exit functions to oprofile_operations. This could be on top of
the changes below.

-Robert

diff --git a/drivers/oprofile/oprof.c b/drivers/oprofile/oprof.c
index f7cd069..d2994e7 100644
--- a/drivers/oprofile/oprof.c
+++ b/drivers/oprofile/oprof.c
@@ -246,26 +246,23 @@ static int __init oprofile_init(void)
int err;

/* always init architecture to setup backtrace support */
+ timer_mode = 0;
err = oprofile_arch_init(&oprofile_ops);
-
- timer_mode = err || timer; /* fall back to timer mode on errors */
- if (timer_mode) {
- if (!err)
- oprofile_arch_exit();
- err = oprofile_timer_init(&oprofile_ops);
- if (err)
- return err;
+ if (!err) {
+ if (!timer && oprofilefs_register())
+ return 0;
+ oprofile_arch_exit();
}

- err = oprofilefs_register();
- if (!err)
- return 0;
-
- /* failed */
- if (!timer_mode)
- oprofile_arch_exit();
+ /* setup timer mode */
+ timer_mode = 1;
+ err = op_nmi_timer_init(&oprofile_ops);
+ if (err)
+ err = oprofile_timer_init(&oprofile_ops);
+ if (err)
+ return err;

- return err;
+ return oprofilefs_register();
}

--
Advanced Micro Devices, Inc.
Operating System Research Center