LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: Miklos Szeredi <miklos@szeredi.hu>
Cc: Song Liu <songliubraving@fb.com>,
	LKML <linux-kernel@vger.kernel.org>,
	kernel-team <kernel-team@fb.com>, Ingo Molnar <mingo@redhat.com>,
	Howard McLauchlan <hmclauchlan@fb.com>,
	Josef Bacik <jbacik@fb.com>,
	Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Subject: Re: [PATCH 1/2] tracing: fix bad use of igrab in trace_uprobe.c
Date: Wed, 18 Apr 2018 11:19:16 -0400	[thread overview]
Message-ID: <20180418111916.2325bf9c@gandalf.local.home> (raw)
In-Reply-To: <CAJfpegtKsMBjqfg6Ewz3o9197GyqS6wTauuuMExj3DDZU+CPFA@mail.gmail.com>

On Wed, 18 Apr 2018 16:40:19 +0200
Miklos Szeredi <miklos@szeredi.hu> wrote:


> > The trace_uprobe (the probe event) is created, it doesn't do anything
> > until it is enabled. This function is called when it is enabled. The
> > trace_uprobe (probe event) can not be deleted while it is enabled
> > (EBUSY).
> >
> > Are you asking what happens if the file is deleted while it has probe?
> > That I don't know about (haven't tried it out). But I would hope that
> > it keeps a reference to the inode, isn't that what the igrab is for?
> > And is now being replaced by a reference on the path, or is that the
> > problem?  
> 
> No, that's not the problem.
> 
> What I don't see is how the uprobe object relates to the trace_uprobe object.
> 
> Because after the patch the uprobe object still only has a ref to the
> inode, and that can lead to the same issue as with trace_uprobe.
> OTOH if uprobe can't survive its creating trace_uprobe, then it
> doesn't need to take a ref to the inode at all, since trace_uprobe
> already holds it.   Taking an extra ref isn't incorrect, it's just
> unnecessary and confusing.
> 
> So this needs to be cleared up in some way.

The uprobe created by the trace_uprobe creation must be deleted before
the trace_uprobe can be deleted. Basically we have this:

 # cd /sys/kernel/tracing
 # echo "uprobe creation text" > uprobe_events

The trace_uprobe is created (but not the uprobe itself). This is what
calls create_trace_uprobe().

 # echo 1 > events/uprobes/enable

This enables all the trace uprobe events, which creates the uprobes.
This is the action that calls probe_event_enable(), which creates
uprobes.

At this point, any write to uprobe_events that would destroy the trace
uprobes would return with -EBUSY, and the trace uprobes will not be
deleted.

 # echo 0 > events/uprobes/enable

This will call the probe_event_disable() which will call
uprobe_unregister() which will destroy the uprobe.

Now we can delete the trace uprobe.

Does that answer your question? A uprobe created for trace uprobes can
not survive the trace uprobe itself.

-- Steve

  reply	other threads:[~2018-04-18 15:19 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-18  6:29 [PATCH 1/2] tracing: fix bad use of igrab in trace_uprobe.c Song Liu
2018-04-18  6:29 ` [PATCH 2/2] perf/core: fix bad use of igrab in kernel/event/core.c Song Liu
2018-04-19  6:17   ` Alexander Shishkin
2018-05-22 21:56     ` Song Liu
2018-05-23 13:11       ` Peter Zijlstra
2018-05-25  9:49   ` [tip:perf/core] perf/core: Fix bad use of igrab() tip-bot for Song Liu
2018-04-18 14:03 ` [PATCH 1/2] tracing: fix bad use of igrab in trace_uprobe.c Miklos Szeredi
2018-04-18 14:25   ` Steven Rostedt
2018-04-18 14:40     ` Miklos Szeredi
2018-04-18 15:19       ` Steven Rostedt [this message]
2018-04-18 16:15     ` Song Liu
2018-04-18 16:08   ` Song Liu
2018-04-18 16:25     ` Steven Rostedt

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=20180418111916.2325bf9c@gandalf.local.home \
    --to=rostedt@goodmis.org \
    --cc=hmclauchlan@fb.com \
    --cc=jbacik@fb.com \
    --cc=kernel-team@fb.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    --cc=mingo@redhat.com \
    --cc=songliubraving@fb.com \
    --cc=srikar@linux.vnet.ibm.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).