[git pull] OProfile fixes for v2.6.28 - Kernel

This is a discussion on [git pull] OProfile fixes for v2.6.28 - Kernel ; Hi Ingo, please pull oprofile fixes for 2.6.28 for tip regression: git://git.kernel.org/pub/scm/linux/kernel/git/rric/oprofile.git oprofile-for-tip Thanks, -Robert Andi Kleen (1): oprofile: Fix p6 counter overflow check Carl Love (2): Cell OProfile: Incorrect local array size in activate spu profiling function Cell OProfile: ...

+ Reply to Thread
Results 1 to 13 of 13

Thread: [git pull] OProfile fixes for v2.6.28

  1. [git pull] OProfile fixes for v2.6.28

    Hi Ingo,

    please pull oprofile fixes for 2.6.28 for tip regression:

    git://git.kernel.org/pub/scm/linux/kernel/git/rric/oprofile.git oprofile-for-tip

    Thanks,

    -Robert

    Andi Kleen (1):
    oprofile: Fix p6 counter overflow check

    Carl Love (2):
    Cell OProfile: Incorrect local array size in activate spu profiling function
    Cell OProfile: Incorrect local array size in activate spu profiling function

    Jesper Dangaard Brouer (1):
    Change UTF8 chars in Kconfig help text about Oprofile AMD barcelona

    Nick Piggin (1):
    oprofile: fix memory ordering

    Robert Richter (1):
    Revert "Cell OProfile: Incorrect local array size in activate spu profiling function"

    --
    Advanced Micro Devices, Inc.
    Operating System Research Center
    email: robert.richter@amd.com

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. Re: [git pull] OProfile fixes for v2.6.28


    * Robert Richter wrote:

    > Hi Ingo,
    >
    > please pull oprofile fixes for 2.6.28 for tip regression:
    >
    > git://git.kernel.org/pub/scm/linux/kernel/git/rric/oprofile.git oprofile-for-tip
    >
    > Thanks,


    pulled, thanks Robert!

    Ingo
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. Re: [git pull] OProfile fixes for v2.6.28

    Robert Richter a écrit :
    > Hi Ingo,
    >
    > please pull oprofile fixes for 2.6.28 for tip regression:
    >
    > git://git.kernel.org/pub/scm/linux/kernel/git/rric/oprofile.git oprofile-for-tip
    >
    > Thanks,
    >
    > -Robert
    >
    > Andi Kleen (1):
    > oprofile: Fix p6 counter overflow check
    >
    > Carl Love (2):
    > Cell OProfile: Incorrect local array size in activate spu profiling function
    > Cell OProfile: Incorrect local array size in activate spu profiling function
    >
    > Jesper Dangaard Brouer (1):
    > Change UTF8 chars in Kconfig help text about Oprofile AMD barcelona
    >
    > Nick Piggin (1):
    > oprofile: fix memory ordering
    >
    > Robert Richter (1):
    > Revert "Cell OProfile: Incorrect local array size in activate spuprofiling function"
    >


    Hi Robert

    I am still trying to find why oprofile stops after some millions of samples on my machine.

    (All versions of linux supporting oprofile/NMI on this machine have this problem)
    Note : I am using a 32bit kernel.

    While doing code review I found this bug.

    Impact: 32bit kernels, memory corruption

    [PATCH] oprofile: fix an overflow in ppro code

    reset_value was changed from long to u64 in commit b99170288421c79f0c2efa8b33e26e65f4bb7fb8
    (oprofile: Implement Intel architectural perfmon support)

    But dynamic allocation of this array use a wrong type (long instead of u64)

    Signed-off-by: Eric Dumazet
    ---
    arch/x86/oprofile/op_model_ppro.c | 2 +-
    1 files changed, 1 insertion(+), 1 deletion(-)


    diff --git a/arch/x86/oprofile/op_model_ppro.c b/arch/x86/oprofile/op_model_ppro.c
    index 3f1b81a..716d26f 100644
    --- a/arch/x86/oprofile/op_model_ppro.c
    +++ b/arch/x86/oprofile/op_model_ppro.c
    @@ -69,7 +69,7 @@ static void ppro_setup_ctrs(struct op_msrs const * const msrs)
    int i;

    if (!reset_value) {
    - reset_value = kmalloc(sizeof(unsigned) * num_counters,
    + reset_value = kmalloc(sizeof(reset_value[0]) * num_counters,
    GFP_ATOMIC);
    if (!reset_value)
    return;


  4. Re: [git pull] OProfile fixes for v2.6.28

    Eric Dumazet writes:

    > diff --git a/arch/x86/oprofile/op_model_ppro.c b/arch/x86/oprofile/op_model_ppro.c
    > index 3f1b81a..716d26f 100644
    > --- a/arch/x86/oprofile/op_model_ppro.c
    > +++ b/arch/x86/oprofile/op_model_ppro.c
    > @@ -69,7 +69,7 @@ static void ppro_setup_ctrs(struct op_msrs const * const msrs)
    > int i;
    >
    > if (!reset_value) {
    > - reset_value = kmalloc(sizeof(unsigned) * num_counters,
    > + reset_value = kmalloc(sizeof(reset_value[0]) * num_counters,


    Thanks for tracking this down.

    But that still doesn't explain why 2.6.27 fails too?

    -Andi

    --
    ak@linux.intel.com
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  5. Re: [git pull] OProfile fixes for v2.6.28

    Andi Kleen a écrit :
    > Eric Dumazet writes:
    >
    >> diff --git a/arch/x86/oprofile/op_model_ppro.c b/arch/x86/oprofile/op_model_ppro.c
    >> index 3f1b81a..716d26f 100644
    >> --- a/arch/x86/oprofile/op_model_ppro.c
    >> +++ b/arch/x86/oprofile/op_model_ppro.c
    >> @@ -69,7 +69,7 @@ static void ppro_setup_ctrs(struct op_msrs const * const msrs)
    >> int i;
    >>
    >> if (!reset_value) {
    >> - reset_value = kmalloc(sizeof(unsigned) * num_counters,
    >> + reset_value = kmalloc(sizeof(reset_value[0]) * num_counters,

    >
    > Thanks for tracking this down.
    >
    > But that still doesn't explain why 2.6.27 fails too?


    No

    In fact 2.6.24 fails too. Oprofile never worked correctly.

    Still investigating...

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  6. [PATCH] oprofile: re-arm APIC_DM_NMI in ppro_check_ctrs()

    Andi Kleen a écrit :
    > Eric Dumazet writes:
    >
    >> diff --git a/arch/x86/oprofile/op_model_ppro.c b/arch/x86/oprofile/op_model_ppro.c
    >> index 3f1b81a..716d26f 100644
    >> --- a/arch/x86/oprofile/op_model_ppro.c
    >> +++ b/arch/x86/oprofile/op_model_ppro.c
    >> @@ -69,7 +69,7 @@ static void ppro_setup_ctrs(struct op_msrs const * const msrs)
    >> int i;
    >>
    >> if (!reset_value) {
    >> - reset_value = kmalloc(sizeof(unsigned) * num_counters,
    >> + reset_value = kmalloc(sizeof(reset_value[0]) * num_counters,

    >
    > Thanks for tracking this down.
    >
    > But that still doesn't explain why 2.6.27 fails too?


    Desesperatly Seeking Oprofile, next round.

    I know *nothing* about APIC but spent few hours to try several tricks
    and finally found something.

    It solved my problem : oprofile can run several hours without
    any freeze of NMI on any core.

    # grep NMI /proc/interrupts
    NMI: 10902884 9635871 10333815 8372989 7971483 8298373 8877495 10206963 Non-maskable interrupts
    ...
    # grep NMI /proc/interrupts
    NMI: 15518834 14340713 15038694 13078235 12676585 13003394 13582115 14912146 Non-maskable interrupts


    Can anybody understand and explain what is happening ?

    Is it a software or hardware problem ?

    [PATCH] oprofile: re-arm APIC_DM_NMI in ppro_check_ctrs()

    While using oprofile on my HP BL460c G1, (two quad core intel E5450 CPU),
    I noticed that one CPU after the other could not get anymore NMI.

    After a while, all cores where blocked (ie not generating events for oprofile)
    I tried all major linux versions and all where affected by this freeze.

    I found that we have to re-arm APIC_DM_NMI *before* writing to MSR counter
    when we get event notification.

    Signed-off-by: Eric Dumazet
    arch/x86/oprofile/op_model_ppro.c | 8 +++++---
    1 files changed, 5 insertions(+), 3 deletions(-)

    diff --git a/arch/x86/oprofile/op_model_ppro.c b/arch/x86/oprofile/op_model_ppro.c
    index 3f1b81a..7b142da 100644
    --- a/arch/x86/oprofile/op_model_ppro.c
    +++ b/arch/x86/oprofile/op_model_ppro.c
    @@ -132,13 +132,15 @@ static int ppro_check_ctrs(struct pt_regs * const regs,
    rdmsrl(msrs->counters[i].addr, val);
    if (CTR_OVERFLOWED(val)) {
    oprofile_add_sample(regs, i);
    + /*
    + * We need to unmask the apic vector *before*
    + * writing reset_value to msr counter
    + */
    + apic_write(APIC_LVTPC, APIC_DM_NMI);
    wrmsrl(msrs->counters[i].addr, -reset_value[i]);
    }
    }

    - /* Only P6 based Pentium M need to re-unmask the apic vector but it
    - * doesn't hurt other P6 variant */
    - apic_write(APIC_LVTPC, apic_read(APIC_LVTPC) & ~APIC_LVT_MASKED);

    /* We can't work out if we really handled an interrupt. We
    * might have caught a *second* counter just after overflowing


  7. Re: [PATCH] oprofile: re-arm APIC_DM_NMI in ppro_check_ctrs()

    > oprofile_add_sample(regs, i);
    > + /*
    > + * We need to unmask the apic vector *before*
    > + * writing reset_value to msr counter
    > + */
    > + apic_write(APIC_LVTPC, APIC_DM_NMI);
    > wrmsrl(msrs->counters[i].addr, -reset_value[i]);
    > }
    > }
    >
    > - /* Only P6 based Pentium M need to re-unmask the apic vector but it
    > - * doesn't hurt other P6 variant */
    > - apic_write(APIC_LVTPC, apic_read(APIC_LVTPC) & ~APIC_LVT_MASKED);


    Did you also test if it really needs to be inside the if () or
    just before the wrmsrl?

    -Andi
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  8. Re: [PATCH] oprofile: re-arm APIC_DM_NMI in ppro_check_ctrs()

    Andi Kleen a écrit :
    >> oprofile_add_sample(regs, i);
    >> + /*
    >> + * We need to unmask the apic vector *before*
    >> + * writing reset_value to msr counter
    >> + */
    >> + apic_write(APIC_LVTPC, APIC_DM_NMI);
    >> wrmsrl(msrs->counters[i].addr, -reset_value[i]);
    >> }
    >> }
    >>
    >> - /* Only P6 based Pentium M need to re-unmask the apic vector but it
    >> - * doesn't hurt other P6 variant */
    >> - apic_write(APIC_LVTPC, apic_read(APIC_LVTPC) & ~APIC_LVT_MASKED);

    >
    > Did you also test if it really needs to be inside the if () or
    > just before the wrmsrl?



    In my testings, the "if (CTR_OVERFLOWED())" condition is always true.

    (I am profiling one event only)

    I felt uncomfortable issuing the apic_write() before oprofile_add_sample()

    Do you mean doing the apic_write() right at the beginning of the ppro_check_ctrs()
    function ?

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  9. Re: [PATCH] oprofile: re-arm APIC_DM_NMI in ppro_check_ctrs()

    [Eric Dumazet - Mon, Nov 10, 2008 at 03:23:00PM +0100]
    > Andi Kleen a écrit :
    >> Eric Dumazet writes:
    >>
    >>> diff --git a/arch/x86/oprofile/op_model_ppro.c b/arch/x86/oprofile/op_model_ppro.c
    >>> index 3f1b81a..716d26f 100644
    >>> --- a/arch/x86/oprofile/op_model_ppro.c
    >>> +++ b/arch/x86/oprofile/op_model_ppro.c
    >>> @@ -69,7 +69,7 @@ static void ppro_setup_ctrs(struct op_msrs const * const msrs)
    >>> int i;
    >>> if (!reset_value) {
    >>> - reset_value = kmalloc(sizeof(unsigned) * num_counters,
    >>> + reset_value = kmalloc(sizeof(reset_value[0]) * num_counters,

    >>
    >> Thanks for tracking this down.
    >>
    >> But that still doesn't explain why 2.6.27 fails too?

    >
    > Desesperatly Seeking Oprofile, next round.
    >
    > I know *nothing* about APIC but spent few hours to try several tricks
    > and finally found something.
    >
    > It solved my problem : oprofile can run several hours without
    > any freeze of NMI on any core.
    >
    > # grep NMI /proc/interrupts
    > NMI: 10902884 9635871 10333815 8372989 7971483 8298373 8877495 10206963 Non-maskable interrupts
    > ...
    > # grep NMI /proc/interrupts
    > NMI: 15518834 14340713 15038694 13078235 12676585 13003394 13582115 14912146 Non-maskable interrupts
    >
    >
    > Can anybody understand and explain what is happening ?
    >
    > Is it a software or hardware problem ?
    >
    > [PATCH] oprofile: re-arm APIC_DM_NMI in ppro_check_ctrs()
    >
    > While using oprofile on my HP BL460c G1, (two quad core intel E5450 CPU),
    > I noticed that one CPU after the other could not get anymore NMI.
    >
    > After a while, all cores where blocked (ie not generating events for oprofile)
    > I tried all major linux versions and all where affected by this freeze.
    >
    > I found that we have to re-arm APIC_DM_NMI *before* writing to MSR counter
    > when we get event notification.
    >
    > Signed-off-by: Eric Dumazet
    > arch/x86/oprofile/op_model_ppro.c | 8 +++++---
    > 1 files changed, 5 insertions(+), 3 deletions(-)


    | diff --git a/arch/x86/oprofile/op_model_ppro.c b/arch/x86/oprofile/op_model_ppro.c
    | index 3f1b81a..7b142da 100644
    | --- a/arch/x86/oprofile/op_model_ppro.c
    | +++ b/arch/x86/oprofile/op_model_ppro.c
    | @@ -132,13 +132,15 @@ static int ppro_check_ctrs(struct pt_regs * const regs,
    | rdmsrl(msrs->counters[i].addr, val);
    | if (CTR_OVERFLOWED(val)) {
    | oprofile_add_sample(regs, i);
    | + /*
    | + * We need to unmask the apic vector *before*
    | + * writing reset_value to msr counter
    | + */
    | + apic_write(APIC_LVTPC, APIC_DM_NMI);
    | wrmsrl(msrs->counters[i].addr, -reset_value[i]);
    | }
    | }
    |
    | - /* Only P6 based Pentium M need to re-unmask the apic vector but it
    | - * doesn't hurt other P6 variant */
    | - apic_write(APIC_LVTPC, apic_read(APIC_LVTPC) & ~APIC_LVT_MASKED);
    |
    | /* We can't work out if we really handled an interrupt. We
    | * might have caught a *second* counter just after overflowing

    Hi Eric,

    for the record

    apic_write(APIC_LVTPC, APIC_DM_NMI);

    is not just 'unmask' but also *zeroify* (not sure if I wrote this
    word right all fields when the origianl code was just 'unmasking'
    TPC register

    apic_write(APIC_LVTPC, apic_read(APIC_LVTPC) & ~APIC_LVT_MASKED);

    that is why apic_read() was in former.

    - Cyrill -
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  10. Re: [PATCH] oprofile: re-arm APIC_DM_NMI in ppro_check_ctrs()

    Cyrill Gorcunov a écrit :
    > [Eric Dumazet - Mon, Nov 10, 2008 at 03:23:00PM +0100]
    >> Andi Kleen a écrit :
    >>> Eric Dumazet writes:
    >>>
    >>>> diff --git a/arch/x86/oprofile/op_model_ppro.c b/arch/x86/oprofile/op_model_ppro.c
    >>>> index 3f1b81a..716d26f 100644
    >>>> --- a/arch/x86/oprofile/op_model_ppro.c
    >>>> +++ b/arch/x86/oprofile/op_model_ppro.c
    >>>> @@ -69,7 +69,7 @@ static void ppro_setup_ctrs(struct op_msrs const * const msrs)
    >>>> int i;
    >>>> if (!reset_value) {
    >>>> - reset_value = kmalloc(sizeof(unsigned) * num_counters,
    >>>> + reset_value = kmalloc(sizeof(reset_value[0]) * num_counters,
    >>> Thanks for tracking this down.
    >>>
    >>> But that still doesn't explain why 2.6.27 fails too?

    >> Desesperatly Seeking Oprofile, next round.
    >>
    >> I know *nothing* about APIC but spent few hours to try several tricks
    >> and finally found something.
    >>
    >> It solved my problem : oprofile can run several hours without
    >> any freeze of NMI on any core.
    >>
    >> # grep NMI /proc/interrupts
    >> NMI: 10902884 9635871 10333815 8372989 7971483 8298373 8877495 10206963 Non-maskable interrupts
    >> ...
    >> # grep NMI /proc/interrupts
    >> NMI: 15518834 14340713 15038694 13078235 12676585 13003394 13582115 14912146 Non-maskable interrupts
    >>
    >>
    >> Can anybody understand and explain what is happening ?
    >>
    >> Is it a software or hardware problem ?
    >>
    >> [PATCH] oprofile: re-arm APIC_DM_NMI in ppro_check_ctrs()
    >>
    >> While using oprofile on my HP BL460c G1, (two quad core intel E5450 CPU),
    >> I noticed that one CPU after the other could not get anymore NMI.
    >>
    >> After a while, all cores where blocked (ie not generating events for oprofile)
    >> I tried all major linux versions and all where affected by this freeze.
    >>
    >> I found that we have to re-arm APIC_DM_NMI *before* writing to MSR counter
    >> when we get event notification.
    >>
    >> Signed-off-by: Eric Dumazet
    >> arch/x86/oprofile/op_model_ppro.c | 8 +++++---
    >> 1 files changed, 5 insertions(+), 3 deletions(-)

    >
    > | diff --git a/arch/x86/oprofile/op_model_ppro.c b/arch/x86/oprofile/op_model_ppro.c
    > | index 3f1b81a..7b142da 100644
    > | --- a/arch/x86/oprofile/op_model_ppro.c
    > | +++ b/arch/x86/oprofile/op_model_ppro.c
    > | @@ -132,13 +132,15 @@ static int ppro_check_ctrs(struct pt_regs * const regs,
    > | rdmsrl(msrs->counters[i].addr, val);
    > | if (CTR_OVERFLOWED(val)) {
    > | oprofile_add_sample(regs, i);
    > | + /*
    > | + * We need to unmask the apic vector *before*
    > | + * writing reset_value to msr counter
    > | + */
    > | + apic_write(APIC_LVTPC, APIC_DM_NMI);
    > | wrmsrl(msrs->counters[i].addr, -reset_value[i]);
    > | }
    > | }
    > |
    > | - /* Only P6 based Pentium M need to re-unmask the apic vector but it
    > | - * doesn't hurt other P6 variant */
    > | - apic_write(APIC_LVTPC, apic_read(APIC_LVTPC) & ~APIC_LVT_MASKED);
    > |
    > | /* We can't work out if we really handled an interrupt. We
    > | * might have caught a *second* counter just after overflowing
    >
    > Hi Eric,
    >
    > for the record
    >
    > apic_write(APIC_LVTPC, APIC_DM_NMI);
    >
    > is not just 'unmask' but also *zeroify* (not sure if I wrote this
    > word right all fields when the origianl code was just 'unmasking'
    > TPC register
    >
    > apic_write(APIC_LVTPC, apic_read(APIC_LVTPC) & ~APIC_LVT_MASKED);
    >
    > that is why apic_read() was in former.
    >


    Well, given that APIC_LVTPC is initialized by oprofile init to value APIC_DM_NMI,
    I avoid an apic_read() and just write APIC_DM_NMI again...

    Presumably, apic_read(APIC_LVTPC) should return APIC_DM_NMI or APIC_DM_NMI|APIC_LVT_MASKED

    Thanks

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  11. Re: [PATCH] oprofile: re-arm APIC_DM_NMI in ppro_check_ctrs()

    [Eric Dumazet - Mon, Nov 10, 2008 at 05:19:20PM +0100]
    ....
    >>
    >> Hi Eric,
    >>
    >> for the record
    >>
    >> apic_write(APIC_LVTPC, APIC_DM_NMI);
    >>
    >> is not just 'unmask' but also *zeroify* (not sure if I wrote this
    >> word right all fields when the origianl code was just 'unmasking'
    >> TPC register
    >>
    >> apic_write(APIC_LVTPC, apic_read(APIC_LVTPC) & ~APIC_LVT_MASKED);
    >>
    >> that is why apic_read() was in former.
    >>

    >
    > Well, given that APIC_LVTPC is initialized by oprofile init to value APIC_DM_NMI,
    > I avoid an apic_read() and just write APIC_DM_NMI again...
    >
    > Presumably, apic_read(APIC_LVTPC) should return APIC_DM_NMI or APIC_DM_NMI|APIC_LVT_MASKED
    >
    > Thanks
    >


    Yes, just grepped the sources -- it seems nobody else touching
    this register indeed.
    - Cyrill -
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  12. Re: [PATCH] oprofile: re-arm APIC_DM_NMI in ppro_check_ctrs()

    On Mon, Nov 10, 2008 at 04:50:00PM +0100, Eric Dumazet wrote:
    > Andi Kleen a écrit



    Can you send the /proc/cpuinfo output privately please?
    I can ask around here.

    > >> oprofile_add_sample(regs, i);
    > >>+ /*
    > >>+ * We need to unmask the apic vector *before*
    > >>+ * writing reset_value to msr counter
    > >>+ */
    > >>+ apic_write(APIC_LVTPC, APIC_DM_NMI);
    > >> wrmsrl(msrs->counters[i].addr, -reset_value[i]);
    > >> }
    > >> }
    > >>
    > >>- /* Only P6 based Pentium M need to re-unmask the apic vector but it
    > >>- * doesn't hurt other P6 variant */
    > >>- apic_write(APIC_LVTPC, apic_read(APIC_LVTPC) & ~APIC_LVT_MASKED);

    > >
    > >Did you also test if it really needs to be inside the if () or
    > >just before the wrmsrl?

    >
    >
    > In my testings, the "if (CTR_OVERFLOWED())" condition is always true.
    >
    > (I am profiling one event only)


    There can be other valid (but rare) cases of NMIs. The code handles this
    by not having any overflow events.

    It might be that the case where your NMI gets stuck is such
    a stray NMI?

    That is in theory it should be safe to unmask that vector
    multiple times (the other NMIs should come from other vectors),
    but local APIC is an fragile area.

    >
    > I felt uncomfortable issuing the apic_write() before oprofile_add_sample()
    >
    > Do you mean doing the apic_write() right at the beginning of the
    > ppro_check_ctrs()
    > function ?


    Better once, otherwise you'll do it multiple times worst case
    (multiple counters can overflow)

    -Andi

    --
    ak@linux.intel.com
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  13. Re: [PATCH] oprofile: re-arm APIC_DM_NMI in ppro_check_ctrs()

    Andi Kleen a écrit :
    >> oprofile_add_sample(regs, i);
    >> + /*
    >> + * We need to unmask the apic vector *before*
    >> + * writing reset_value to msr counter
    >> + */
    >> + apic_write(APIC_LVTPC, APIC_DM_NMI);
    >> wrmsrl(msrs->counters[i].addr, -reset_value[i]);
    >> }
    >> }
    >>
    >> - /* Only P6 based Pentium M need to re-unmask the apic vector but it
    >> - * doesn't hurt other P6 variant */
    >> - apic_write(APIC_LVTPC, apic_read(APIC_LVTPC) & ~APIC_LVT_MASKED);

    >
    > Did you also test if it really needs to be inside the if () or
    > just before the wrmsrl?
    >


    Andi, the following patch also works well for me. The key is we must
    unmask APIC before the wrmsrl. after/before the rdmsrl has no impact.

    I wonder then if the final comment in ppro_check_ctrs() is still applicable.


    PATCH] oprofile: un-mask APIC before resetting counter in ppro_check_ctrs()

    While using oprofile on my HP BL460c G1, (two quad core intel E5450 CPU),
    I noticed that one CPU after the other could not get anymore NMI.

    After a while, all cores where blocked (ie not generating events for oprofile)
    I tried all major linux versions and all where affected by this freeze.

    I found that we have to un-mask APIC *before* writing to MSR counter
    when we get event notification, because we use APIC_LVTPC in edge triggered mode.

    Signed-off-by: Eric Dumazet
    ---
    arch/x86/oprofile/op_model_ppro.c | 10 ++++++----
    1 files changed, 6 insertions(+), 4 deletions(-)

    diff --git a/arch/x86/oprofile/op_model_ppro.c b/arch/x86/oprofile/op_model_ppro.c
    index 3f1b81a..8484528 100644
    --- a/arch/x86/oprofile/op_model_ppro.c
    +++ b/arch/x86/oprofile/op_model_ppro.c
    @@ -126,6 +126,12 @@ static int ppro_check_ctrs(struct pt_regs * const regs,
    u64 val;
    int i;

    + /*
    + * We need to unmask the apic vector *before* writing reset_value
    + * to msr counter, because we use edge trigger
    + */
    + apic_write(APIC_LVTPC, apic_read(APIC_LVTPC) & ~APIC_LVT_MASKED);
    +
    for (i = 0 ; i < num_counters; ++i) {
    if (!reset_value[i])
    continue;
    @@ -136,10 +142,6 @@ static int ppro_check_ctrs(struct pt_regs * const regs,
    }
    }

    - /* Only P6 based Pentium M need to re-unmask the apic vector but it
    - * doesn't hurt other P6 variant */
    - apic_write(APIC_LVTPC, apic_read(APIC_LVTPC) & ~APIC_LVT_MASKED);
    -
    /* We can't work out if we really handled an interrupt. We
    * might have caught a *second* counter just after overflowing
    * the interrupt for this counter then arrives


+ Reply to Thread