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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 19 Oct 2011 18:33:28 +0200
From:	Robert Richter <robert.richter@....com>
To:	Ingo Molnar <mingo@...e.hu>
CC:	LKML <linux-kernel@...r.kernel.org>,
	oprofile-list <oprofile-list@...ts.sourceforge.net>
Subject: Re: [PATCH 4/5] oprofile: Remove exit function for timer mode

On 18.10.11 02:13:47, Ingo Molnar wrote:
> 
> * Robert Richter <robert.richter@....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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/