lkml.org 
[lkml]   [2011]   [Oct]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH 4/5] oprofile: Remove exit function for timer mode
    On 18.10.11 02:13:47, Ingo Molnar wrote:
    >
    > * Robert Richter <robert.richter@amd.com> wrote:
    >
    > > /* failed */
    > > - if (timer_mode)
    > > - oprofile_timer_exit();
    > > - else
    > > + if (!timer_mode)
    > > oprofile_arch_exit();
    >
    > > - if (timer_mode)
    > > - oprofile_timer_exit();
    > > - else
    > > + if (!timer_mode)
    > > oprofile_arch_exit();
    >
    > ... and the ugliness didnt improve much here either.
    >
    > Any reason why oprofile_arch_exit() couldnt check timer_mode?

    The two fixes are the smallest possible solution for stable kernels.
    ... and they don't look nice.

    Moving the check to oprofile_arch_exit() would mean to make timer_mode
    global and to touch all oprofile_arch_exit() functions for all archs.
    But we could get rid of timer_mode if the exit function would be part
    of struct oprofile_operations. For this we would have to touch the
    code for all architectures too, and I actually want to avoid this. But
    it would be a nice solution.

    Anyway, I have some improvements in patch #5 that make the code looks a
    bit better. See below for a preview, will post a v2 patch set after
    testing. As said, I could remove variable timer_mode completely by
    moving exit functions to oprofile_operations. This could be on top of
    the changes below.

    -Robert



    diff --git a/drivers/oprofile/oprof.c b/drivers/oprofile/oprof.c
    index f7cd069..d2994e7 100644
    --- a/drivers/oprofile/oprof.c
    +++ b/drivers/oprofile/oprof.c
    @@ -246,26 +246,23 @@ static int __init oprofile_init(void)
    int err;

    /* always init architecture to setup backtrace support */
    + timer_mode = 0;
    err = oprofile_arch_init(&oprofile_ops);
    -
    - timer_mode = err || timer; /* fall back to timer mode on errors */
    - if (timer_mode) {
    - if (!err)
    - oprofile_arch_exit();
    - err = oprofile_timer_init(&oprofile_ops);
    - if (err)
    - return err;
    + if (!err) {
    + if (!timer && oprofilefs_register())
    + return 0;
    + oprofile_arch_exit();
    }

    - err = oprofilefs_register();
    - if (!err)
    - return 0;
    -
    - /* failed */
    - if (!timer_mode)
    - oprofile_arch_exit();
    + /* setup timer mode */
    + timer_mode = 1;
    + err = op_nmi_timer_init(&oprofile_ops);
    + if (err)
    + err = oprofile_timer_init(&oprofile_ops);
    + if (err)
    + return err;

    - return err;
    + return oprofilefs_register();
    }





    --
    Advanced Micro Devices, Inc.
    Operating System Research Center



    \
     
     \ /
      Last update: 2011-10-19 18:37    [W:0.027 / U:1.792 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site