linux-kernel - Re: register_timer_hook use in arch/sh/oprofile

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]

Message-ID: <20090624112929.GB2079@linux-sh.org>
Date:	Wed, 24 Jun 2009 20:29:29 +0900
From:	Paul Mundt <lethal@...ux-sh.org>
To:	Martin Schwidefsky <schwidefsky@...ibm.com>
Cc:	Ingo Molnar <mingo@...e.hu>,
	linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: register_timer_hook use in arch/sh/oprofile

On Wed, Jun 24, 2009 at 01:11:04PM +0200, Martin Schwidefsky wrote:
> Hi Paul,
> following Ingo's suggestion I'm working on a patch to use a per-cpu
> hrtimer instead of the timer_hook for the oprofile ticks. Given that
> oprofile is the only user of the timer_hook I want to remove it
> completely. That way I came across the register_timer_hook call in
> arch/sh/oprofile/op_model_sh7750.c. Did this ever work? The startup
> of oprofile is done in two steps, on module load oprofile_init is
> called, on profiling start oprofile_start is called.
> Now for SH7750 this happens:
> 
> oprofile_init()
> 	...
> 	oprofile_arch_init(&oprofile_ops);
> 		...
> 		lmodel->init(); 
> 		/* init() is sh7750_ppc_init for CPU_SH7750/CPU_7750S */
> 			sh7750_ppc_reset();
> 			register_timer_hook(sh7750_timer_notify);
> 			...
> 		...
> 	oprofile_timer_init(&oprofile_ops);
> 		...
> 		ops->start = timer_start;
> 		...
> 	...
> 
> oprofile_start()
> 	...
> 	oprofile_ops.start();
> 	/* start() is timer_start set by oprofile_timer_init */
> 		register_timer_hook(timer_notify);
> 	...
> 
> As far as I can see the second register_timer_hook will fail
> because sh7750_timer_notify is already registered. That will cause
> oprofile_start to fail with -EBUSY, no?
> 
No. oprofile_timer_init() is only entered if the performance counters
fail to register in the SH7750 case, so there is only one timer hook user
at a time:

static int __init oprofile_init(void)
{
        int err;

        err = oprofile_arch_init(&oprofile_ops);

        if (err < 0 || timer) {
                printk(KERN_INFO "oprofile: using timer interrupt.\n");
                oprofile_timer_init(&oprofile_ops);
        }
...

The sh7750 performance counter code has always purposely abused the timer
interrupt as its profiling interrupt given that the counters themselves do not
generate IRQs on their own. There are a couple of 64-bit counters that can
count various events, but have no real event associated with them. As long as
they are periodically read, they return accurate statistics, but the granularity
is never better than the timer IRQ.

In practice oprofile has never been a good fit for these sorts of counters, so
this has fairly limited use. If there's a way to wiggle these types of counters
in to the new perf_counter API, then I'll convert that over and just kill the
old oprofile driver off completely. Barring that, I'll just end up converting it
over to hrtimers as well, so don't let that stop you from ripping out the timer
hook bits.

Most of this code predates hrtimers anyways, and it also predates the timer
hook, which is only something that we converted to some years back.
--
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/