linux-next: manual merge of the ptrace tree with the s390 tree

linux-next: manual merge of the ptrace tree with the s390 tree

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 461 bytes --]

Hi all,

Today's linux-next merge of the ptrace tree got a conflict in
arch/s390/kernel/traps.c between commit 248bed4b0f3c ("[S390] use siginfo
for sigtrap signals") from the s390 tree and commit a288eecce525
("ptrace: kill trivial tracehooks") from the ptrace tree.

It looks like the former is a superset of the latter, so I used the
former.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 490 bytes --]

Re: linux-next: manual merge of the ptrace tree with the s390 tree

[Tejun Heo]

Hello,

On Wed, Jul 20, 2011 at 03:33:20PM +1000, Stephen Rothwell wrote:
> Today's linux-next merge of the ptrace tree got a conflict in
> arch/s390/kernel/traps.c between commit 248bed4b0f3c ("[S390] use siginfo
> for sigtrap signals") from the s390 tree and commit a288eecce525
> ("ptrace: kill trivial tracehooks") from the ptrace tree.
> 
> It looks like the former is a superset of the latter, so I used the
> former.

Yeap, pretty much.  Martin, testing if (current->ptrace) is enough.
If PT_PTRACED is not set, no other flag there is allowed to set.
Also, I think we really should standardize what gets reported in these
debug traps instead of letting each arch do its own thing.

Thanks.

-- 
tejun

Re: linux-next: manual merge of the ptrace tree with the s390 tree

[Oleg Nesterov]

On 07/21, Tejun Heo wrote:
>
>
> On Wed, Jul 20, 2011 at 03:33:20PM +1000, Stephen Rothwell wrote:
> > Today's linux-next merge of the ptrace tree got a conflict in
> > arch/s390/kernel/traps.c between commit 248bed4b0f3c ("[S390] use siginfo
> > for sigtrap signals") from the s390 tree and commit a288eecce525
> > ("ptrace: kill trivial tracehooks") from the ptrace tree.
> >
> > It looks like the former is a superset of the latter, so I used the
> > former.
>
> Yeap, pretty much.  Martin, testing if (current->ptrace) is enough.
> If PT_PTRACED is not set, no other flag there is allowed to set.

Agreed,

> Also, I think we really should standardize what gets reported in these
> debug traps instead of letting each arch do its own thing.

May be we can standardize .si_info within the single arch at least ;)

I never understood what TRAP_HWBKPT/TRAP_BRKPT actually means, and
I can be easily wrong. But, afaics, on x86 PTRACE_SINGLESTEP results
in TRAP_TRACE. Unless the tracee steps over syscall, in this case
user_single_step_siginfo() sets TRAP_BRKPT. Hmm.

And unless I misread 248bed4b0f3c s390 thinks we need TRAP_HWBKPT.

Oleg.

Re: linux-next: manual merge of the ptrace tree with the s390 tree

[Tejun Heo]

On Thu, Jul 21, 2011 at 04:44:47PM +0200, Oleg Nesterov wrote:
> On 07/21, Tejun Heo wrote:
> > Also, I think we really should standardize what gets reported in these
> > debug traps instead of letting each arch do its own thing.
> 
> May be we can standardize .si_info within the single arch at least ;)
> 
> I never understood what TRAP_HWBKPT/TRAP_BRKPT actually means, and
> I can be easily wrong. But, afaics, on x86 PTRACE_SINGLESTEP results
> in TRAP_TRACE. Unless the tracee steps over syscall, in this case
> user_single_step_siginfo() sets TRAP_BRKPT. Hmm.
> 
> And unless I misread 248bed4b0f3c s390 thinks we need TRAP_HWBKPT.

Yeah, it looks like a proper mess.  It seems ptrace left too much for
archs to decide.  Events to be reported should be defined by generic
ptrace code (there can be some exceptions but only few) and archs
should provide the mechanisms to implement them.  I'm planning on
going through ptrace arch specifics soonish.  Hopefully it can be
cleaned up somehow.

Thanks.

-- 
tejun

Re: linux-next: manual merge of the ptrace tree with the s390 tree

[Martin Schwidefsky]

On Fri, 22 Jul 2011 10:06:00 +0200
Tejun Heo <tj@kernel.org> wrote:

> On Thu, Jul 21, 2011 at 04:44:47PM +0200, Oleg Nesterov wrote:
> > On 07/21, Tejun Heo wrote:
> > > Also, I think we really should standardize what gets reported in these
> > > debug traps instead of letting each arch do its own thing.
> > 
> > May be we can standardize .si_info within the single arch at least ;)
> > 
> > I never understood what TRAP_HWBKPT/TRAP_BRKPT actually means, and
> > I can be easily wrong. But, afaics, on x86 PTRACE_SINGLESTEP results
> > in TRAP_TRACE. Unless the tracee steps over syscall, in this case
> > user_single_step_siginfo() sets TRAP_BRKPT. Hmm.
> > 
> > And unless I misread 248bed4b0f3c s390 thinks we need TRAP_HWBKPT.
> 
> Yeah, it looks like a proper mess.  It seems ptrace left too much for
> archs to decide.  Events to be reported should be defined by generic
> ptrace code (there can be some exceptions but only few) and archs
> should provide the mechanisms to implement them.  I'm planning on
> going through ptrace arch specifics soonish.  Hopefully it can be
> cleaned up somehow.

The ptrace code differs because the architectures are different in
regard to tracing, no? For s390 instruction single stepping and
hardware breakpoints are the same thing. It all gets mapped to the
progam-event-recording (PER) tracer. You have a start and an end
address and a few events you want to see for this area (e.g.
instruction fetch or storage alteration).
I could special case ptrace with PTRACE_SINGLESTEP to return with
TRAP_TRACE instead of TRAP_HWBKPT but that would be artificial.
>From the hardware perspective its the same.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

Re: linux-next: manual merge of the ptrace tree with the s390 tree

[Tejun Heo]

Hello, Martin.

On Fri, Jul 22, 2011 at 10:29:22AM +0200, Martin Schwidefsky wrote:
> I could special case ptrace with PTRACE_SINGLESTEP to return with
> TRAP_TRACE instead of TRAP_HWBKPT but that would be artificial.
> From the hardware perspective its the same.

It's true that ptrace is more dependent on arch details than most but
still part of user-facing API and maintaining API consistency should
haver higher priority, so I really think we should have well defined
generic set of event information all archs implement and then some
arch dependent exception where absolutely necessary.  The problem at
this stage tho is that I don't have much idea how to divide them.  Eh
well, let's see how it goes. :)

Thanks.

-- 
tejun

Re: linux-next: manual merge of the ptrace tree with the s390 tree

[Oleg Nesterov]

On 07/22, Tejun Heo wrote:
>
> Yeah, it looks like a proper mess.

Yes.

> It seems ptrace left too much for
> archs to decide.  Events to be reported should be defined by generic
> ptrace code

I agree very much. Right now I am not sure if it really makes sense
to avoid the SIGTRAP signals, but in any case I think that at least
we need the generic ptrace_sigtrap(si_code, ...) helper which hides
all details.

And note that force_sig*() we use currently is wrong in this case,
it removes SIGNAL_UNKILLABLE.

And we should also cleanup the force_* mess. Also, it would be
nice to remove the "task_struct *t" argument, force_sig_info()
should be only used for synchronous signals. Afaics, only oom
killer really needs force_sig_info() with t != current. And this
reminds me, we need send_sigkill().

Oleg.