| 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
| ||
|
Date: Mon, 04 Feb 2008 11:29:58 +1000
From: Greg Ungerer <gerg@...pgear.com>
To: Peter Zijlstra <a.p.zijlstra@...llo.nl>
CC: rth@...ddle.net, rmk@....linux.org.uk, bryan.wu@...log.com,
dhowells@...hat.com, gerg@...inux.org, lethal@...ux-sh.org,
wli@...omorphy.com, davem@...emloft.net,
linux-kernel@...r.kernel.org, linux-arch@...r.kernel.org,
tglx@...utronix.de, mingo@...e.hu, torvalds@...ux-foundation.org
Subject: Re: [RFC][PATCH 2/2] xtime_lock vs update_process_times
Hi Peter,
Peter Zijlstra wrote:
> move update_process_times() out from under xtime_lock.
>
> Signed-off-by: Peter Zijlstra <a.p.zijlstra@...llo.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@...inux.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@...pgear.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
--
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/
Powered by blists - more mailing lists