Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: Linux: Kernel

[PATCH -tip v3 0/3] tracepoint: Add signal events

 

 

Linux kernel RSS feed   Index | Next | Previous | View Threaded


mhiramat at redhat

Nov 20, 2009, 1:31 PM

Post #1 of 7 (106 views)
Permalink
[PATCH -tip v3 0/3] tracepoint: Add signal events

Hi,

These patches add signal related tracepoints including
signal generation, delivery, and loss. First patch also
moves signal-sending tracepoint from events/sched.h to
events/signal.h.

Changes in v3
- Add Docbook style comments

Changes in v2
- Add siginfo arguments

Thank you,

---

Masami Hiramatsu (3):
tracepoint: Add signal loss events
tracepoint: Add signal deliver event
tracepoint: Move signal sending tracepoint to events/signal.h


Documentation/DocBook/tracepoint.tmpl | 5 +
include/trace/events/sched.h | 25 -----
include/trace/events/signal.h | 173 +++++++++++++++++++++++++++++++++
kernel/signal.c | 27 ++++-
4 files changed, 198 insertions(+), 32 deletions(-)
create mode 100644 include/trace/events/signal.h

--
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division
e-mail: mhiramat [at] redhat
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


mingo at elte

Nov 23, 2009, 9:57 AM

Post #2 of 7 (90 views)
Permalink
Re: [PATCH -tip v3 0/3] tracepoint: Add signal events [In reply to]

* Masami Hiramatsu <mhiramat [at] redhat> wrote:

> Hi,
>
> These patches add signal related tracepoints including
> signal generation, delivery, and loss. First patch also
> moves signal-sending tracepoint from events/sched.h to
> events/signal.h.
>
> Changes in v3
> - Add Docbook style comments
>
> Changes in v2
> - Add siginfo arguments
>
> Thank you,
>
> ---
>
> Masami Hiramatsu (3):
> tracepoint: Add signal loss events
> tracepoint: Add signal deliver event
> tracepoint: Move signal sending tracepoint to events/signal.h
>
>
> Documentation/DocBook/tracepoint.tmpl | 5 +
> include/trace/events/sched.h | 25 -----
> include/trace/events/signal.h | 173 +++++++++++++++++++++++++++++++++
> kernel/signal.c | 27 ++++-
> 4 files changed, 198 insertions(+), 32 deletions(-)
> create mode 100644 include/trace/events/signal.h

Would be nice to have Roland's and Oleg's Acked-by tags in the patches -
to show that this is a representative and useful looking set of signal
events.

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


jbaron at redhat

Nov 23, 2009, 3:00 PM

Post #3 of 7 (89 views)
Permalink
Re: [PATCH -tip v3 0/3] tracepoint: Add signal events [In reply to]

On Fri, Nov 20, 2009 at 04:31:08PM -0500, Masami Hiramatsu wrote:
> Hi,
>
> These patches add signal related tracepoints including
> signal generation, delivery, and loss. First patch also
> moves signal-sending tracepoint from events/sched.h to
> events/signal.h.
>
> Changes in v3
> - Add Docbook style comments
>

Documentation bits look good. thanks for adding them.

Reviewed-by: Jason Baron <jbaron [at] redhat>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/


oleg at redhat

Nov 24, 2009, 1:22 PM

Post #4 of 7 (79 views)
Permalink
Re: [PATCH -tip v3 0/3] tracepoint: Add signal events [In reply to]

On 11/23, Ingo Molnar wrote:
>
> * Masami Hiramatsu <mhiramat [at] redhat> wrote:
>
> > Hi,
> >
> > These patches add signal related tracepoints including
> > signal generation, delivery, and loss. First patch also
> > moves signal-sending tracepoint from events/sched.h to
> > events/signal.h.
> >
> > Changes in v3
> > - Add Docbook style comments
> >
> > Changes in v2
> > - Add siginfo arguments
> >
> > Thank you,
> >
> > ---
> >
> > Masami Hiramatsu (3):
> > tracepoint: Add signal loss events
> > tracepoint: Add signal deliver event
> > tracepoint: Move signal sending tracepoint to events/signal.h
> >
> >
> > Documentation/DocBook/tracepoint.tmpl | 5 +
> > include/trace/events/sched.h | 25 -----
> > include/trace/events/signal.h | 173 +++++++++++++++++++++++++++++++++
> > kernel/signal.c | 27 ++++-
> > 4 files changed, 198 insertions(+), 32 deletions(-)
> > create mode 100644 include/trace/events/signal.h
>
> Would be nice to have Roland's and Oleg's Acked-by tags in the patches -
> to show that this is a representative and useful looking set of signal
> events.

Sorry, I can't really comment these patches.

I mean, I do not know which info is useful and which is not.
For example, I am a bit surprized we report trace_signal_lose_info()
but please do not consider this as if I think we shouldn't. Just I
do not know.

OTOH, we do not report if __send_signal() fails just because the
legacy signal is already queued. We do not report who sends the signal,
we do not report if it was private or shared. zap_process, complete_signal
can "send" SIGKILL via sigaddset, this won't be noticed. But again, it is
not that I think this should be reported.

In short: I think any info may be useful, and these patches can help.
But I do not understand what exactly should be reported to userspace.

Oleg.

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


mingo at elte

Nov 24, 2009, 1:37 PM

Post #5 of 7 (78 views)
Permalink
Re: [PATCH -tip v3 0/3] tracepoint: Add signal events [In reply to]

* Oleg Nesterov <oleg [at] redhat> wrote:

> On 11/23, Ingo Molnar wrote:
> >
> > * Masami Hiramatsu <mhiramat [at] redhat> wrote:
> >
> > > Hi,
> > >
> > > These patches add signal related tracepoints including
> > > signal generation, delivery, and loss. First patch also
> > > moves signal-sending tracepoint from events/sched.h to
> > > events/signal.h.
> > >
> > > Changes in v3
> > > - Add Docbook style comments
> > >
> > > Changes in v2
> > > - Add siginfo arguments
> > >
> > > Thank you,
> > >
> > > ---
> > >
> > > Masami Hiramatsu (3):
> > > tracepoint: Add signal loss events
> > > tracepoint: Add signal deliver event
> > > tracepoint: Move signal sending tracepoint to events/signal.h
> > >
> > >
> > > Documentation/DocBook/tracepoint.tmpl | 5 +
> > > include/trace/events/sched.h | 25 -----
> > > include/trace/events/signal.h | 173 +++++++++++++++++++++++++++++++++
> > > kernel/signal.c | 27 ++++-
> > > 4 files changed, 198 insertions(+), 32 deletions(-)
> > > create mode 100644 include/trace/events/signal.h
> >
> > Would be nice to have Roland's and Oleg's Acked-by tags in the patches -
> > to show that this is a representative and useful looking set of signal
> > events.
>
> Sorry, I can't really comment these patches.
>
> I mean, I do not know which info is useful and which is not. For
> example, I am a bit surprized we report trace_signal_lose_info() but
> please do not consider this as if I think we shouldn't. Just I do not
> know.

well, there we lose information, so it's basically an exception/anomaly
that a person doing analysis might be interested in.

> OTOH, we do not report if __send_signal() fails just because the
> legacy signal is already queued. [...]

We could do that (beyond the queued signals full event), but i think
it's rather common to see signal overlap in the legacy case, right?

> [...] We do not report who sends the signal, [...]

The PID of any task generating an event can be sampled, so that's
implicit.

> [...] we do not report if it was private or shared. zap_process,
> complete_signal can "send" SIGKILL via sigaddset, this won't be
> noticed. But again, it is not that I think this should be reported.
>
> In short: I think any info may be useful, and these patches can help.
> But I do not understand what exactly should be reported to userspace.

The principe is this: there's two extremes:

A- report no event

B- report every event precisely, that allows all signal state and
actions to be reconstructed in hindsight.

And there's a continuum between the two extremes. Just a random state
between A) and B) makes little sense - but certain subsets (say an
'overview' of major signal events) might make sense from an analysis
POV.

But the thing is, by my reading of these patches we are pretty close to
B) right now and the tracepoints still look sane - so we might as well
implement your suggestions and achieve B)? That's a well-defined target
to achieve. It would mean we need events of sigmask manipulations as
well, and handler setting events. Plus the missing events you pointed
out. (plus other stuff i might have forgotten about)

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


mhiramat at redhat

Nov 24, 2009, 1:45 PM

Post #6 of 7 (78 views)
Permalink
Re: [PATCH -tip v3 0/3] tracepoint: Add signal events [In reply to]

Oleg Nesterov wrote:
> On 11/23, Ingo Molnar wrote:
>>
>> * Masami Hiramatsu<mhiramat [at] redhat> wrote:
>>
>>> Hi,
>>>
>>> These patches add signal related tracepoints including
>>> signal generation, delivery, and loss. First patch also
>>> moves signal-sending tracepoint from events/sched.h to
>>> events/signal.h.
>>>
>>> Changes in v3
>>> - Add Docbook style comments
>>>
>>> Changes in v2
>>> - Add siginfo arguments
>>>
>>> Thank you,
>>>
>>> ---
>>>
>>> Masami Hiramatsu (3):
>>> tracepoint: Add signal loss events
>>> tracepoint: Add signal deliver event
>>> tracepoint: Move signal sending tracepoint to events/signal.h
>>>
>>>
>>> Documentation/DocBook/tracepoint.tmpl | 5 +
>>> include/trace/events/sched.h | 25 -----
>>> include/trace/events/signal.h | 173 +++++++++++++++++++++++++++++++++
>>> kernel/signal.c | 27 ++++-
>>> 4 files changed, 198 insertions(+), 32 deletions(-)
>>> create mode 100644 include/trace/events/signal.h
>>
>> Would be nice to have Roland's and Oleg's Acked-by tags in the patches -
>> to show that this is a representative and useful looking set of signal
>> events.
>
> Sorry, I can't really comment these patches.
>
> I mean, I do not know which info is useful and which is not.
> For example, I am a bit surprized we report trace_signal_lose_info()
> but please do not consider this as if I think we shouldn't. Just I
> do not know.
>
> OTOH, we do not report if __send_signal() fails just because the
> legacy signal is already queued. We do not report who sends the signal,
> we do not report if it was private or shared. zap_process, complete_signal
> can "send" SIGKILL via sigaddset, this won't be noticed. But again, it is
> not that I think this should be reported.
>
> In short: I think any info may be useful, and these patches can help.
> But I do not understand what exactly should be reported to userspace.

Yeah, any comments are welcome:-) IMHO, these tracepoints are just for
providing options for users who care about who sent the signal, etc.

Thank you,

--
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: mhiramat [at] redhat

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


oleg at redhat

Nov 25, 2009, 9:41 AM

Post #7 of 7 (75 views)
Permalink
Re: [PATCH -tip v3 0/3] tracepoint: Add signal events [In reply to]

On 11/24, Ingo Molnar wrote:
>
> * Oleg Nesterov <oleg [at] redhat> wrote:
>
> > Sorry, I can't really comment these patches.
> >
> > I mean, I do not know which info is useful and which is not. For
> > example, I am a bit surprized we report trace_signal_lose_info() but
> > please do not consider this as if I think we shouldn't. Just I do not
> > know.
>
> well, there we lose information, so it's basically an exception/anomaly
> that a person doing analysis might be interested in.
>
> > OTOH, we do not report if __send_signal() fails just because the
> > legacy signal is already queued. [...]
>
> We could do that (beyond the queued signals full event), but i think
> it's rather common to see signal overlap in the legacy case, right?

Yes, right. My point was, we do not know if people want to know
about "lost" signal in this case. Perhaps some application forgot
to unblock the signal, or the sender shouldn't send it, or the
reciever didn't react to the previous one.

But once again, I do not argue. I think the patches are nice and
useful. All I wanted to say is: I trust Masami and I have no idea
whether we need more or less info, and which events are more
"interesting".

> > [...] We do not report who sends the signal, [...]
>
> The PID of any task generating an event can be sampled, so that's
> implicit.

Yes, I missed this. If current != sender (timers, SIGIO) one can
look at entry->code = si_code.

> The principe is this: there's two extremes:
>
> A- report no event
>
> B- report every event precisely, that allows all signal state and
> actions to be reconstructed in hindsight.
>
> And there's a continuum between the two extremes. Just a random state
> between A) and B) makes little sense - but certain subsets (say an
> 'overview' of major signal events) might make sense from an analysis
> POV.
>
> But the thing is, by my reading of these patches we are pretty close to
> B) right now and the tracepoints still look sane - so we might as well
> implement your suggestions and achieve B)? That's a well-defined target
> to achieve. It would mean we need events of sigmask manipulations as
> well, and handler setting events. Plus the missing events you pointed
> out. (plus other stuff i might have forgotten about)

Fortunately, Roland has already replied:

> If we
> need to change them, that will become clear from the experiences of people
> actually using these.

In fact, the above is very close to what I meant but failed to explain ;)

Oleg.

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

Linux kernel RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact Gossamer Threads
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.