linux-kernel - Re: [RFC][PATCH 2/3] clocksource: scx200_hrt: Convert scx200 to use clocksource_register

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: <CAJfuBxwBfB7V_gikW44rTNAYvxKZwORwzdRrvyZM19feFwcwDQ@mail.gmail.com>
Date:	Tue, 24 Jan 2012 11:53:32 -0700
From:	Jim Cromie <jim.cromie@...il.com>
To:	John Stultz <john.stultz@...aro.org>
Cc:	lkml <linux-kernel@...r.kernel.org>
Subject: Re: [RFC][PATCH 2/3] clocksource: scx200_hrt: Convert scx200 to use clocksource_register_hz

On Mon, Jan 23, 2012 at 5:05 PM, John Stultz <john.stultz@...aro.org> wrote:
> Converts the scx200 clocksource to using clocksource_register_hz.
>
> This wasn't a straight conversion, so would like some extra review
> by the maintainers.
>

hi John,

very nice.  Tested with mhz27 too.

Signed-off-by: Jim Cromie <jim.cromie@...il.com>

It eliminates this noise that Ive started seeing recently (for some
value of recent)

enabling scx200 high-res timer (1 MHz +0 ppm)
------------[ cut here ]------------
WARNING: at /home/jimc/projects/lx/linux-2.6/kernel/time/clocksource.c:740
clocksource_register+0x78/0xbd()
Clocksource scx200_hrt might overflow on 11% adjustment
Modules linked in: scx200_hrt(+) scx200 pc8736x_gpio nsc_gpio pc87360
hwmon_vid scx200_acb ohci_hcd pata_sc1200
Pid: 291, comm: modprobe Not tainted 3.2.0-skd-00063-g296c136 #256
Call Trace:
 [<c01201a1>] warn_slowpath_common+0x7c/0x91
 [<c0140266>] ? clocksource_register+0x78/0xbd
 [<c0140266>] ? clocksource_register+0x78/0xbd
 [<c0120241>] warn_slowpath_fmt+0x33/0x35
 [<c0140266>] clocksource_register+0x78/0xbd
 [<c886412d>] init_hrt_clocksource+0x12d/0x139 [scx200_hrt]
 [<c010108b>] do_one_initcall+0x8b/0x138
 [<c8864000>] ? 0xc8863fff
 [<c014a6db>] sys_init_module+0x13ef/0x160e
 [<c042015c>] syscall_call+0x7/0xb
---[ end trace 69c8ca76cf89ab2d ]---
Switching to clocksource scx200_hrt
Loading kernel modules...chrdev_alloc_region: chrdev_alloc_region
reserves a dynamic major, but was called with major:3077.  Caller
should pass major:0 for this.
done.
------------[ cut here ]------------
WARNING: at /home/jimc/projects/lx/linux-2.6/kernel/time/timekeeping.c:863
do_timer+0x56f/0x6ae()
Adjusting scx200_hrt more then 11% (-100663808 vs 360710144)

NB - s/then/than/   Im attaching a patch for that.
Also, "when then" is a little awkward.
		/*
		 * Have to be careful here. If we're in oneshot mode,
		 * before we call tick_periodic() in a loop, we need
		 * to be sure we're using a real hardware clocksource.
		 * Otherwise we could get trapped in an infinite
		 * loop, as the tick_periodic() increments jiffies,
		 * when then will increment time, posibly causing
		 * the loop to trigger again and again.
		 */


Modules linked in: scx200_gpio scx200_hrt scx200 pc8736x_gpio nsc_gpio
pc87360 hwmon_vid scx200_acb ohci_hcd pata_sc1200
Pid: 58, comm: collector Tainted: G        W    3.2.0-skd-00063-g296c136 #256
Call Trace:
 [<c01201a1>] warn_slowpath_common+0x7c/0x91
 [<c013ebbe>] ? do_timer+0x56f/0x6ae
 [<c013ebbe>] ? do_timer+0x56f/0x6ae
 [<c0120241>] warn_slowpath_fmt+0x33/0x35
 [<c013ebbe>] do_timer+0x56f/0x6ae
 [<c014319b>] tick_do_update_jiffies64+0xc0/0xee
 [<c01432e1>] tick_sched_timer+0x45/0x9e
 [<c0139c01>] ? __remove_hrtimer+0x2a/0x87
 [<c0139d81>] __run_hrtimer+0x93/0x131
 [<c014329c>] ? tick_nohz_handler+0xd3/0xd3
 [<c013a549>] hrtimer_interrupt+0xe1/0x1f4
 [<c010346d>] timer_interrupt+0x18/0x1f
 [<c015fbcf>] handle_irq_event_percpu+0x4a/0x197
 [<c014bcfe>] ? pm_qos_request+0xe/0x16
 [<c0104155>] ? mask_and_ack_8259A+0x77/0xde
 [<c0161a3c>] ? unmask_irq+0x23/0x23
 [<c015fd58>] handle_irq_event+0x3c/0x54
 [<c0161a8e>] handle_level_irq+0x52/0x80
 <IRQ>  [<c0102f2c>] ? do_IRQ+0x39/0x87
 [<c02b561a>] ? disk_seqf_start+0x2e/0x84
 [<c0424c70>] ? common_interrupt+0x30/0x40
 [<c01c007b>] ? do_vfs_ioctl+0x30c/0x481
 [<c01f21e9>] ? proc_readdir_de+0x166/0x186
 [<c01c0324>] ? filldir64+0xcc/0xcc
 [<c01f222a>] ? proc_readdir+0x21/0x25
 [<c01c0324>] ? filldir64+0xcc/0xcc
 [<c01edbe9>] ? proc_root_readdir+0x1f/0x41
 [<c01c0509>] ? vfs_readdir+0x6a/0x8f
 [<c01c0324>] ? filldir64+0xcc/0xcc
 [<c01c05e7>] ? sys_getdents+0x63/0x9f
 [<c042015c>] ? syscall_call+0x7/0xb
 [<c0420000>] ? rwsem_down_failed_common+0x5f/0xbc
---[ end trace 69c8ca76cf89ab2e ]---




> CC: Jim Cromie <jim.cromie@...il.com>
> Signed-off-by: John Stultz <john.stultz@...aro.org>
> ---
>  drivers/clocksource/scx200_hrt.c |   19 ++++++-------------
>  1 files changed, 6 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/clocksource/scx200_hrt.c b/drivers/clocksource/scx200_hrt.c
> index 27f4d96..24f884c 100644
> --- a/drivers/clocksource/scx200_hrt.c
> +++ b/drivers/clocksource/scx200_hrt.c
> @@ -49,9 +49,6 @@ static cycle_t read_hrt(struct clocksource *cs)
>        return (cycle_t) inl(scx200_cb_base + SCx200_TIMER_OFFSET);
>  }
>
> -#define HRT_SHIFT_1    22
> -#define HRT_SHIFT_27   26
> -
>  static struct clocksource cs_hrt = {
>        .name           = "scx200_hrt",
>        .rating         = 250,
> @@ -63,6 +60,7 @@ static struct clocksource cs_hrt = {
>
>  static int __init init_hrt_clocksource(void)
>  {
> +       u32 freq;
>        /* Make sure scx200 has initialized the configuration block */
>        if (!scx200_cb_present())
>                return -ENODEV;
> @@ -79,19 +77,14 @@ static int __init init_hrt_clocksource(void)
>        outb(HR_TMEN | (mhz27 ? HR_TMCLKSEL : 0),
>             scx200_cb_base + SCx200_TMCNFG_OFFSET);
>
> -       if (mhz27) {
> -               cs_hrt.shift = HRT_SHIFT_27;
> -               cs_hrt.mult = clocksource_hz2mult((HRT_FREQ + ppm) * 27,
> -                                                 cs_hrt.shift);
> -       } else {
> -               cs_hrt.shift = HRT_SHIFT_1;
> -               cs_hrt.mult = clocksource_hz2mult(HRT_FREQ + ppm,
> -                                                 cs_hrt.shift);
> -       }
> +       freq = (HRT_FREQ + ppm);
> +       if (mhz27)
> +               freq *= 27;
> +
>        printk(KERN_INFO "enabling scx200 high-res timer (%s MHz +%d ppm)\n",
>                mhz27 ? "27":"1", ppm);
>
> -       return clocksource_register(&cs_hrt);
> +       return clocksource_register_hz(&cs_hrt, freq);
>  }
>
>  module_init(init_hrt_clocksource);
> --
> 1.7.3.2.146.gca209
>

View attachment "0001-time-s-then-than-where-appropriate.patch" of type "text/x-patch" (4037 bytes)