| Message ID | 20210207105934.2001-1-thunder.leizhen@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/1] ARM: hw_breakpoint: Do not directly check the event's overflow_handler hook | expand |
On 2021/2/7 18:59, Zhen Lei wrote: > The commit 1879445dfa7b ("perf/core: Set event's default > ::overflow_handler()") set a default event->overflow_handler in > perf_event_alloc(), and replace the check event->overflow_handler with > is_default_overflow_handler(), but one is missing. > > Currently, the bp->overflow_handler can not be NULL. As a result, > enable_single_step() is always not invoked. Our test case use "perf record -e mem:$TEST_LOC_ADDR:x -p $TEST_PID sleep 10" to track the invocation of a test function, the entry address of the function is $TEST_LOC_ADDR. This function was called only once. Therefore, the sampling result of perf should be oneļ¼ as follow: [ perf record: Captured and wrote 0.001 MB perf.data (1 samples) ] But it's actually infinite. [ perf record: Captured and wrote 135.426 MB perf.data (2068004 samples) ] If sleep 10 is not specified, OOM will be reported. > > Fixes: 1879445dfa7b ("perf/core: Set event's default ::overflow_handler()") > Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> > Cc: Wang Nan <wangnan0@huawei.com> > --- > arch/arm/kernel/hw_breakpoint.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm/kernel/hw_breakpoint.c b/arch/arm/kernel/hw_breakpoint.c > index 08660ae9dcbce1c..b1423fb130ea4a0 100644 > --- a/arch/arm/kernel/hw_breakpoint.c > +++ b/arch/arm/kernel/hw_breakpoint.c > @@ -886,7 +886,7 @@ static void breakpoint_handler(unsigned long unknown, struct pt_regs *regs) > info->trigger = addr; > pr_debug("breakpoint fired: address = 0x%x\n", addr); > perf_bp_event(bp, regs); > - if (!bp->overflow_handler) > + if (is_default_overflow_handler(bp)) > enable_single_step(bp, addr); > goto unlock; > } >
On Sun, Feb 07, 2021 at 06:59:34PM +0800, Zhen Lei wrote: > The commit 1879445dfa7b ("perf/core: Set event's default > ::overflow_handler()") set a default event->overflow_handler in > perf_event_alloc(), and replace the check event->overflow_handler with > is_default_overflow_handler(), but one is missing. > > Currently, the bp->overflow_handler can not be NULL. As a result, > enable_single_step() is always not invoked. > > Fixes: 1879445dfa7b ("perf/core: Set event's default ::overflow_handler()") > Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> > Cc: Wang Nan <wangnan0@huawei.com> > --- > arch/arm/kernel/hw_breakpoint.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm/kernel/hw_breakpoint.c b/arch/arm/kernel/hw_breakpoint.c > index 08660ae9dcbce1c..b1423fb130ea4a0 100644 > --- a/arch/arm/kernel/hw_breakpoint.c > +++ b/arch/arm/kernel/hw_breakpoint.c > @@ -886,7 +886,7 @@ static void breakpoint_handler(unsigned long unknown, struct pt_regs *regs) > info->trigger = addr; > pr_debug("breakpoint fired: address = 0x%x\n", addr); > perf_bp_event(bp, regs); > - if (!bp->overflow_handler) > + if (is_default_overflow_handler(bp)) > enable_single_step(bp, addr); > goto unlock; > } Acked-by: Will Deacon <will@kernel.org> Please can you put this into the patch tracker? https://www.arm.linux.org.uk/developer/patches/ Will
On 2021/2/12 18:02, Will Deacon wrote: > On Sun, Feb 07, 2021 at 06:59:34PM +0800, Zhen Lei wrote: >> The commit 1879445dfa7b ("perf/core: Set event's default >> ::overflow_handler()") set a default event->overflow_handler in >> perf_event_alloc(), and replace the check event->overflow_handler with >> is_default_overflow_handler(), but one is missing. >> >> Currently, the bp->overflow_handler can not be NULL. As a result, >> enable_single_step() is always not invoked. >> >> Fixes: 1879445dfa7b ("perf/core: Set event's default ::overflow_handler()") >> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> >> Cc: Wang Nan <wangnan0@huawei.com> >> --- >> arch/arm/kernel/hw_breakpoint.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/arch/arm/kernel/hw_breakpoint.c b/arch/arm/kernel/hw_breakpoint.c >> index 08660ae9dcbce1c..b1423fb130ea4a0 100644 >> --- a/arch/arm/kernel/hw_breakpoint.c >> +++ b/arch/arm/kernel/hw_breakpoint.c >> @@ -886,7 +886,7 @@ static void breakpoint_handler(unsigned long unknown, struct pt_regs *regs) >> info->trigger = addr; >> pr_debug("breakpoint fired: address = 0x%x\n", addr); >> perf_bp_event(bp, regs); >> - if (!bp->overflow_handler) >> + if (is_default_overflow_handler(bp)) >> enable_single_step(bp, addr); >> goto unlock; >> } > > Acked-by: Will Deacon <will@kernel.org> > > Please can you put this into the patch tracker? > > https://www.arm.linux.org.uk/developer/patches/ I've added it. https://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=9064/1 > > Will > > . >
On Thu, Feb 18, 2021 at 10:03:22AM +0800, Leizhen (ThunderTown) wrote: > > > On 2021/2/12 18:02, Will Deacon wrote: > > On Sun, Feb 07, 2021 at 06:59:34PM +0800, Zhen Lei wrote: > >> The commit 1879445dfa7b ("perf/core: Set event's default > >> ::overflow_handler()") set a default event->overflow_handler in > >> perf_event_alloc(), and replace the check event->overflow_handler with > >> is_default_overflow_handler(), but one is missing. > >> > >> Currently, the bp->overflow_handler can not be NULL. As a result, > >> enable_single_step() is always not invoked. > >> > >> Fixes: 1879445dfa7b ("perf/core: Set event's default ::overflow_handler()") > >> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> > >> Cc: Wang Nan <wangnan0@huawei.com> > >> --- > >> arch/arm/kernel/hw_breakpoint.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/arch/arm/kernel/hw_breakpoint.c b/arch/arm/kernel/hw_breakpoint.c > >> index 08660ae9dcbce1c..b1423fb130ea4a0 100644 > >> --- a/arch/arm/kernel/hw_breakpoint.c > >> +++ b/arch/arm/kernel/hw_breakpoint.c > >> @@ -886,7 +886,7 @@ static void breakpoint_handler(unsigned long unknown, struct pt_regs *regs) > >> info->trigger = addr; > >> pr_debug("breakpoint fired: address = 0x%x\n", addr); > >> perf_bp_event(bp, regs); > >> - if (!bp->overflow_handler) > >> + if (is_default_overflow_handler(bp)) > >> enable_single_step(bp, addr); > >> goto unlock; > >> } > > > > Acked-by: Will Deacon <will@kernel.org> > > > > Please can you put this into the patch tracker? > > > > https://www.arm.linux.org.uk/developer/patches/ > > I've added it. > > https://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=9064/1 Thanks, although you forgot to add my Ack! Will
On 2021/2/18 17:13, Will Deacon wrote: > On Thu, Feb 18, 2021 at 10:03:22AM +0800, Leizhen (ThunderTown) wrote: >> >> >> On 2021/2/12 18:02, Will Deacon wrote: >>> On Sun, Feb 07, 2021 at 06:59:34PM +0800, Zhen Lei wrote: >>>> The commit 1879445dfa7b ("perf/core: Set event's default >>>> ::overflow_handler()") set a default event->overflow_handler in >>>> perf_event_alloc(), and replace the check event->overflow_handler with >>>> is_default_overflow_handler(), but one is missing. >>>> >>>> Currently, the bp->overflow_handler can not be NULL. As a result, >>>> enable_single_step() is always not invoked. >>>> >>>> Fixes: 1879445dfa7b ("perf/core: Set event's default ::overflow_handler()") >>>> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> >>>> Cc: Wang Nan <wangnan0@huawei.com> >>>> --- >>>> arch/arm/kernel/hw_breakpoint.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/arch/arm/kernel/hw_breakpoint.c b/arch/arm/kernel/hw_breakpoint.c >>>> index 08660ae9dcbce1c..b1423fb130ea4a0 100644 >>>> --- a/arch/arm/kernel/hw_breakpoint.c >>>> +++ b/arch/arm/kernel/hw_breakpoint.c >>>> @@ -886,7 +886,7 @@ static void breakpoint_handler(unsigned long unknown, struct pt_regs *regs) >>>> info->trigger = addr; >>>> pr_debug("breakpoint fired: address = 0x%x\n", addr); >>>> perf_bp_event(bp, regs); >>>> - if (!bp->overflow_handler) >>>> + if (is_default_overflow_handler(bp)) >>>> enable_single_step(bp, addr); >>>> goto unlock; >>>> } >>> >>> Acked-by: Will Deacon <will@kernel.org> >>> >>> Please can you put this into the patch tracker? >>> >>> https://www.arm.linux.org.uk/developer/patches/ >> >> I've added it. >> >> https://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=9064/1 > > Thanks, although you forgot to add my Ack! Sorry, the first time I operated this web, the patch in the attachment can no longer be updated. I just mentioned it in the new "Node". > > Will > > . >
diff --git a/arch/arm/kernel/hw_breakpoint.c b/arch/arm/kernel/hw_breakpoint.c index 08660ae9dcbce1c..b1423fb130ea4a0 100644 --- a/arch/arm/kernel/hw_breakpoint.c +++ b/arch/arm/kernel/hw_breakpoint.c @@ -886,7 +886,7 @@ static void breakpoint_handler(unsigned long unknown, struct pt_regs *regs) info->trigger = addr; pr_debug("breakpoint fired: address = 0x%x\n", addr); perf_bp_event(bp, regs); - if (!bp->overflow_handler) + if (is_default_overflow_handler(bp)) enable_single_step(bp, addr); goto unlock; }
The commit 1879445dfa7b ("perf/core: Set event's default ::overflow_handler()") set a default event->overflow_handler in perf_event_alloc(), and replace the check event->overflow_handler with is_default_overflow_handler(), but one is missing. Currently, the bp->overflow_handler can not be NULL. As a result, enable_single_step() is always not invoked. Fixes: 1879445dfa7b ("perf/core: Set event's default ::overflow_handler()") Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> Cc: Wang Nan <wangnan0@huawei.com> --- arch/arm/kernel/hw_breakpoint.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)