All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Ingo Molnar <mingo@elte.hu>
Cc: Masami Hiramatsu <mhiramat@redhat.com>,
	lkml <linux-kernel@vger.kernel.org>,
	Roland McGrath <roland@redhat.com>,
	Jason Baron <jbaron@redhat.com>,
	systemtap <systemtap@sources.redhat.com>,
	DLE <dle-develop@lists.sourceforge.net>
Subject: Re: [PATCH -tip v3 0/3] tracepoint: Add signal events
Date: Wed, 25 Nov 2009 18:41:01 +0100	[thread overview]
Message-ID: <20091125174101.GA18349@redhat.com> (raw)
In-Reply-To: <20091124213727.GA11347@elte.hu>

On 11/24, Ingo Molnar wrote:
>
> * Oleg Nesterov <oleg@redhat.com> 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.


  reply	other threads:[~2009-11-25 17:46 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-11-20 21:31 [PATCH -tip v3 0/3] tracepoint: Add signal events Masami Hiramatsu
2009-11-20 21:31 ` [PATCH -tip v3 1/3] tracepoint: Move signal sending tracepoint to events/signal.h Masami Hiramatsu
2009-11-24 20:49   ` Oleg Nesterov
2009-11-24 21:00     ` Masami Hiramatsu
2009-11-20 21:31 ` [PATCH -tip v3 2/3] tracepoint: Add signal deliver event Masami Hiramatsu
2009-11-20 21:31 ` [PATCH -tip v3 3/3] tracepoint: Add signal loss events Masami Hiramatsu
2009-11-23 17:57 ` [PATCH -tip v3 0/3] tracepoint: Add signal events Ingo Molnar
2009-11-24 21:22   ` Oleg Nesterov
2009-11-24 21:37     ` Ingo Molnar
2009-11-25 17:41       ` Oleg Nesterov [this message]
2009-11-24 21:45     ` Masami Hiramatsu
2009-11-23 23:00 ` Jason Baron

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=20091125174101.GA18349@redhat.com \
    --to=oleg@redhat.com \
    --cc=dle-develop@lists.sourceforge.net \
    --cc=jbaron@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhiramat@redhat.com \
    --cc=mingo@elte.hu \
    --cc=roland@redhat.com \
    --cc=systemtap@sources.redhat.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 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.