PTRACE_{READ,WRITE}{TEXT,DATA}

PTRACE_{READ,WRITE}{TEXT,DATA}

[Benjamin Herrenschmidt]

Hi Roland, David !

I noticed kernel/ptrace.c has ptrace_readdata/writedata functions that
are only used by sparc and sparc64 which implements the ptrace requests
PTRACE_READ_DATA, PTRACE_WRITE_DATA (and _TEXT variants).

Any reason not to make everybody benefit from these and moving the sparc
implementation to the generic ptrace_request (&compat) ?

It's more efficient than read/writing one word at a time... I thought
about it in the light of some work Rik is doing to make
access_process_vm useable on video ram mappings done by the X server...

If you are ok, I'll do a patch.

Cheers,
Ben

Re: PTRACE_{READ,WRITE}{TEXT,DATA}

[David Miller]

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: Tue, 29 Apr 2008 13:54:08 +1000

> I noticed kernel/ptrace.c has ptrace_readdata/writedata functions that
> are only used by sparc and sparc64 which implements the ptrace requests
> PTRACE_READ_DATA, PTRACE_WRITE_DATA (and _TEXT variants).
> 
> Any reason not to make everybody benefit from these and moving the sparc
> implementation to the generic ptrace_request (&compat) ?
> 
> It's more efficient than read/writing one word at a time... I thought
> about it in the light of some work Rik is doing to make
> access_process_vm useable on video ram mappings done by the X server...

It's kind of pointless because what gdb does these days on Linux is
use the procfs 'mem' file to directly read in parts of the inferior's
address space.

See linux_proc_xfer_partial() in gdb/linux-nat.c

Re: PTRACE_{READ,WRITE}{TEXT,DATA}

[Rik van Riel]

On Mon, 28 Apr 2008 21:34:16 -0700 (PDT)
David Miller <davem@davemloft.net> wrote:

> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Date: Tue, 29 Apr 2008 13:54:08 +1000
> 
> > I noticed kernel/ptrace.c has ptrace_readdata/writedata functions that
> > are only used by sparc and sparc64 which implements the ptrace requests
> > PTRACE_READ_DATA, PTRACE_WRITE_DATA (and _TEXT variants).
> > 
> > Any reason not to make everybody benefit from these and moving the sparc
> > implementation to the generic ptrace_request (&compat) ?
> > 
> > It's more efficient than read/writing one word at a time... I thought
> > about it in the light of some work Rik is doing to make
> > access_process_vm useable on video ram mappings done by the X server...
> 
> It's kind of pointless because what gdb does these days on Linux is
> use the procfs 'mem' file to directly read in parts of the inferior's
> address space.
> 
> See linux_proc_xfer_partial() in gdb/linux-nat.c

Strange, changing access_process_vm on Fedora 9 made gdb able to
see video memory that the X server had mmapped.

Are you sure gdb behaves as you suggest?  

On x86 my patch seems to work as I expected...

-- 
All rights reversed.

Re: PTRACE_{READ,WRITE}{TEXT,DATA}

[Benjamin Herrenschmidt]

On Mon, 2008-04-28 at 21:34 -0700, David Miller wrote:
> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Date: Tue, 29 Apr 2008 13:54:08 +1000
> 
> > I noticed kernel/ptrace.c has ptrace_readdata/writedata functions that
> > are only used by sparc and sparc64 which implements the ptrace requests
> > PTRACE_READ_DATA, PTRACE_WRITE_DATA (and _TEXT variants).
> > 
> > Any reason not to make everybody benefit from these and moving the sparc
> > implementation to the generic ptrace_request (&compat) ?
> > 
> > It's more efficient than read/writing one word at a time... I thought
> > about it in the light of some work Rik is doing to make
> > access_process_vm useable on video ram mappings done by the X server...
> 
> It's kind of pointless because what gdb does these days on Linux is
> use the procfs 'mem' file to directly read in parts of the inferior's
> address space.
> 
> See linux_proc_xfer_partial() in gdb/linux-nat.c=

Good point. That still uses access_process_vm() so Rik and I work
is still valid tho.

Thanks,
Ben.

Re: PTRACE_{READ,WRITE}{TEXT,DATA}

[Benjamin Herrenschmidt]

> Strange, changing access_process_vm on Fedora 9 made gdb able to
> see video memory that the X server had mmapped.
> 
> Are you sure gdb behaves as you suggest?  
> 
> On x86 my patch seems to work as I expected...

Because /proc/<pid>/mem also uses access_process_vm(). See
fs/proc/base.c

Cheers,
Ben.

Re: PTRACE_{READ,WRITE}{TEXT,DATA}

[Roland McGrath]

I don't see any sense in adding that.  Using /proc/pid/mem is already the
"efficient" API for this known to userland.  I don't think keeping the fd
around for that is any great burden, and if you do then the one pread64
call is just as efficient as one ptrace call to do the same thing.


Thanks,
Roland

Re: PTRACE_{READ,WRITE}{TEXT,DATA}

[Benjamin Herrenschmidt]

On Tue, 2008-04-29 at 15:07 -0700, Roland McGrath wrote:
> I don't see any sense in adding that.  Using /proc/pid/mem is already the
> "efficient" API for this known to userland.  I don't think keeping the fd
> around for that is any great burden, and if you do then the one pread64
> call is just as efficient as one ptrace call to do the same thing.

Yup, I didn't know gdb used /proc/pid/mem to be honest ... as long as it
goes through access_process_vm(), I'm fine anyway.

Cheers,
Ben.

Re: PTRACE_{READ,WRITE}{TEXT,DATA}

[Roland McGrath]

> Yup, I didn't know gdb used /proc/pid/mem to be honest ... as long as it
> goes through access_process_vm(), I'm fine anyway.

All user ABIs for this have always boiled down to access_process_vm.