[RFC, PATCH 0/2] utrace/ptrace: simplify/cleanup ptrace attach

[RFC, PATCH 0/2] utrace/ptrace: simplify/cleanup ptrace attach

[Oleg Nesterov]

Shouldn't be applied without Roland's ack.

I just don't know how to merge the second patch properly. I think it would be
really nice to cleanup ptrace attach before "ptracee data structures cleanup",
but this depends on utrace-core.patch which adds exclude_ptrace().

With the first patch, the second one (and hopefully the further cleanups) does
not depend on utrace.

Or, we can start with something like the patch below. Or, instead we can move
the "already traced" check into exclude_ptrace, this way only this helper will
depend on utrace changes. But personally I'd prefer to remove exclude_ptrace()
for now and add the correct checks later.

Roland, what do you think?

Oleg.

--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -190,6 +190,9 @@ int ptrace_attach(struct task_struct *ta
 
 	audit_ptrace(task);
 
+	if (exclude_ptrace(task))
+		return -EBUSY;
+
 	retval = -EPERM;
 	if (same_thread_group(task, current))
 		goto out;
@@ -221,11 +224,6 @@ repeat:
 		goto repeat;
 	}
 
-	if (exclude_ptrace(task)) {
-		retval = -EBUSY;
-		goto bad;
-	}
-
 	if (!task->mm)
 		goto bad;
 	/* the same process cannot be attached many times */
@@ -597,14 +595,14 @@ int ptrace_traceme(void)
 {
 	int ret = -EPERM;
 
+	if (exclude_ptrace(current))
+		return -EBUSY;
 	/*
 	 * Are we already being traced?
 	 */
 repeat:
 	task_lock(current);
-	if (exclude_ptrace(current)) {
-		ret = -EBUSY;
-	} else if (!(current->ptrace & PT_PTRACED)) {
+	if (!(current->ptrace & PT_PTRACED)) {
 		/*
 		 * See ptrace_attach() comments about the locking here.
 		 */

Re: [RFC, PATCH 0/2] utrace/ptrace: simplify/cleanup ptrace attach

[Roland McGrath]

I guess I'm slightly confused.  We want to merge all of the "pure" ptrace
cleanup patches before any utrace patch.  When those are on their way,
we'll update the utrace patches not to conflict.  I don't think it makes
sense to include utrace.patch's little ptrace.c change in the baseline tree
for your ptrace cleanup patches.


Thanks,
Roland

Re: [RFC, PATCH 0/2] utrace/ptrace: simplify/cleanup ptrace attach

[Oleg Nesterov]

On 05/04, Roland McGrath wrote:
>
> I guess I'm slightly confused.

Me too ;)

> We want to merge all of the "pure" ptrace
> cleanup patches before any utrace patch.

Yes, exactly!

The second patch "ptrace: do not use task_lock() for attach" has nothing
to do with utrace, and it is really pure ptrace cleanup.

But it can't be applied to -mm tree, because it (textually) conficts with
utrace changes in ptrace_attach().

> When those are on their way,
> we'll update the utrace patches not to conflict.  I don't think it makes
> sense to include utrace.patch's little ptrace.c change in the baseline tree
> for your ptrace cleanup patches.

Yes, but in this case, how can we push it before utrace-core.patch ?

The first patch is only for -mm, to avoid the painful dependencies.


Since you seem to mostly agree with the second patch, what should I do?

Oleg.

Re: [RFC, PATCH 0/2] utrace/ptrace: simplify/cleanup ptrace attach

[Roland McGrath]

> The second patch "ptrace: do not use task_lock() for attach" has nothing
> to do with utrace, and it is really pure ptrace cleanup.

Indeed.

> But it can't be applied to -mm tree, because it (textually) conficts with
> utrace changes in ptrace_attach().

Oh, -mm.  I had not thought about the -mm patch merge order.  I just look
at the whole ptrace-related series from you as an independent series on top
of Linus -current, preceding anything else related.

> > When those are on their way,
> > we'll update the utrace patches not to conflict.  I don't think it makes
> > sense to include utrace.patch's little ptrace.c change in the baseline tree
> > for your ptrace cleanup patches.
> 
> Yes, but in this case, how can we push it before utrace-core.patch ?
> 
> The first patch is only for -mm, to avoid the painful dependencies.

I guess we should take Andrew's advice on this.  To me, it makes most sense
just to order the -mm patches so utrace comes later, and replace the utrace
patch as necessary with a compatible version.  Perhaps things would be
simpler if we made a separate standalone series or git tree (tip/ptrace?)
for ptrace cleanups.


Thanks,
Roland

Re: [RFC, PATCH 0/2] utrace/ptrace: simplify/cleanup ptrace attach

[Andrew Morton]

On Mon,  4 May 2009 12:43:48 -0700 (PDT)
Roland McGrath <roland@redhat.com> wrote:

> > > When those are on their way,
> > > we'll update the utrace patches not to conflict.  I don't think it makes
> > > sense to include utrace.patch's little ptrace.c change in the baseline tree
> > > for your ptrace cleanup patches.
> > 
> > Yes, but in this case, how can we push it before utrace-core.patch ?
> > 
> > The first patch is only for -mm, to avoid the painful dependencies.
> 
> I guess we should take Andrew's advice on this.  To me, it makes most sense
> just to order the -mm patches so utrace comes later, and replace the utrace
> patch as necessary with a compatible version.  Perhaps things would be
> simpler if we made a separate standalone series or git tree (tip/ptrace?)
> for ptrace cleanups.

Staging the utrace patch at end-of-series would make sense if utrace is
not on track for a 2.6.31 merge.

And afaict, this is indeed the case - things seem to have gone a bit
quiet on the utrace front lately.

Re: [RFC, PATCH 0/2] utrace/ptrace: simplify/cleanup ptrace attach

[Roland McGrath]

> Staging the utrace patch at end-of-series would make sense if utrace is
> not on track for a 2.6.31 merge.
> 
> And afaict, this is indeed the case - things seem to have gone a bit
> quiet on the utrace front lately.

I don't think that is really accurate.  There has been a lack of any
reviewer comments on the actual content of the utrace patch (aside from
Oleg's own), which is indeed quieter on that front than I had expected.

The comments we did get, e.g. from hch, were that a compelling user of the
API should go in, such as converting ptrace.  Oleg's current ptrace revamp
work will culminate in replacing its innards with utrace calls.  It's my
hope that all this work will be ready in time for 2.6.31.

The reason the utrace patch should appear later in the series is that the
bulk of the ptrace cleanup series (including all patches done so far) will
not depend on utrace at all and will be mergeable independent of the fate
of utrace or that of any later utrace-dependent ptrace patches.  We expect
the utrace patch will get more updates that we hash out in the course of
the ptrace work.  That being so, it makes more sense (to me) to plan to
replace it later before merge time rather than include the old patch
earlier in the series and have other patches (including ones unrelated to
it) need to do incremental updates relative to it.


Thanks,
Roland

Re: [RFC, PATCH 0/2] utrace/ptrace: simplify/cleanup ptrace attach

[Oleg Nesterov]

On 05/04, Andrew Morton wrote:
>
> On Mon,  4 May 2009 12:43:48 -0700 (PDT)
> Roland McGrath <roland@redhat.com> wrote:
>
> > I guess we should take Andrew's advice on this.  To me, it makes most sense
> > just to order the -mm patches so utrace comes later, and replace the utrace
> > patch as necessary with a compatible version.  Perhaps things would be
> > simpler if we made a separate standalone series or git tree (tip/ptrace?)
> > for ptrace cleanups.
>
> Staging the utrace patch at end-of-series would make sense if utrace is
> not on track for a 2.6.31 merge.
>
> And afaict, this is indeed the case - things seem to have gone a bit
> quiet on the utrace front lately.

The only goal of current ptrace cleanups is to simplify the "ptrace over
utrace" change (hopefully they make sense by themselves though).

I am obviously biased, but imho the only real problem with utrace-ptrace.patch
is the current ptrace code which needs cleanups.

Oleg.

Re: [RFC, PATCH 0/2] utrace/ptrace: simplify/cleanup ptrace attach

[Ingo Molnar]

* Oleg Nesterov <oleg@redhat.com> wrote:

> On 05/04, Andrew Morton wrote:
> >
> > On Mon,  4 May 2009 12:43:48 -0700 (PDT)
> > Roland McGrath <roland@redhat.com> wrote:
> >
> > > I guess we should take Andrew's advice on this.  To me, it 
> > > makes most sense just to order the -mm patches so utrace comes 
> > > later, and replace the utrace patch as necessary with a 
> > > compatible version.  Perhaps things would be simpler if we 
> > > made a separate standalone series or git tree (tip/ptrace?) 
> > > for ptrace cleanups.
> >
> > Staging the utrace patch at end-of-series would make sense if 
> > utrace is not on track for a 2.6.31 merge.
> >
> > And afaict, this is indeed the case - things seem to have gone a 
> > bit quiet on the utrace front lately.
> 
> The only goal of current ptrace cleanups is to simplify the 
> "ptrace over utrace" change (hopefully they make sense by 
> themselves though).
> 
> I am obviously biased, but imho the only real problem with 
> utrace-ptrace.patch is the current ptrace code which needs 
> cleanups.

Yes. But realize the fundamental reason for that: _without_ 
ptrace-over-utrace the utrace core code is a big chunk of dead code 
only used on the fringes. I see and agree with all the future uses 
of utrace, but it's easy to be problem-free if a facility is not 
used by anything significant.

So a clean ptrace-over-utrace plugin is absolutely needed for utrace 
to go upstream in v2.6.31. The ftrace plugin alone does not justify 
it. The real prize here is a (much!) cleaner ptrace code. Once 
ptrace is driven via utrace and it works, its value (and trust 
level) will skyrocket.

	Ingo

Re: [RFC, PATCH 0/2] utrace/ptrace: simplify/cleanup ptrace attach

[Christoph Hellwig]

On Wed, May 06, 2009 at 10:12:25AM +0200, Ingo Molnar wrote:
> Yes. But realize the fundamental reason for that: _without_ 
> ptrace-over-utrace the utrace core code is a big chunk of dead code 
> only used on the fringes. I see and agree with all the future uses 
> of utrace, but it's easy to be problem-free if a facility is not 
> used by anything significant.

The ptrace cleanups might be required for utrace, but they by
themselves don't make utrace any more useful without another
user.

> So a clean ptrace-over-utrace plugin is absolutely needed for utrace 
> to go upstream in v2.6.31. The ftrace plugin alone does not justify 
> it. The real prize here is a (much!) cleaner ptrace code. Once 
> ptrace is driven via utrace and it works, its value (and trust 
> level) will skyrocket.

There are two blockers for utrace:

 - first all architectures need to be converted to the ptrace world
   order with regsets, tracehooks and so on.  I hope we are on track
   to get this done now after I've pinged all arch maintainers.
 - we actually need a useful user of the utrace abstraction.  And just
   converting ptrace to make it slightly more complicated by using
   another abstraction just isn't it.  One useful bit that is in the
   queue is a in-kernel gdbstub for user process which would allow
   to get out of the ptrace and re-parenting mess for basic use
   cases.  But a really convincing user would be even better.

I don't think 2.6.31 is a very realistic target.  While a lot of arch
maintainers are working on their ptrace code 2.6.31 is just a too short
deadline, and I'm also not sure we'll have the ptrace code in shape
by then.  2.6.32 is much more realistic.

Re: [RFC, PATCH 0/2] utrace/ptrace: simplify/cleanup ptrace attach

[Ingo Molnar]

* Christoph Hellwig <hch@infradead.org> wrote:

> On Wed, May 06, 2009 at 10:12:25AM +0200, Ingo Molnar wrote:
> > Yes. But realize the fundamental reason for that: _without_ 
> > ptrace-over-utrace the utrace core code is a big chunk of dead code 
> > only used on the fringes. I see and agree with all the future uses 
> > of utrace, but it's easy to be problem-free if a facility is not 
> > used by anything significant.
> 
> The ptrace cleanups might be required for utrace, but they by
> themselves don't make utrace any more useful without another
> user.
> 
> > So a clean ptrace-over-utrace plugin is absolutely needed for utrace 
> > to go upstream in v2.6.31. The ftrace plugin alone does not justify 
> > it. The real prize here is a (much!) cleaner ptrace code. Once 
> > ptrace is driven via utrace and it works, its value (and trust 
> > level) will skyrocket.
> 
> There are two blockers for utrace:
> 
>  - first all architectures need to be converted to the ptrace world
>    order with regsets, tracehooks and so on.  I hope we are on track
>    to get this done now after I've pinged all arch maintainers.

It might be more effective if you also wrote patches and if you 
would shop for maintainer Acks, instead of just "pinging" people? 
;-) We've already got enough would-be-managers on lkml really.

>  - we actually need a useful user of the utrace abstraction.  And just
>    converting ptrace to make it slightly more complicated by using
>    another abstraction just isn't it.  One useful bit that is in the
>    queue is a in-kernel gdbstub for user process which would allow
>    to get out of the ptrace and re-parenting mess for basic use
>    cases.  But a really convincing user would be even better.
> 
> I don't think 2.6.31 is a very realistic target.  While a lot of 
> arch maintainers are working on their ptrace code 2.6.31 is just a 
> too short deadline, and I'm also not sure we'll have the ptrace 
> code in shape by then.  2.6.32 is much more realistic.

Really, the above isnt a blocker list, it's your personal wish-list 
for the future. Cleaning up ptrace itself is already an upstream 
advantage worth having - for years ptrace was barely maintained. It 
interfaces to enough critical projects (gdb, strace, UML, etc.) to 
be a realiable (and testable) basis for utrace.

The new features you are suggesting look potentially interesting, 
but they have zero usage right now and they will take time to 
develop. So they should be decoupled, otherwise we will just have a 
huge and problematic change-the-world merge down the line, instead 
of a more manageable gradual approach.

	Ingo

Re: [RFC, PATCH 0/2] utrace/ptrace: simplify/cleanup ptrace attach

[Christoph Hellwig]

On Wed, May 06, 2009 at 11:05:12AM +0200, Ingo Molnar wrote:
> It might be more effective if you also wrote patches and if you 
> would shop for maintainer Acks, instead of just "pinging" people? 
> ;-) We've already got enough would-be-managers on lkml really.

I have no interest touching tons of architectures where the maintainers
are much better of looking at those lowlevel bits.  See the case where
Roland tried to do ARM but still hasn't gotten any feedback as a
negative example.

> Really, the above isnt a blocker list, it's your personal wish-list 
> for the future. Cleaning up ptrace itself is already an upstream 
> advantage worth having - for years ptrace was barely maintained. It 
> interfaces to enough critical projects (gdb, strace, UML, etc.) to 
> be a realiable (and testable) basis for utrace.

The cleanups aren't there for cleanup purposes, but to actually allow
the utrace-based ptrace being used unconditionally.  There is really
no point in merging a second conditional ptrace implementation that
has to be maintained while we add another one that doesn't add a single
new feature.

Re: [RFC, PATCH 0/2] utrace/ptrace: simplify/cleanup ptrace attach

[Ingo Molnar]

* Christoph Hellwig <hch@infradead.org> wrote:

> On Wed, May 06, 2009 at 11:05:12AM +0200, Ingo Molnar wrote:
> > It might be more effective if you also wrote patches and if you 
> > would shop for maintainer Acks, instead of just "pinging" people? 
> > ;-) We've already got enough would-be-managers on lkml really.
> 
> I have no interest touching tons of architectures where the 
> maintainers are much better of looking at those lowlevel bits. 
> [...]

That's a somewhat naive expectation. Currently ptrace has a low 
mindshare and an even lower know-how share, even amongst 
architecture maintainers. Much of the ptrace code has been many 
years ago and often it has been copied over from other architectures 
and has been hacked to work sort-of. There's positive exceptions for 
sure, but generally ptrace know-how is extremely limited and there's 
a lot of architectures with little proactivity.

It is far more efficient if Roland, Oleg (or you, if you are 
interested in this stuff - which you seem to be) did RFC patches and 
asked for maintainer acks, than to depend on maintainers to do it.

We have about a dozen core kernel features that still have not 
propagated to all architectures: irqflags-tracking (for lockdep), 
genirq, stacktrace support, latencytop support, and more. We are 
just getting around to make GENERIC_TIME the only option [maybe..] - 
after years of migration.

We've got 22 architectures and they tend to slow down certain types 
of core kernel changes significantly.

> [...] See the case where Roland tried to do ARM but still hasn't 
> gotten any feedback as a negative example.

That really reinforces my point: arch maintainers are even less 
inclined to do it proactively.

> > Really, the above isnt a blocker list, it's your personal 
> > wish-list for the future. Cleaning up ptrace itself is already 
> > an upstream advantage worth having - for years ptrace was barely 
> > maintained. It interfaces to enough critical projects (gdb, 
> > strace, UML, etc.) to be a realiable (and testable) basis for 
> > utrace.
> 
> The cleanups aren't there for cleanup purposes, but to actually 
> allow the utrace-based ptrace being used unconditionally.  There 
> is really no point in merging a second conditional ptrace 
> implementation that has to be maintained while we add another one 
> that doesn't add a single new feature.

I'm well aware of what these patches are trying to achieve.

We've got the main mass of architectures covered:

 arch/ia64/Kconfig:	select HAVE_ARCH_TRACEHOOK
 arch/powerpc/Kconfig:	select HAVE_ARCH_TRACEHOOK
 arch/s390/Kconfig:	select HAVE_ARCH_TRACEHOOK
 arch/sh/Kconfig:	select HAVE_ARCH_TRACEHOOK
 arch/sparc/Kconfig:	select HAVE_ARCH_TRACEHOOK
 arch/x86/Kconfig:	select HAVE_ARCH_TRACEHOOK

I'd expect the remaining arch conversion to tracehooks to be 
deterministically finished if done by the ptrace folks - i.e. Roland 
and Oleg. It will take forever if all that happens is a 'ping' from 
you ;-)

	Ingo

Re: [RFC, PATCH 0/2] utrace/ptrace: simplify/cleanup ptrace attach

[Roland McGrath]

> It is far more efficient if Roland, Oleg (or you, if you are 
> interested in this stuff - which you seem to be) did RFC patches and 
> asked for maintainer acks, than to depend on maintainers to do it.

This has been on offer since the first user_regset stuff went into 2.6.25,
and I think I reiterated that on linux-arch when CONFIG_HAVE_ARCH_TRACEHOOK
went in.

What it does require is some arch person to at least show interest in
seeing the patches, test-build them and/or point to usable cross compiler
setups, etc.  It doesn't have to be arch maintainers, but someone at all
who uses the arch and is prepared to build kernels for it.

In the case of arm, the fine Fedora/ARM folks had already made it easy
enough for me to do two web searches and find the cross compilers, qemu
settings, and system images I could get going lickety-split without even
asking anyone for pointers.  But as hch noted, even doing 95% of the work
myself up front (built and tested!) hasn't yet helped get any feedback.

For any arch where there is anyone out there but the crickets, it's easy
for me to help with the actual code.  I just need a little direction on
arch build setups and maybe some specific arch details questions, and a
little feedback.  But where the only people you can find who've heard of an
arch say, "We haven't looked what's upstream since 2.6.22 or so," I don't
want to waste my time on untried patches that will just go stale without
ever being compiled.


Thanks,
Roland

Re: [RFC, PATCH 0/2] utrace/ptrace: simplify/cleanup ptrace attach

[Ingo Molnar]

* Roland McGrath <roland@redhat.com> wrote:

> > It is far more efficient if Roland, Oleg (or you, if you are 
> > interested in this stuff - which you seem to be) did RFC patches and 
> > asked for maintainer acks, than to depend on maintainers to do it.
> 
> This has been on offer since the first user_regset stuff went into 
> 2.6.25, and I think I reiterated that on linux-arch when 
> CONFIG_HAVE_ARCH_TRACEHOOK went in.
> 
> What it does require is some arch person to at least show interest 
> in seeing the patches, test-build them and/or point to usable 
> cross compiler setups, etc.  It doesn't have to be arch 
> maintainers, but someone at all who uses the arch and is prepared 
> to build kernels for it.
> 
> In the case of arm, the fine Fedora/ARM folks had already made it 
> easy enough for me to do two web searches and find the cross 
> compilers, qemu settings, and system images I could get going 
> lickety-split without even asking anyone for pointers.  But as hch 
> noted, even doing 95% of the work myself up front (built and 
> tested!) hasn't yet helped get any feedback.
> 
> For any arch where there is anyone out there but the crickets, 
> it's easy for me to help with the actual code.  I just need a 
> little direction on arch build setups and maybe some specific arch 
> details questions, and a little feedback.  But where the only 
> people you can find who've heard of an arch say, "We haven't 
> looked what's upstream since 2.6.22 or so," I don't want to waste 
> my time on untried patches that will just go stale without ever 
> being compiled.

that's OK. If you went so far, if you were proactive and did due 
diligence, and nobody bothered, just push the changes into 
linux-next and there's no valid basis for future objections against 
those patches.

	Ingo