linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Will Deacon <will.deacon@arm.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Wang Nan <wangnan0@huawei.com>,
	mingo@redhat.com, linux-kernel@vger.kernel.org,
	He Kuang <hekuang@huawei.com>,
	Alexei Starovoitov <ast@kernel.org>,
	Arnaldo Carvalho de Melo <acme@redhat.com>,
	Brendan Gregg <brendan.d.gregg@gmail.com>,
	Jiri Olsa <jolsa@kernel.org>,
	Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>,
	Namhyung Kim <namhyung@kernel.org>, Zefan Li <lizefan@huawei.com>,
	pi3orama@163.com
Subject: Re: [PATCH 2/5] perf core: Set event's default overflow_handler
Date: Wed, 23 Mar 2016 18:13:49 +0000	[thread overview]
Message-ID: <20160323181348.GA2149@arm.com> (raw)
In-Reply-To: <20160323175021.GK6356@twins.programming.kicks-ass.net>

On Wed, Mar 23, 2016 at 06:50:21PM +0100, Peter Zijlstra wrote:
> On Mon, Mar 14, 2016 at 09:59:42AM +0000, Wang Nan wrote:
> > +++ b/arch/arm/kernel/hw_breakpoint.c
> > @@ -631,7 +631,7 @@ int arch_validate_hwbkpt_settings(struct perf_event *bp)
> >  	info->address &= ~alignment_mask;
> >  	info->ctrl.len <<= offset;
> >  
> > -	if (!bp->overflow_handler) {
> > +	if (is_default_overflow_handler(bp)) {
> >  		/*
> >  		 * Mismatch breakpoints are required for single-stepping
> >  		 * breakpoints.
> > @@ -754,7 +754,7 @@ static void watchpoint_handler(unsigned long addr, unsigned int fsr,
> >  		 * mismatch breakpoint so we can single-step over the
> >  		 * watchpoint trigger.
> >  		 */
> > -		if (!wp->overflow_handler)
> > +		if (is_default_overflow_handler(wp))
> >  			enable_single_step(wp, instruction_pointer(regs));
> >  
> >  unlock:
> > diff --git a/arch/arm64/kernel/hw_breakpoint.c b/arch/arm64/kernel/hw_breakpoint.c
> > index b45c95d..4ef5373 100644
> > --- a/arch/arm64/kernel/hw_breakpoint.c
> > +++ b/arch/arm64/kernel/hw_breakpoint.c
> > @@ -616,7 +616,7 @@ static int breakpoint_handler(unsigned long unused, unsigned int esr,
> >  		perf_bp_event(bp, regs);
> >  
> >  		/* Do we need to handle the stepping? */
> > -		if (!bp->overflow_handler)
> > +		if (is_default_overflow_handler(bp))
> >  			step = 1;
> >  unlock:
> >  		rcu_read_unlock();
> > @@ -712,7 +712,7 @@ static int watchpoint_handler(unsigned long addr, unsigned int esr,
> >  		perf_bp_event(wp, regs);
> >  
> >  		/* Do we need to handle the stepping? */
> > -		if (!wp->overflow_handler)
> > +		if (is_default_overflow_handler(wp))
> >  			step = 1;
> >  
> >  unlock:
> 
> Will, why does it matter what the overflow handler is for this stuff?

Because ptrace registers an overflow handler for raising a SIGTRAP and
ptrace users (e.g. GDB) expect to handle the single-stepping themselves.
Perf, on the other hand, will livelock if the kernel doesn't do the
stepping.

FWIW, I hate this whole thing. The only users of the perf side just seem
to be people running whacky test cases and then pointing out the places
where we're not identical to x86 :(

Will

  reply	other threads:[~2016-03-23 18:13 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-14  9:59 [PATCH 0/5] perf core: Support overwrite ring buffer Wang Nan
2016-03-14  9:59 ` [PATCH 1/5] perf core: Introduce new ioctl options to pause and resume " Wang Nan
2016-03-23  9:16   ` Peter Zijlstra
2016-03-23  9:33     ` Wangnan (F)
2016-03-23  9:52       ` Peter Zijlstra
2016-03-23 12:43       ` Vince Weaver
2016-10-21  7:06       ` Michael Kerrisk (man-pages)
2016-10-21  7:13         ` Wangnan (F)
2016-10-21  8:55           ` Michael Kerrisk (man-pages)
2016-03-14  9:59 ` [PATCH 2/5] perf core: Set event's default overflow_handler Wang Nan
2016-03-23 17:50   ` Peter Zijlstra
2016-03-23 18:13     ` Will Deacon [this message]
2016-03-23 19:13       ` Peter Zijlstra
2016-03-23 19:29       ` Peter Zijlstra
2016-03-24  9:58         ` Will Deacon
2016-03-24 10:42           ` Peter Zijlstra
2016-03-14  9:59 ` [PATCH 3/5] perf core: Prepare writing into ring buffer from end Wang Nan
2016-03-23  9:50   ` Peter Zijlstra
2016-03-23 10:08     ` Wangnan (F)
2016-03-23 19:25       ` Alexei Starovoitov
2016-03-24  3:48         ` Wangnan (F)
2016-03-24 17:29           ` Alexei Starovoitov
2016-03-25 12:26     ` Wangnan (F)
2016-03-25 12:36       ` Wangnan (F)
2016-03-25 14:14         ` Wangnan (F)
2016-03-27 15:20           ` Peter Zijlstra
2016-03-27 15:30             ` pi3orama
2016-03-28  1:07               ` Wangnan (F)
2016-03-28  1:58                 ` Wangnan (F)
2016-03-28  2:58                   ` Wangnan (F)
2016-03-14  9:59 ` [PATCH 4/5] perf core: Add backward attribute to perf event Wang Nan
2016-03-14  9:59 ` [PATCH 5/5] perf core: Reduce perf event output overhead by new overflow handler Wang Nan
2016-03-23 10:02   ` Peter Zijlstra
  -- strict thread matches above, loose matches on Subject: below --
2016-01-25  8:33 [PATCH 0/5] perf core: Read from overwrite ring buffer Wang Nan
2016-01-25  8:33 ` [PATCH 2/5] perf core: Set event's default overflow_handler Wang Nan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160323181348.GA2149@arm.com \
    --to=will.deacon@arm.com \
    --cc=acme@redhat.com \
    --cc=ast@kernel.org \
    --cc=brendan.d.gregg@gmail.com \
    --cc=hekuang@huawei.com \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lizefan@huawei.com \
    --cc=masami.hiramatsu.pt@hitachi.com \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    --cc=pi3orama@163.com \
    --cc=wangnan0@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).