All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Ingo Molnar <mingo@elte.hu>,
	Peter Zijlstra <peterz@infradead.org>,
	Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>,
	Anton Arapov <anton@redhat.com>,
	linux-kernel@vger.kernel.org
Subject: [PATCH 3/7] uprobes: change uprobe_mmap() to ignore the errors but check fatal_signal_pending()
Date: Wed, 8 Aug 2012 19:37:42 +0200	[thread overview]
Message-ID: <20120808173742.GA13262@redhat.com> (raw)
In-Reply-To: <20120808173659.GA13220@redhat.com>

Once install_breakpoint() fails uprobe_mmap() "ignores" all other
uprobes and returns the error.

It was never really needed to to stop after the first error, and
in fact it was always wrong at least in -ENOTSUPP case.

Change uprobe_mmap() to ignore the errors and always return 0.
This is not what we want in the long term, but until we teach
the callers to handle the failure it would be better to remove
the pointless complications. And this doesn't look too bad, the
only "reasonable" error is ENOMEM but in this case the caller
should be oom-killed in the likely case or the system has more
serious problems.

However it makes sense to stop if fatal_signal_pending() == T.
In particular this helps if the task was oom-killed.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 kernel/events/uprobes.c |   27 ++++++---------------------
 1 files changed, 6 insertions(+), 21 deletions(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 08ea627..2e269d1 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -996,23 +996,16 @@ static void build_probe_list(struct inode *inode,
 }
 
 /*
- * Called from mmap_region.
- * called with mm->mmap_sem acquired.
+ * Called from mmap_region/vma_adjust with mm->mmap_sem acquired.
  *
- * Return -ve no if we fail to insert probes and we cannot
- * bail-out.
- * Return 0 otherwise. i.e:
- *
- *	- successful insertion of probes
- *	- (or) no possible probes to be inserted.
- *	- (or) insertion of probes failed but we can bail-out.
+ * Currently we ignore all errors and always return 0, the callers
+ * can't handle the failure anyway.
  */
 int uprobe_mmap(struct vm_area_struct *vma)
 {
 	struct list_head tmp_list;
 	struct uprobe *uprobe, *u;
 	struct inode *inode;
-	int ret;
 
 	if (!atomic_read(&uprobe_events) || !valid_vma(vma, true))
 		return 0;
@@ -1024,24 +1017,16 @@ int uprobe_mmap(struct vm_area_struct *vma)
 	mutex_lock(uprobes_mmap_hash(inode));
 	build_probe_list(inode, vma, vma->vm_start, vma->vm_end, &tmp_list);
 
-	ret = 0;
 	list_for_each_entry_safe(uprobe, u, &tmp_list, pending_list) {
-		if (!ret) {
+		if (!fatal_signal_pending(current)) {
 			unsigned long vaddr = offset_to_vaddr(vma, uprobe->offset);
-
-			ret = install_breakpoint(uprobe, vma->vm_mm, vma, vaddr);
-			/*
-			 * We can race against uprobe_register(), see the
-			 * comment near uprobe_hash().
-			 */
-			if (ret == -EEXIST)
-				ret = 0;
+			install_breakpoint(uprobe, vma->vm_mm, vma, vaddr);
 		}
 		put_uprobe(uprobe);
 	}
 	mutex_unlock(uprobes_mmap_hash(inode));
 
-	return ret;
+	return 0;
 }
 
 /*
-- 
1.5.5.1


  parent reply	other threads:[~2012-08-08 17:41 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-08 17:36 [PATCH 0/7] uprobes: kill uprobes_state->count, add MMF_HAS_UPROBES Oleg Nesterov
2012-08-08 17:37 ` [PATCH 1/7] uprobes: kill uprobes_state->count Oleg Nesterov
2012-08-13 13:18   ` Srikar Dronamraju
2012-08-08 17:37 ` [PATCH 2/7] uprobes: kill dup_mmap()->uprobe_mmap(), simplify uprobe_mmap/munmap Oleg Nesterov
2012-08-13 13:20   ` Srikar Dronamraju
2012-08-08 17:37 ` Oleg Nesterov [this message]
2012-08-13 13:21   ` [PATCH 3/7] uprobes: change uprobe_mmap() to ignore the errors but check fatal_signal_pending() Srikar Dronamraju
2012-08-08 17:37 ` [PATCH 4/7] uprobes: do not use -EEXIST in install_breakpoint() paths Oleg Nesterov
2012-08-13 13:21   ` Srikar Dronamraju
2012-08-08 17:37 ` [PATCH 5/7] uprobes: introduce MMF_HAS_UPROBES Oleg Nesterov
2012-08-09 13:32   ` Srikar Dronamraju
2012-08-09 14:17     ` Oleg Nesterov
2012-08-13 13:22   ` Srikar Dronamraju
2012-08-08 17:37 ` [PATCH 6/7] uprobes: fold uprobe_reset_state() into uprobe_dup_mmap() Oleg Nesterov
2012-08-13 13:23   ` Srikar Dronamraju
2012-08-08 17:37 ` [PATCH 7/7] uprobes: remove "verify" argument from set_orig_insn() Oleg Nesterov
2012-08-09 13:33   ` Srikar Dronamraju

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=20120808173742.GA13262@redhat.com \
    --to=oleg@redhat.com \
    --cc=ananth@in.ibm.com \
    --cc=anton@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=peterz@infradead.org \
    --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 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.