From: Jiri Olsa <jolsa@redhat.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>,
Michael Neuling <mikey@neuling.org>,
Paul Mackerras <paulus@samba.org>, Jiri Olsa <jolsa@kernel.org>,
lkml <linux-kernel@vger.kernel.org>,
Ingo Molnar <mingo@kernel.org>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Jan Stancek <jstancek@redhat.com>
Subject: Re: [PATCHv2] perf powerpc: Don't call perf_event_disable from atomic context
Date: Mon, 24 Oct 2016 17:49:49 +0200 [thread overview]
Message-ID: <20161024154949.GA31643@krava> (raw)
In-Reply-To: <20161024122639.GN3102@twins.programming.kicks-ass.net>
On Mon, Oct 24, 2016 at 02:26:39PM +0200, Peter Zijlstra wrote:
>
>
>
> On Thu, Oct 06, 2016 at 02:33:01PM +0200, Jiri Olsa wrote:
> > diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c
> > index aec9a1b1d25b..4d3bcbbf626a 100644
> > --- a/arch/powerpc/kernel/hw_breakpoint.c
> > +++ b/arch/powerpc/kernel/hw_breakpoint.c
> > @@ -275,7 +275,7 @@ int __kprobes hw_breakpoint_handler(struct die_args *args)
> > if (!stepped) {
> > WARN(1, "Unable to handle hardware breakpoint. Breakpoint at "
> > "0x%lx will be disabled.", info->address);
> > - perf_event_disable(bp);
> > + perf_event_disable_inatomic(bp);
> > goto out;
> > }
> > /*
> > diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> > index 5c5362584aba..c794fd84a595 100644
> > --- a/include/linux/perf_event.h
> > +++ b/include/linux/perf_event.h
> > @@ -1248,6 +1248,7 @@ extern u64 perf_swevent_set_period(struct perf_event *event);
> > extern void perf_event_enable(struct perf_event *event);
> > extern void perf_event_disable(struct perf_event *event);
> > extern void perf_event_disable_local(struct perf_event *event);
> > +extern void perf_event_disable_inatomic(struct perf_event *event);
> > extern void perf_event_task_tick(void);
> > #else /* !CONFIG_PERF_EVENTS: */
> > static inline void *
> > diff --git a/kernel/events/core.c b/kernel/events/core.c
> > index 7c0d263f6bc5..3d650ccf4def 100644
> > --- a/kernel/events/core.c
> > +++ b/kernel/events/core.c
> > @@ -1960,6 +1960,13 @@ void perf_event_disable(struct perf_event *event)
> > }
> > EXPORT_SYMBOL_GPL(perf_event_disable);
> >
> > +void perf_event_disable_inatomic(struct perf_event *event)
> > +{
> > + event->pending_kill = POLL_HUP;
> > + event->pending_disable = 1;
> > + irq_work_queue(&event->pending);
> > +}
> > +
> > static void perf_set_shadow_time(struct perf_event *event,
> > struct perf_event_context *ctx,
> > u64 tstamp)
> > @@ -7074,9 +7081,7 @@ static int __perf_event_overflow(struct perf_event *event,
> > event->pending_kill = POLL_IN;
> > if (events && atomic_dec_and_test(&event->event_limit)) {
> > ret = 1;
> > - event->pending_kill = POLL_HUP;
> > - event->pending_disable = 1;
> > - irq_work_queue(&event->pending);
> > + perf_event_disable_inatomic(event);
> > }
> >
> > event->overflow_handler(event, data, regs);
>
>
> I'm a wee bit confused on the event->pending_kill assignment in that
> helper. AFAICT that's not actually used in the PPC case, and if for some
> reason, pending_wakeup gets set, do we want to deliver a HUP? The old
> code didn't do that.
right, delivering the signal might change what app is expecting,
how about attached change.. I still need to test it
thanks,
jirka
---
diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c
index 9781c69eae57..58024eecbd9e 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -275,7 +275,7 @@ int hw_breakpoint_handler(struct die_args *args)
if (!stepped) {
WARN(1, "Unable to handle hardware breakpoint. Breakpoint at "
"0x%lx will be disabled.", info->address);
- perf_event_disable(bp);
+ perf_event_disable_inatomic(bp, 0);
goto out;
}
/*
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 060d0ede88df..055bc837bfc1 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -1257,6 +1257,7 @@ extern u64 perf_swevent_set_period(struct perf_event *event);
extern void perf_event_enable(struct perf_event *event);
extern void perf_event_disable(struct perf_event *event);
extern void perf_event_disable_local(struct perf_event *event);
+extern void perf_event_disable_inatomic(struct perf_event *event, int kill);
extern void perf_event_task_tick(void);
#else /* !CONFIG_PERF_EVENTS: */
static inline void *
diff --git a/kernel/events/core.c b/kernel/events/core.c
index c6e47e97b33f..04477983945e 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -1960,6 +1960,13 @@ void perf_event_disable(struct perf_event *event)
}
EXPORT_SYMBOL_GPL(perf_event_disable);
+void perf_event_disable_inatomic(struct perf_event *event, int kill)
+{
+ event->pending_kill = kill;
+ event->pending_disable = 1;
+ irq_work_queue(&event->pending);
+}
+
static void perf_set_shadow_time(struct perf_event *event,
struct perf_event_context *ctx,
u64 tstamp)
@@ -7074,9 +7081,7 @@ static int __perf_event_overflow(struct perf_event *event,
event->pending_kill = POLL_IN;
if (events && atomic_dec_and_test(&event->event_limit)) {
ret = 1;
- event->pending_kill = POLL_HUP;
- event->pending_disable = 1;
- irq_work_queue(&event->pending);
+ perf_event_disable_inatomic(event, POLL_HUP);
}
READ_ONCE(event->overflow_handler)(event, data, regs);
next prev parent reply other threads:[~2016-10-24 15:49 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-21 13:55 [PATCH] perf powerpc: Don't call perf_event_disable from atomic context Jiri Olsa
2016-09-23 16:37 ` Peter Zijlstra
2016-10-03 13:29 ` Jiri Olsa
2016-10-03 13:47 ` Peter Zijlstra
2016-10-04 4:29 ` Michael Ellerman
2016-10-04 7:06 ` Peter Zijlstra
2016-10-10 13:19 ` Will Deacon
2016-10-05 8:09 ` Jiri Olsa
2016-10-05 19:53 ` Jiri Olsa
2016-10-06 7:24 ` Peter Zijlstra
2016-10-06 12:33 ` [PATCHv2] " Jiri Olsa
2016-10-24 12:26 ` Peter Zijlstra
2016-10-24 15:49 ` Jiri Olsa [this message]
2016-10-25 6:40 ` [perf powerpc] 18d1796d0b: [No primary change] kernel test robot
2016-10-25 6:40 ` [lkp] " kernel test robot
2016-10-25 9:06 ` Peter Zijlstra
2016-10-25 9:06 ` [lkp] " Peter Zijlstra
2016-10-26 2:09 ` Huang, Ying
2016-10-26 2:09 ` [LKP] [lkp] " Huang, Ying
2016-10-26 9:48 ` [PATCHv3] perf powerpc: Don't call perf_event_disable from atomic context Jiri Olsa
2016-10-26 9:48 ` Jiri Olsa
2016-10-26 15:12 ` Peter Zijlstra
2016-10-26 15:12 ` Peter Zijlstra
2016-10-26 15:24 ` Jiri Olsa
2016-10-26 15:24 ` Jiri Olsa
2016-10-28 10:10 ` [tip:perf/urgent] perf/powerpc: Don't call perf_event_disable() " tip-bot for Jiri Olsa
2016-10-04 4:08 ` [PATCH] perf powerpc: Don't call perf_event_disable " Michael Ellerman
2016-10-05 8:08 ` Jiri Olsa
2016-10-05 8:21 ` Jan Stancek
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=20161024154949.GA31643@krava \
--to=jolsa@redhat.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=jolsa@kernel.org \
--cc=jstancek@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mikey@neuling.org \
--cc=mingo@kernel.org \
--cc=mpe@ellerman.id.au \
--cc=paulus@samba.org \
--cc=peterz@infradead.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.