LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Roland McGrath <roland@redhat.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: linux-arch@vger.kernel.org, tony.luck@intel.com,
	linux-ia64@vger.kernel.org, linux-kernel@vger.kernel.org,
	David Miller <davem@davemloft.net>,
	linuxppc-dev@ozlabs.org, Thomas Gleixner <tglx@linutronix.de>,
	sparclinux@vger.kernel.org, Paul Mackerras <paulus@samba.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Ingo Molnar <mingo@elte.hu>, Richard Henderson <rth@twiddle.net>
Subject: Re: [PATCH 6/8] ptrace: arch_ptrace -ENOSYS return
Date: Thu, 20 Mar 2008 01:16:58 -0700 (PDT)	[thread overview]
Message-ID: <20080320081658.0B23826F995@magilla.localdomain> (raw)
In-Reply-To: Linus Torvalds's message of  Wednesday, 19 March 2008 19:40:25 -0700 <alpine.LFD.1.00.0803191935080.3020@woody.linux-foundation.org>

> Hmm.. I see the whole series, and I see this patch, but I think it adds 
> new code and new complexity, and I don't really see *why*.

The motivation is to get the arch function out of the code path for the
machine-independent request handling.  I want to be able to change the
implementation later without touching the arch code again.  

The arguments passed down to arch_ptrace are sufficient for what the arch
code itself needs and for the current implementation in ptrace_request.
In future, I'd like the option of changing the code for the standard
requests to use a local data structure set up at the start of ptrace, and
such like (so more pointers and the like would need to be passed down to
ptrace_request).  These patches let me remove ptrace_request or change
its calling convention without touching the arch code again.

> Wouldn't it be nicer to just let "arch_ptrace()" return a flag saying 
> whether it handled things or not?

It would certainly be nicer.  I would prefer:

extern int arch_ptrace(struct task_struct *child, long request,
       	    	       long addr, long data, long *retval);

where it returns an error code or it returns 0 and *retval is the value
or it returns 1 and it didn't do anything.

The reason I took the approach I did instead is incrementalism.
I can't change that signature without breaking about 22 arch builds.
I'm only really prepared to thoroughly verify a change on 2 of those
myself.  It should be a simple enough change to make blind and get 
right.  But I've gotten a lot of things wrong before.  On principle,
I wouldn't really expect anyone to sign off on stuff I won't even
claim to have tried.  I did the forced_successful_syscall_return()
macro for arch's I don't try to build, and was just awful sure golly
that I hadn't got them wrong, because the generic change would break
those few arch's (not 20) without it.

So this ugliness seemed like a better bet than waiting for 20 more
arch sign-offs before any of it could go in.  You are certainly in a
position to just change the generic signature and make every arch do
the update (or fix your typos if you just tweak them all blind), and
let them grumble.  I did not presume to do so.

If you'd like a patch that changes this signature, updates all arch
implementations, and is actually verified to compile and work only
on x86 and powerpc, I'll be happy to provide that.


Thanks,
Roland

  parent reply	other threads:[~2008-03-20  8:17 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-03-19 21:17 [PATCH 1/8] ptrace: forced_successful_syscall_return() macro Roland McGrath
2008-03-19 21:18 ` [PATCH 2/8] alpha ptrace: forced_successful_syscall_return() Roland McGrath
2008-03-19 21:19 ` [PATCH 3/8] ia64 " Roland McGrath
2008-03-19 21:19 ` [PATCH 4/8] powerpc " Roland McGrath
2008-03-24  7:55   ` Paul Mackerras
2008-03-19 21:20 ` [PATCH 5/8] sparc64 " Roland McGrath
2008-03-19 21:30   ` David Miller
2008-03-19 21:20 ` [PATCH 6/8] ptrace: arch_ptrace -ENOSYS return Roland McGrath
2008-03-20  2:40   ` Linus Torvalds
2008-03-20  7:40     ` Christoph Hellwig
2008-03-20  8:16     ` Roland McGrath [this message]
2008-03-21 13:50       ` Thomas Gleixner
2008-03-21 14:07         ` Christoph Hellwig
2008-03-21 14:10         ` Sam Ravnborg
2008-03-21 14:55       ` Linus Torvalds
2008-03-21 15:07         ` Ingo Molnar
2008-03-19 21:20 ` [PATCH 7/8] x86 " Roland McGrath
2008-03-19 21:21 ` [PATCH 8/8] powerpc " Roland McGrath

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=20080320081658.0B23826F995@magilla.localdomain \
    --to=roland@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=davem@davemloft.net \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-ia64@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=mingo@elte.hu \
    --cc=paulus@samba.org \
    --cc=rth@twiddle.net \
    --cc=sparclinux@vger.kernel.org \
    --cc=tglx@linutronix.de \
    --cc=tony.luck@intel.com \
    --cc=torvalds@linux-foundation.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