linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: peterz@infradead.org, lkml <linux-kernel@vger.kernel.org>,
	oleg@redhat.com, Paul Mackerras <paulus@samba.org>,
	Anton Blanchard <anton@samba.org>, Ingo Molnar <mingo@elte.hu>,
	ppcdev <linuxppc-dev@lists.ozlabs.org>
Subject: Re: [PATCH v4 2/2] powerpc: Uprobes port to powerpc
Date: Fri, 24 Aug 2012 07:57:25 +1000	[thread overview]
Message-ID: <1345759045.29170.12.camel@pasglop> (raw)
In-Reply-To: <20120823161737.GG25338@linux.vnet.ibm.com>

On Thu, 2012-08-23 at 21:47 +0530, Srikar Dronamraju wrote:
> * Benjamin Herrenschmidt <benh@kernel.crashing.org> [2012-08-23 20:06:18]:
> 
> > On Thu, 2012-08-23 at 11:02 +0530, Srikar Dronamraju wrote:
> > > > 
> > > 
> > > insn is updated/accessed in the arch independent code. Size of
> > > uprobe_opcode_t could be different for different archs.
> > > uprobe_opcode_t
> > > represents the size of the smallest breakpoint instruction for an
> > > arch.
> > > 
> > > Hence u8 works out the best. I know we could still use uprobe_opcode_t
> > > and achieve the same. In which case, we would have to interpret
> > > MAX_UINSN_BYTES differently. Do you see any advantages of using
> > > uprobe_opcode_t instead of u8 across archs?
> > 
> > But don't you actively rely on the fact that on powerpc, unlike x86, you
> > -can- atomically replace an instruction with a single 32-bit store ?
> > 
> 
> We are not doing a replace here, we are only copying from the ->vm_file
> for the largest size instruction possible for that instruction. For
> powerpc, this is easy because of fixed size instructions.  
> 
> On other archs, at this point, we dont even know the length of the
> underlying instruction.
> 
> Now there are 3 ways to handle this:
> 1. use arch independent copy_insn() (current.) (handles if the
> instruction spreads across multiple pages on non fixed instruction
> archs). 
> 
> 2. make the copy_insn() arch specific, that would mean every arch will
> have to do read_mapping_page etc.
> 
> 3. have a arch specific hook in arch independent copy_insn code that
> either does a memcpy for non fixed instruction archs or does an
> assignment in archs like powerpc.
>
> I think you are suggesting option 3.
> But instead of adding another call that does the arch specific stuff, we
> are probably be better of doing a memcpy. Right?
> 
> For all powerpc references to insn we could refer to it as u32 as
> suggested by Oleg.

Ok, doens't matter much either way, it's just odd and inefficient.

Cheers,
Ben.

  reply	other threads:[~2012-08-23 21:58 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-22  8:22 [PATCH 1/2] powerpc: Add trap_nr to thread_struct Ananth N Mavinakayanahalli
2012-08-22  8:27 ` [PATCH v4 2/2] powerpc: Uprobes port to powerpc Ananth N Mavinakayanahalli
2012-08-22 15:55   ` Oleg Nesterov
2012-08-23  4:28   ` Michael Ellerman
2012-08-23  5:32     ` Srikar Dronamraju
2012-08-23 10:06       ` Benjamin Herrenschmidt
2012-08-23  9:02         ` Oleg Nesterov
2012-08-23 16:02           ` Srikar Dronamraju
2012-08-23 16:17         ` Srikar Dronamraju
2012-08-23 21:57           ` Benjamin Herrenschmidt [this message]
2012-08-24  1:33       ` Michael Ellerman
2012-08-23  5:58     ` Ananth N Mavinakayanahalli
2012-08-24  1:13       ` Michael Ellerman
2012-08-24  7:07         ` Benjamin Herrenschmidt
2012-08-24  7:37           ` Ananth N Mavinakayanahalli

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=1345759045.29170.12.camel@pasglop \
    --to=benh@kernel.crashing.org \
    --cc=anton@samba.org \
    --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 \
    --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).