lkml.org 
[lkml]   [2011]   [Jan]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRE: [PATCH] oprofile: add SMP barriers for hrtimer hotplug code
    > -----Original Message-----
    > From: Will Deacon [mailto:will.deacon@arm.com]
    > Sent: Monday, January 31, 2011 8:38 PM
    > To: linux-kernel@vger.kernel.org; oprofile-list@lists.sf.net
    > Cc: Will Deacon; Santosh Shilimkar; Robert Richter
    > Subject: [PATCH] oprofile: add SMP barriers for hrtimer hotplug code
    >
    > OProfile uses a CPU notifier to start and stop any hrtimers when
    > CPUs change
    > between ONLINE and DEAD. A static int ctr_running is used to keep
    > track of
    > the counter state.
    >
    > This can lead to problems where writes to the state variable are re-
    > ordered
    > with repect to reads of the variable occurring on other CPUs,
    > meaning that
    > __oprofile_hrtimer_start may read ctr_running as 0 and not
    > initialise the
    > hrtimer. Potential deadlock can occur in __oprofile_hrtimer_stop
    > because
    > lock_hrtimer_base will poll until timer->base != NULL, which will
    > never
    > happen.
    >
    > This patch adds smp_mb()s to ensure that ctr_running mirrors the
    > correct
    > counter state.
    >
    > Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
    > Cc: Robert Richter <robert.richter@amd.com>
    > Signed-off-by: Will Deacon <will.deacon@arm.com>
    > ---
    > drivers/oprofile/timer_int.c | 2 ++
    > 1 files changed, 2 insertions(+), 0 deletions(-)
    >
    > diff --git a/drivers/oprofile/timer_int.c
    > b/drivers/oprofile/timer_int.c
    > index 0107251..38c1e1b 100644
    > --- a/drivers/oprofile/timer_int.c
    > +++ b/drivers/oprofile/timer_int.c
    > @@ -48,6 +48,7 @@ static int oprofile_hrtimer_start(void)
    > {
    > get_online_cpus();
    > ctr_running = 1;
    > + smp_mb();
    Is smp_wmb() more appropriate ?

    > on_each_cpu(__oprofile_hrtimer_start, NULL, 1);
    > put_online_cpus();
    > return 0;
    > @@ -70,6 +71,7 @@ static void oprofile_hrtimer_stop(void)
    > get_online_cpus();
    > for_each_online_cpu(cpu)
    > __oprofile_hrtimer_stop(cpu);
    > + smp_mb();
    > ctr_running = 0;
    > put_online_cpus();
    > }
    > --
    Otherwise patch is fine.
    Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>


    \
     
     \ /
      Last update: 2011-01-31 16:45    [W:0.027 / U:90.672 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site