Re: s390 user_enable_single_step() (Was: odd utrace testing results on s390x)
On Mon, 4 Jan 2010 16:52:25 +0100 Oleg Nesterov o...@redhat.com wrote: Hi! We have some strange problems with utrace on s390, and so far this _looks_ like a s390 problem. Looks like, on any CPU user_enable_single_step() does not work until at least one thread with per_info.single_step = 1 does the context switch. This doesn't matter with the old ptrace implementation, but with utrace the tracee itself does user_enable_single_step(current) and returns to user-mode. Until it does at least one context switch the single-stepping doesn't work, after that everything works fine till the next reboot. The PER control registers only get reloaded on task switch. Can you test if this patch fixes your problem? -- Subject: [PATCH] fix loading of PER control registers for utrace. From: Martin Schwidefsky schwidef...@de.ibm.com If the current task enables / disables PER tracing for itself the PER control registers need to be loaded in FixPerRegisters. Signed-off-by: Martin Schwidefsky schwidef...@de.ibm.com --- arch/s390/kernel/ptrace.c |3 +++ 1 file changed, 3 insertions(+) --- a/arch/s390/kernel/ptrace.c +++ b/arch/s390/kernel/ptrace.c @@ -98,6 +98,9 @@ FixPerRegisters(struct task_struct *task per_info-control_regs.bits.storage_alt_space_ctl = 1; else per_info-control_regs.bits.storage_alt_space_ctl = 0; + + if (task == current) + __ctl_load(per_info-control_regs.words, 9, 11); } void user_enable_single_step(struct task_struct *task) -- blue skies, Martin. Reality continues to ruin my life. - Calvin.
Re: s390 user_enable_single_step() (Was: odd utrace testing results on s390x)
On 01/04, Oleg Nesterov wrote: IOW. I think this problem is minor and probably can be ignored, Or may be not... Even if the child is not killed by SIGTRAP, it can get a lot of unnecessary traps. To verify, I did another trivial patch (below), and the test case from 6580807da14c423f0d0a708108e6df6ebc8bc83d does trigger a lot of false step printks. Hmm. And sometimes there is nothing in dmesg, but the test-case needs a lot of time to complete. taskset -c seems to always trigger printk's. Magic. Oleg. --- arch/s390/kernel/traps.c~ 2009-12-22 10:41:52.909174198 -0500 +++ arch/s390/kernel/traps.c2010-01-04 13:19:51.038187586 -0500 @@ -384,6 +384,8 @@ void __kprobes do_single_step(struct pt_ } if (tracehook_consider_fatal_signal(current, SIGTRAP)) force_sig(SIGTRAP, current); + else + printk(false step\n); } static void default_trap_handler(struct pt_regs * regs, long interruption_code)
Re: s390 user_enable_single_step() (Was: odd utrace testing results on s390x)
The PER control registers only get reloaded on task switch. Can you test if this patch fixes your problem? Long ago when I first worked with David Wilder on s390 arch code, I remember we made this change. It seems to have been forgotten in the later rounds of reworking and merging. Thanks, Roland
Re: s390 user_enable_single_step() (Was: odd utrace testing results on s390x)
This probably means that copy_process()-user_disable_single_step() is not enough to clear the this task wants single-stepping copied from parent. I would suspect s390's TIF_SINGLE_STEP flag here. That flag means a single-step trap occurred. This is what causes do_single_step to be called before returning to user mode, rather than the machine trap doing it directly as is done in the other arch implementations. If I'm right, then this task wants single-stepping is not the problem, and that really is fully cleared. In fact, looking at s390's copy_thread (arch/s390/kernel/process.c) it clears out all the state that is actually touched by user_enable_single_step and user_disable_single_step. So for s390 the new fork.c call is actually superfluous AFAICT. The problem is that the copied parent state includes the this task has a pending single-step to report flag. IMHO it clearly makes sense for s390's copy_thread to clear this flag in a new task, which it does not do now. An alternative to that would be just to make its user_disable_single_step clear the flag. That could in theory also have an effect on e.g. the (authentic) pending step report of a tracee that was stopped with TIF_SINGLE_STEP set when its tracer detached. This might be considered a good thing, but since every other arch posts the SIGTRAP immediately they all have the equivalent issue and s390 doesn't need to be any better than they are before we have a generic resolution to the whole subject of tracer-induced signals (which we won't get into now). I'm not even sure from my insufficient reading of the s390 assembly code whether this path is even possible, i.e. do_signal called before do_single_step. Martin, I suggest having copy_thread clear TIF_SINGLE_STEP. That bit is always task-private state that should not be copied. Btw, given the complexity of FixPerRegisters (and its new additional cost on task==current), you might want to make user_*_single_step bail out if per_info.single_step is already set/clear on entry. Thanks, Roland