From: Greg Ungerer <gerg@snapgear.com>
To: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: rth@twiddle.net, rmk@arm.linux.org.uk, bryan.wu@analog.com,
dhowells@redhat.com, gerg@uclinux.org, lethal@linux-sh.org,
wli@holomorphy.com, davem@davemloft.net,
linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org,
tglx@linutronix.de, mingo@elte.hu, torvalds@linux-foundation.org
Subject: Re: [RFC][PATCH 2/2] xtime_lock vs update_process_times
Date: Mon, 04 Feb 2008 11:29:58 +1000 [thread overview]
Message-ID: <47A66A96.6000301@snapgear.com> (raw)
In-Reply-To: <20080128104056.184943000@chello.nl>
Hi Peter,
Peter Zijlstra wrote:
> move update_process_times() out from under xtime_lock.
>
> Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
> ---
> arch/alpha/kernel/time.c | 15 ++++++++-------
> arch/arm/common/time-acorn.c | 2 --
> arch/arm/mach-at91/at91sam926x_time.c | 3 ---
> arch/arm/plat-iop/time.c | 4 ----
> arch/arm/plat-s3c24xx/time.c | 2 --
> arch/blackfin/kernel/time.c | 8 +++++---
> arch/frv/kernel/time.c | 6 ++++--
> arch/m68knommu/kernel/time.c | 12 +++++++-----
> arch/sh/kernel/time.c | 19 +++++++++++++++----
> arch/sh/kernel/timers/timer-cmt.c | 9 ---------
> arch/sh/kernel/timers/timer-mtu2.c | 2 --
> arch/sh64/kernel/time.c | 31 +++++++++++++++++--------------
> arch/sparc/kernel/pcic.c | 2 +-
> arch/sparc/kernel/time.c | 7 +++----
> 14 files changed, 60 insertions(+), 62 deletions(-)
Tested the m68knommu change. Works fine. So for that part:
Acked-by: Greg Ungerer <gerg@uclinux.org>
Regards
Greg
> Index: linux-2.6/arch/alpha/kernel/time.c
> ===================================================================
> --- linux-2.6.orig/arch/alpha/kernel/time.c
> +++ linux-2.6/arch/alpha/kernel/time.c
> @@ -119,13 +119,8 @@ irqreturn_t timer_interrupt(int irq, voi
> state.partial_tick = delta & ((1UL << FIX_SHIFT) - 1);
> nticks = delta >> FIX_SHIFT;
>
> - while (nticks > 0) {
> - do_timer(1);
> -#ifndef CONFIG_SMP
> - update_process_times(user_mode(get_irq_regs()));
> -#endif
> - nticks--;
> - }
> + if (nticks)
> + do_timer(nticks);
>
> /*
> * If we have an externally synchronized Linux clock, then update
> @@ -141,6 +136,12 @@ irqreturn_t timer_interrupt(int irq, voi
> }
>
> write_sequnlock(&xtime_lock);
> +
> +#ifndef CONFIG_SMP
> + while (nticks--)
> + update_process_times(user_mode(get_irq_regs()));
> +#endif
> +
> return IRQ_HANDLED;
> }
>
> Index: linux-2.6/arch/arm/common/time-acorn.c
> ===================================================================
> --- linux-2.6.orig/arch/arm/common/time-acorn.c
> +++ linux-2.6/arch/arm/common/time-acorn.c
> @@ -69,9 +69,7 @@ void __init ioctime_init(void)
> static irqreturn_t
> ioc_timer_interrupt(int irq, void *dev_id)
> {
> - write_seqlock(&xtime_lock);
> timer_tick();
> - write_sequnlock(&xtime_lock);
> return IRQ_HANDLED;
> }
>
> Index: linux-2.6/arch/arm/mach-at91/at91sam926x_time.c
> ===================================================================
> --- linux-2.6.orig/arch/arm/mach-at91/at91sam926x_time.c
> +++ linux-2.6/arch/arm/mach-at91/at91sam926x_time.c
> @@ -49,8 +49,6 @@ static irqreturn_t at91sam926x_timer_int
> volatile long nr_ticks;
>
> if (at91_sys_read(AT91_PIT_SR) & AT91_PIT_PITS) { /* This is a shared interrupt */
> - write_seqlock(&xtime_lock);
> -
> /* Get number to ticks performed before interrupt and clear PIT interrupt */
> nr_ticks = PIT_PICNT(at91_sys_read(AT91_PIT_PIVR));
> do {
> @@ -58,7 +56,6 @@ static irqreturn_t at91sam926x_timer_int
> nr_ticks--;
> } while (nr_ticks);
>
> - write_sequnlock(&xtime_lock);
> return IRQ_HANDLED;
> } else
> return IRQ_NONE; /* not handled */
> Index: linux-2.6/arch/arm/plat-iop/time.c
> ===================================================================
> --- linux-2.6.orig/arch/arm/plat-iop/time.c
> +++ linux-2.6/arch/arm/plat-iop/time.c
> @@ -57,8 +57,6 @@ unsigned long iop_gettimeoffset(void)
> static irqreturn_t
> iop_timer_interrupt(int irq, void *dev_id)
> {
> - write_seqlock(&xtime_lock);
> -
> write_tisr(1);
>
> while ((signed long)(next_jiffy_time - read_tcr1())
> @@ -67,8 +65,6 @@ iop_timer_interrupt(int irq, void *dev_i
> next_jiffy_time -= ticks_per_jiffy;
> }
>
> - write_sequnlock(&xtime_lock);
> -
> return IRQ_HANDLED;
> }
>
> Index: linux-2.6/arch/arm/plat-s3c24xx/time.c
> ===================================================================
> --- linux-2.6.orig/arch/arm/plat-s3c24xx/time.c
> +++ linux-2.6/arch/arm/plat-s3c24xx/time.c
> @@ -130,9 +130,7 @@ static unsigned long s3c2410_gettimeoffs
> static irqreturn_t
> s3c2410_timer_interrupt(int irq, void *dev_id)
> {
> - write_seqlock(&xtime_lock);
> timer_tick();
> - write_sequnlock(&xtime_lock);
> return IRQ_HANDLED;
> }
>
> Index: linux-2.6/arch/blackfin/kernel/time.c
> ===================================================================
> --- linux-2.6.orig/arch/blackfin/kernel/time.c
> +++ linux-2.6/arch/blackfin/kernel/time.c
> @@ -137,9 +137,6 @@ irqreturn_t timer_interrupt(int irq, voi
>
> do_timer(1);
>
> -#ifndef CONFIG_SMP
> - update_process_times(user_mode(get_irq_regs()));
> -#endif
> profile_tick(CPU_PROFILING);
>
> /*
> @@ -161,6 +158,11 @@ irqreturn_t timer_interrupt(int irq, voi
> last_rtc_update = xtime.tv_sec - 600;
> }
> write_sequnlock(&xtime_lock);
> +
> +#ifndef CONFIG_SMP
> + update_process_times(user_mode(get_irq_regs()));
> +#endif
> +
> return IRQ_HANDLED;
> }
>
> Index: linux-2.6/arch/frv/kernel/time.c
> ===================================================================
> --- linux-2.6.orig/arch/frv/kernel/time.c
> +++ linux-2.6/arch/frv/kernel/time.c
> @@ -63,6 +63,7 @@ static irqreturn_t timer_interrupt(int i
> /* last time the cmos clock got updated */
> static long last_rtc_update = 0;
>
> + profile_tick(CPU_PROFILING);
> /*
> * Here we are in the timer irq handler. We just have irqs locally
> * disabled but we don't know if the timer_bh is running on the other
> @@ -73,8 +74,6 @@ static irqreturn_t timer_interrupt(int i
> write_seqlock(&xtime_lock);
>
> do_timer(1);
> - update_process_times(user_mode(get_irq_regs()));
> - profile_tick(CPU_PROFILING);
>
> /*
> * If we have an externally synchronized Linux clock, then update
> @@ -99,6 +98,9 @@ static irqreturn_t timer_interrupt(int i
> #endif /* CONFIG_HEARTBEAT */
>
> write_sequnlock(&xtime_lock);
> +
> + update_process_times(user_mode(get_irq_regs()));
> +
> return IRQ_HANDLED;
> }
>
> Index: linux-2.6/arch/m68knommu/kernel/time.c
> ===================================================================
> --- linux-2.6.orig/arch/m68knommu/kernel/time.c
> +++ linux-2.6/arch/m68knommu/kernel/time.c
> @@ -43,14 +43,12 @@ irqreturn_t arch_timer_interrupt(int irq
> /* last time the cmos clock got updated */
> static long last_rtc_update=0;
>
> + if (current->pid)
> + profile_tick(CPU_PROFILING);
> +
> write_seqlock(&xtime_lock);
>
> do_timer(1);
> -#ifndef CONFIG_SMP
> - update_process_times(user_mode(get_irq_regs()));
> -#endif
> - if (current->pid)
> - profile_tick(CPU_PROFILING);
>
> /*
> * If we have an externally synchronized Linux clock, then update
> @@ -91,6 +89,10 @@ irqreturn_t arch_timer_interrupt(int irq
> #endif /* CONFIG_HEARTBEAT */
>
> write_sequnlock(&xtime_lock);
> +
> +#ifndef CONFIG_SMP
> + update_process_times(user_mode(get_irq_regs()));
> +#endif
> return(IRQ_HANDLED);
> }
>
> Index: linux-2.6/arch/sh/kernel/time.c
> ===================================================================
> --- linux-2.6.orig/arch/sh/kernel/time.c
> +++ linux-2.6/arch/sh/kernel/time.c
> @@ -120,10 +120,6 @@ static long last_rtc_update;
> */
> void handle_timer_tick(void)
> {
> - do_timer(1);
> -#ifndef CONFIG_SMP
> - update_process_times(user_mode(get_irq_regs()));
> -#endif
> if (current->pid)
> profile_tick(CPU_PROFILING);
>
> @@ -133,6 +129,16 @@ void handle_timer_tick(void)
> #endif
>
> /*
> + * Here we are in the timer irq handler. We just have irqs locally
> + * disabled but we don't know if the timer_bh is running on the other
> + * CPU. We need to avoid to SMP race with it. NOTE: we don' t need
> + * the irq version of write_lock because as just said we have irq
> + * locally disabled. -arca
> + */
> + write_seqlock(&xtime_lock);
> + do_timer(1);
> +
> + /*
> * If we have an externally synchronized Linux clock, then update
> * RTC clock accordingly every ~11 minutes. Set_rtc_mmss() has to be
> * called as close as possible to 500 ms before the new second starts.
> @@ -147,6 +153,11 @@ void handle_timer_tick(void)
> /* do it again in 60s */
> last_rtc_update = xtime.tv_sec - 600;
> }
> + write_sequnlock(&xtime_lock);
> +
> +#ifndef CONFIG_SMP
> + update_process_times(user_mode(get_irq_regs()));
> +#endif
> }
> #endif /* !CONFIG_GENERIC_CLOCKEVENTS */
>
> Index: linux-2.6/arch/sh/kernel/timers/timer-cmt.c
> ===================================================================
> --- linux-2.6.orig/arch/sh/kernel/timers/timer-cmt.c
> +++ linux-2.6/arch/sh/kernel/timers/timer-cmt.c
> @@ -98,16 +98,7 @@ static irqreturn_t cmt_timer_interrupt(i
> timer_status &= ~0x80;
> ctrl_outw(timer_status, CMT_CMCSR_0);
>
> - /*
> - * Here we are in the timer irq handler. We just have irqs locally
> - * disabled but we don't know if the timer_bh is running on the other
> - * CPU. We need to avoid to SMP race with it. NOTE: we don' t need
> - * the irq version of write_lock because as just said we have irq
> - * locally disabled. -arca
> - */
> - write_seqlock(&xtime_lock);
> handle_timer_tick();
> - write_sequnlock(&xtime_lock);
>
> return IRQ_HANDLED;
> }
> Index: linux-2.6/arch/sh/kernel/timers/timer-mtu2.c
> ===================================================================
> --- linux-2.6.orig/arch/sh/kernel/timers/timer-mtu2.c
> +++ linux-2.6/arch/sh/kernel/timers/timer-mtu2.c
> @@ -100,9 +100,7 @@ static irqreturn_t mtu2_timer_interrupt(
> ctrl_outb(timer_status, MTU2_TSR_1);
>
> /* Do timer tick */
> - write_seqlock(&xtime_lock);
> handle_timer_tick();
> - write_sequnlock(&xtime_lock);
>
> return IRQ_HANDLED;
> }
> Index: linux-2.6/arch/sh64/kernel/time.c
> ===================================================================
> --- linux-2.6.orig/arch/sh64/kernel/time.c
> +++ linux-2.6/arch/sh64/kernel/time.c
> @@ -285,15 +285,22 @@ static long last_rtc_update = 0;
> static inline void do_timer_interrupt(void)
> {
> unsigned long long current_ctc;
> +
> + if (current->pid)
> + profile_tick(CPU_PROFILING);
> +
> + /*
> + * Here we are in the timer irq handler. We just have irqs locally
> + * disabled but we don't know if the timer_bh is running on the other
> + * CPU. We need to avoid to SMP race with it. NOTE: we don' t need
> + * the irq version of write_lock because as just said we have irq
> + * locally disabled. -arca
> + */
> + write_lock(&xtime_lock);
> asm ("getcon cr62, %0" : "=r" (current_ctc));
> ctc_last_interrupt = (unsigned long) current_ctc;
>
> do_timer(1);
> -#ifndef CONFIG_SMP
> - update_process_times(user_mode(get_irq_regs()));
> -#endif
> - if (current->pid)
> - profile_tick(CPU_PROFILING);
>
> #ifdef CONFIG_HEARTBEAT
> {
> @@ -317,6 +324,11 @@ static inline void do_timer_interrupt(vo
> else
> last_rtc_update = xtime.tv_sec - 600; /* do it again in 60 s */
> }
> + write_unlock(&xtime_lock);
> +
> +#ifndef CONFIG_SMP
> + update_process_times(user_mode(get_irq_regs()));
> +#endif
> }
>
> /*
> @@ -333,16 +345,7 @@ static irqreturn_t timer_interrupt(int i
> timer_status &= ~0x100;
> ctrl_outw(timer_status, TMU0_TCR);
>
> - /*
> - * Here we are in the timer irq handler. We just have irqs locally
> - * disabled but we don't know if the timer_bh is running on the other
> - * CPU. We need to avoid to SMP race with it. NOTE: we don' t need
> - * the irq version of write_lock because as just said we have irq
> - * locally disabled. -arca
> - */
> - write_lock(&xtime_lock);
> do_timer_interrupt();
> - write_unlock(&xtime_lock);
>
> return IRQ_HANDLED;
> }
> Index: linux-2.6/arch/sparc/kernel/pcic.c
> ===================================================================
> --- linux-2.6.orig/arch/sparc/kernel/pcic.c
> +++ linux-2.6/arch/sparc/kernel/pcic.c
> @@ -713,10 +713,10 @@ static irqreturn_t pcic_timer_handler (i
> write_seqlock(&xtime_lock); /* Dummy, to show that we remember */
> pcic_clear_clock_irq();
> do_timer(1);
> + write_sequnlock(&xtime_lock);
> #ifndef CONFIG_SMP
> update_process_times(user_mode(get_irq_regs()));
> #endif
> - write_sequnlock(&xtime_lock);
> return IRQ_HANDLED;
> }
>
> Index: linux-2.6/arch/sparc/kernel/time.c
> ===================================================================
> --- linux-2.6.orig/arch/sparc/kernel/time.c
> +++ linux-2.6/arch/sparc/kernel/time.c
> @@ -128,10 +128,6 @@ irqreturn_t timer_interrupt(int irq, voi
> clear_clock_irq();
>
> do_timer(1);
> -#ifndef CONFIG_SMP
> - update_process_times(user_mode(get_irq_regs()));
> -#endif
> -
>
> /* Determine when to update the Mostek clock. */
> if (ntp_synced() &&
> @@ -145,6 +141,9 @@ irqreturn_t timer_interrupt(int irq, voi
> }
> write_sequnlock(&xtime_lock);
>
> +#ifndef CONFIG_SMP
> + update_process_times(user_mode(get_irq_regs()));
> +#endif
> return IRQ_HANDLED;
> }
>
>
> --
>
>
--
------------------------------------------------------------------------
Greg Ungerer -- Chief Software Dude EMAIL: gerg@snapgear.com
Secure Computing Corporation PHONE: +61 7 3435 2888
825 Stanley St, FAX: +61 7 3891 3630
Woolloongabba, QLD, 4102, Australia WEB: http://www.SnapGear.com
next prev parent reply other threads:[~2008-02-04 1:30 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-01-28 10:39 [RFC][PATCH 0/2] hrtimer breakage Peter Zijlstra
2008-01-28 10:39 ` [RFC][PATCH 1/2] Fix many ARM timer handlers Peter Zijlstra
2008-01-28 10:39 ` [RFC][PATCH 2/2] xtime_lock vs update_process_times Peter Zijlstra
2008-01-28 12:20 ` Russell King
2008-01-28 12:29 ` Peter Zijlstra
2008-01-28 13:40 ` Russell King
2008-02-04 1:29 ` Greg Ungerer [this message]
2008-02-13 21:18 ` Ivan Kokshaysky
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=47A66A96.6000301@snapgear.com \
--to=gerg@snapgear.com \
--cc=a.p.zijlstra@chello.nl \
--cc=bryan.wu@analog.com \
--cc=davem@davemloft.net \
--cc=dhowells@redhat.com \
--cc=gerg@uclinux.org \
--cc=lethal@linux-sh.org \
--cc=linux-arch@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=rmk@arm.linux.org.uk \
--cc=rth@twiddle.net \
--cc=tglx@linutronix.de \
--cc=torvalds@linux-foundation.org \
--cc=wli@holomorphy.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).