From: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
To: Oleg Nesterov <oleg@redhat.com>
Cc: Ingo Molnar <mingo@elte.hu>,
Peter Zijlstra <peterz@infradead.org>,
Ananth N Mavinakayanahalli <ananth@in.ibm.com>,
Anton Arapov <anton@redhat.com>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/3] uprobes: teach find_active_uprobe() to clear MMF_HAS_UPROBES
Date: Fri, 14 Sep 2012 22:13:24 +0530 [thread overview]
Message-ID: <20120914164324.GE28033@linux.vnet.ibm.com> (raw)
In-Reply-To: <20120819164042.GA25490@redhat.com>
* Oleg Nesterov <oleg@redhat.com> [2012-08-19 18:40:42]:
> The wrong MMF_HAS_UPROBES doesn't really hurt, just it triggers
> the "slow" and unnecessary handle_swbp() path if the task hits
> the non-uprobe breakpoint.
>
> So this patch changes find_active_uprobe() to check every valid
> vma and clear MMF_HAS_UPROBES if no uprobes were found. This is
> adds the slow O(n) path, but it is only called in unlikely case
> when the task hits the normal breakpoint first time after
> uprobe_unregister().
>
> Note the "not strictly accurate" comment in mmf_recalc_uprobes().
> We can fix this, we only need to teach vma_has_uprobes() to return
> a bit more more info, but I am not sure this worth the trouble.
>
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>
> ---
> kernel/events/uprobes.c | 22 ++++++++++++++++++++++
> 1 files changed, 22 insertions(+), 0 deletions(-)
>
> diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
> index 176de8c..0b7918c 100644
> --- a/kernel/events/uprobes.c
> +++ b/kernel/events/uprobes.c
> @@ -1397,6 +1397,25 @@ static bool can_skip_sstep(struct uprobe *uprobe, struct pt_regs *regs)
> return false;
> }
>
> +static void mmf_recalc_uprobes(struct mm_struct *mm)
> +{
> + struct vm_area_struct *vma;
> +
> + for (vma = mm->mmap; vma; vma = vma->vm_next) {
> + if (!valid_vma(vma, false))
> + continue;
> + /*
> + * This is not strictly accurate, we can race with
> + * uprobe_unregister() and see the already removed
> + * uprobe if delete_uprobe() was not yet called.
> + */
> + if (vma_has_uprobes(vma, vma->vm_start, vma->vm_end))
Should we set the MMF_RECALC_UPROBES here?
Its harmless but my thought was if we indeed saw a uprobe that was
already deleted, then the next time we hit a non uprobe breakpoint in
the same process context, we will not come here because
MMF_RECALC_UPROBES is cleared.
The rest looks fine.
Acked-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> + return;
> + }
> +
> + clear_bit(MMF_HAS_UPROBES, &mm->flags);
> +}
> +
> static struct uprobe *find_active_uprobe(unsigned long bp_vaddr, int *is_swbp)
> {
> struct mm_struct *mm = current->mm;
> @@ -1418,6 +1437,9 @@ static struct uprobe *find_active_uprobe(unsigned long bp_vaddr, int *is_swbp)
> } else {
> *is_swbp = -EFAULT;
> }
> +
> + if (!uprobe && test_and_clear_bit(MMF_RECALC_UPROBES, &mm->flags))
> + mmf_recalc_uprobes(mm);
> up_read(&mm->mmap_sem);
>
> return uprobe;
> --
> 1.5.5.1
>
next prev parent reply other threads:[~2012-09-14 16:47 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-08-19 16:40 [PATCH 0/3] uprobes: teach uprobes to clear MMF_HAS_UPROBES Oleg Nesterov
2012-08-19 16:40 ` [PATCH 1/3] uprobes: uprobes_treelock should not disable irqs Oleg Nesterov
2012-09-14 16:18 ` Srikar Dronamraju
2012-08-19 16:40 ` [PATCH 2/3] uprobes: introduce MMF_RECALC_UPROBES Oleg Nesterov
2012-09-14 16:32 ` Srikar Dronamraju
2012-08-19 16:40 ` [PATCH 3/3] uprobes: teach find_active_uprobe() to clear MMF_HAS_UPROBES Oleg Nesterov
2012-09-14 16:43 ` Srikar Dronamraju [this message]
2012-09-15 14:41 ` Oleg Nesterov
2012-08-27 17:44 ` [PATCH 0/3] uprobes: teach uprobes " Oleg Nesterov
2012-08-29 17:54 ` [GIT PULL] (Was: uprobes: teach uprobes to clear MMF_HAS_UPROBES) Oleg Nesterov
2012-08-30 8:18 ` Ingo Molnar
2012-09-14 17:12 ` [PATCH 0/3] uprobes: teach uprobes to clear MMF_HAS_UPROBES 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=20120914164324.GE28033@linux.vnet.ibm.com \
--to=srikar@linux.vnet.ibm.com \
--cc=ananth@in.ibm.com \
--cc=anton@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=oleg@redhat.com \
--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 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).