Hi Linus,
I realized that the patch is still incomplete when answering Al...
Am 21.06.2021 um 15:37 schrieb Linus Torvalds:
On Sun, Jun 20, 2021 at 8:18 PM Michael Schmitz <schmitzmic@xxxxxxxxx> wrote:
I hope that makes more sense?
So the problem is in your debug patch: you don't set that
TIS_SWITCH_STACK in nearly enough places.
In this particular example, I think it's that you don't set it in
do_trace_exit, so when you strace the process, the system call exit -
which is where the return value will be picked up - gets that warning.
You did set TIS_SWITCH_STACK on trace_entry, but then it's cleared
again during the system call, and not set at the trace_exit path.
Oddly, your debug patch also _clears_ it on the exit path, but it
doesn't set it when do_trace_exit does the SAVE_SWITCH_STACK.
You oddly also set it for __sys_exit, but not all the other special
system calls that also do that SAVE_SWITCH_STACK.
That's the one I used to test whether my debug patch had any ill side
effects (i.e. smashing the stack) late yesterday. Forgot to add that to
the other cases.
Really, pretty much every single case of SAVE_SWITCH_STACK would need
to set it. Not just do_trace_enter/exit
Yes - done that now and the warning is gone.
It's why I didn't like Eric's debug patch either. It's quite expensive
to do, partly because you look up that curptr thing. All very nasty.
I need to talk to Geert and Andreas to find where register a1 is
preserved, but if I have to reload a1 all the time, this won't be useful
except for debugging.
It would be *much* better to make the flag be part of the stack frame,
but sadly at least on alpha we had exported the format of that stack
frame to user space.
Same on m68k, but can we push a flag _after_ the switch stack?
Anyway, I think these debug patches are not just expensive but the
m68k one most definitely is also very incomplete.
Yes, I've seen that in the meantime. Need to triple check my work next
time.
Sorry for the extra noise!
Cheers,
Michael
Linus
