From: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
To: Oleg Nesterov <oleg@redhat.com>
Cc: peterz@infradead.org, antonb@thinktux.localdomain,
lkml <linux-kernel@vger.kernel.org>,
Jim Keniston <jkenisto@us.ibm.com>,
Paul Mackerras <paulus@samba.org>, Ingo Molnar <mingo@elte.hu>,
linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH v2 1/2] uprobes: Pass probed vaddr to arch_uprobe_analyze_insn()
Date: Tue, 12 Jun 2012 22:24:04 +0530 [thread overview]
Message-ID: <20120612165404.GB30555@linux.vnet.ibm.com> (raw)
In-Reply-To: <20120611161215.GA12116@redhat.com>
>
> Well, IMHO, this is confusing.
>
> First of all, why do we have this "addr" or even "vaddr"? It should
> not exists. We pass it to copy_insn(), but for what?? copy_insn()
> should simply use uprobe->offset, the virtual address for this
> particular mapping does not matter at all. I am going to send
> the cleanup.
>
Yes, we can use uprobe->offset instead of vaddr/addr.
> Note also that we should move this !UPROBE_COPY_INSN from
> install_breakpoint() to somewhere near alloc_uprobe(). This code
> is called only once, it looks a bit strange to use the "random" mm
> (the first mm vma_prio_tree_foreach() finds) and its mapping to
> verify the insn. In fact this is simply not correct and should be
> fixed, note that on x86 arch_uprobe_analyze_insn() checks
The reason we "delay" the copy_insn to the first insert is because
we have to get access to mm. For archs like x86, we want to know if the
executable is 32 bit or not (since we have a different valid set of
instructions for 32 bit and 64 bit). So in effect, if we get access to
struct file corresponding to the inode and if the inode corresponds to
32 bit executable file or 64 bit executable file during register, then
we can move it around alloc_uprobe().
WARNING: multiple messages have this Message-ID (diff)
From: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
To: Oleg Nesterov <oleg@redhat.com>
Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>,
linuxppc-dev@lists.ozlabs.org,
lkml <linux-kernel@vger.kernel.org>,
michael@ellerman.id.au, antonb@thinktux.localdomain,
Paul Mackerras <paulus@samba.org>,
benh@kernel.crashing.org, Ingo Molnar <mingo@elte.hu>,
peterz@infradead.org, Jim Keniston <jkenisto@us.ibm.com>
Subject: Re: [PATCH v2 1/2] uprobes: Pass probed vaddr to arch_uprobe_analyze_insn()
Date: Tue, 12 Jun 2012 22:24:04 +0530 [thread overview]
Message-ID: <20120612165404.GB30555@linux.vnet.ibm.com> (raw)
In-Reply-To: <20120611161215.GA12116@redhat.com>
>
> Well, IMHO, this is confusing.
>
> First of all, why do we have this "addr" or even "vaddr"? It should
> not exists. We pass it to copy_insn(), but for what?? copy_insn()
> should simply use uprobe->offset, the virtual address for this
> particular mapping does not matter at all. I am going to send
> the cleanup.
>
Yes, we can use uprobe->offset instead of vaddr/addr.
> Note also that we should move this !UPROBE_COPY_INSN from
> install_breakpoint() to somewhere near alloc_uprobe(). This code
> is called only once, it looks a bit strange to use the "random" mm
> (the first mm vma_prio_tree_foreach() finds) and its mapping to
> verify the insn. In fact this is simply not correct and should be
> fixed, note that on x86 arch_uprobe_analyze_insn() checks
The reason we "delay" the copy_insn to the first insert is because
we have to get access to mm. For archs like x86, we want to know if the
executable is 32 bit or not (since we have a different valid set of
instructions for 32 bit and 64 bit). So in effect, if we get access to
struct file corresponding to the inode and if the inode corresponds to
32 bit executable file or 64 bit executable file during register, then
we can move it around alloc_uprobe().
next prev parent reply other threads:[~2012-06-12 17:08 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-06-08 9:32 [PATCH v2 1/2] uprobes: Pass probed vaddr to arch_uprobe_analyze_insn() Ananth N Mavinakayanahalli
2012-06-08 9:32 ` Ananth N Mavinakayanahalli
2012-06-08 9:34 ` [PATCH v2 2/2] [POWERPC] uprobes: powerpc port Ananth N Mavinakayanahalli
2012-06-08 9:34 ` Ananth N Mavinakayanahalli
2012-06-08 14:58 ` [tip:perf/core] uprobes: Pass probed vaddr to arch_uprobe_analyze_insn() tip-bot for Ananth N Mavinakayanahalli
2012-06-11 16:12 ` [PATCH v2 1/2] " Oleg Nesterov
2012-06-11 16:12 ` Oleg Nesterov
2012-06-11 19:09 ` Q: a_ops->readpage() && struct file Oleg Nesterov
2012-06-11 19:09 ` Oleg Nesterov
2012-06-13 9:58 ` Peter Zijlstra
2012-06-13 9:58 ` Peter Zijlstra
2012-06-13 19:19 ` Oleg Nesterov
2012-06-13 19:19 ` Oleg Nesterov
2012-06-12 16:54 ` Srikar Dronamraju [this message]
2012-06-12 16:54 ` [PATCH v2 1/2] uprobes: Pass probed vaddr to arch_uprobe_analyze_insn() Srikar Dronamraju
2012-06-12 17:43 ` Oleg Nesterov
2012-06-12 17:43 ` Oleg Nesterov
2012-06-13 19:15 ` Oleg Nesterov
2012-06-13 19:15 ` Oleg Nesterov
2012-06-14 11:45 ` Srikar Dronamraju
2012-06-14 11:45 ` Srikar Dronamraju
2012-06-14 18:19 ` Oleg Nesterov
2012-06-14 18:19 ` Oleg Nesterov
2012-06-15 12:33 ` Srikar Dronamraju
2012-06-16 18:05 ` Oleg Nesterov
2012-06-18 12:06 ` Srikar Dronamraju
2012-06-20 17:15 ` Oleg Nesterov
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=20120612165404.GB30555@linux.vnet.ibm.com \
--to=srikar@linux.vnet.ibm.com \
--cc=antonb@thinktux.localdomain \
--cc=jkenisto@us.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=mingo@elte.hu \
--cc=oleg@redhat.com \
--cc=paulus@samba.org \
--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 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.