> Btw, I believe we have another problem. utrace_control(SINGLESTEP) calls > user_enable_single_step() under utrace->lock. I don't really understand > the magic in enable_single_step() but is_setting_trap_flag() calls > access_process_vm(), this doesn't look good under spinlock.
Yes, this has been on my list of concerns for a long time but I always forget about it. Perhaps I intentionally let it lie as a test for reviewers' attention to detail, we'll never know. :-) > Hmm. Not sure how to fix this... Well, one thing it can do is just punt them down to UTRACE_REPORT. It's only a non-guaranteed optimization that these effects happen directly from utrace_control() without being reinforced by a final report_quiesce/report_signal callback, after all. Most or all other machines do not have anything complex or blocky going on inside those calls at all. So potentially we could have arch macros and punt only on x86. But anyway. I'd sort of intended to leave this undecided until we get more final certainty on the locking corners of the utrace API. For this and other purposes it would be advantageous to use a mutex rather than a spin lock for the utrace lock. But that doesn't mesh with the hairy RCU guarantees we try to make to avoid callers needing task refs. So perhaps we'll in the end sort all that out differently and have a mutex. Then it would be plausible to hold that mutex while doing access_process_vm, though it's still not ideal to have "instant" async utrace ops like set_events and detach block on something external like arbitrary user page faults. So perhaps independent of the mutex/spin question (which we don't need to get into right this minute), we can devise some scheme to avoid wanting to hold that lock while doing user_enable_single_step. I haven't lately thought through what exactly is getting synchronized there that matters. Thanks, Roland