Re: s390 && user_enable_single_step() (Was: odd utrace testing results on s390x)

Roland McGrath
2010-01-07T21:32:44Z

> > Right. That means we should leave the status quo of not clearing
> > TIF_SINGLE_STEP in user_disable_single_step.
>
> Ok, although it seems a bit strange not to do it. Perhaps I should add a
> comment about it.

It doesn't seem strange to me, but then I've just been through all this.
user_*_step is about what the task will do next. TIF_SINGLE_STEP is about
what the task has done recently. Of course more good comments always help.
I might be inclined to change the name of TIF_SINGLE_STEP so that its true
purpose is more obvious.

AFAICT, in fact it is not even about single-step per se. It means some PER
trap happened and should produce SIGTRAP. Don't you get the same thing if
you haven't used single-step, but instead used PTRACE_POKEUSR to set up
per_struct with bits that say to trigger for some other reason?

How about calling it TIF_PER_PENDING?

> So after everthing has been converted to utrace we always will load the
> control registers in FixPerRegisters.

Right. (This could well still change in the future. But that's how it is
in utrace now. And regardless of possible future implementation changes it
will always be the case that sometimes it will be called on current.)

> We could use the same compare of the control registers as the code in
> __switch_to. See below.

Yes, sounds good.

> The PSW_MASK_PER is the "global" PER enablement switch, the PER_EM_MASK
> bits enable the different PER events. We check for the PER_EM_MASK bits
> because it is easier to access in __switch_to. The return PSW is stored
> in the pt_regs structure, we would have to get a pointer to it (what
> "regs = task_pt_regs(taks)" does in FixPerRegisters). In
> FixPerRegisters we can as well use the PSW_MASK_PER bit.

I see. Thanks for the explanation.

Thanks,
Roland