Re: [RFC] TIF_NOTIFY_RESUME, arch/*/*/*signal*.c and all such

[Al Viro <viro@ZenIV.linux.org.uk>]

On Fri, Apr 27, 2012 at 01:20:02PM -0700, Roland McGrath wrote:

> But I will say that the intent of tracehook_signal_handler has always
> been what its kerneldoc says: Call it once when handler setup is
> complete (exactly once per signal delivery, so potentially multiple
> times before actually returning to user mode).  Though it is indeed a
> no-op today when stepping==0, we want its use to continue to conform
> to that exact definition so that one day we could add e.g. a
> PTRACE_EVENT_SIGNAL_HANDLED feature just by hacking tracehook.h and
> not have to go back into every arch's signal code and recover
> understanding of how the call is being used.  (It was more than enough
> work to do that once when I broke out and documented the tracehook.h
> interfaces the first time.)  You know, as if we thought modularity
> were a useful notion.

OK...  FWIW, it sounds like an argument for using it (or a function in
kernel/signal.c that would call it) on all architectures.

Note that there's an extra complication on alpha and itanic - we have more
than just struct pt_regs * there.  If we care about the rest of registers
(struct switch_stack on alpha), we probably need to do something about that.
Hell knows...  On alpha, in particular, we always get switch_stack argument
of do_notify_resume() et.al. at the constant offset below pt_regs one -
        mov     $sp, $16
        bsr     $1, do_switch_stack
        mov     $sp, $17
is how it's done.  So there we could switch to use of container_of().  On
ia64 we have struct sigscratch with pt_regs embedded into it, so there
container_of() is also reasonable.  I wonder how alpha and itanic deal
with do_coredump(), BTW - it gets pt_regs, but not the rest...

Another fun story: x86 and mips has do_notify_resume() with void *unused as
its second argument.  It used to be sigset_t *oldset and it remains unused
since 2006 or so.  Three years later arch/score went in - with the same
void *unused as the second argument of do_notify_resume().  Gotta love the
cargo-cult programming...

BTW, what's the story with 'cookie' argument of get_signal_to_deliver()?
Everything passes NULL in it and nothing actually looks at the value
passed (it's passed further without being looked at, until it reaches
ptrace_signal_deliver(), which ignores it completely on all architectures).
AFAICS, it's a rudiment of something that was used only on sparc and got
discarded as hopeless in commit 28e6103665301ce60634e8a77f0b657c6cc099de
Author: David S. Miller <davem@davemloft.net>
Date:   Sun May 11 02:07:19 2008 -0700
    sparc: Fix debugger syscall restart interactions.
Is there anything else to it?  If not, I'd rather get rid of that thing...
Frankly, I'm somewhat tempted to add something like
struct k_sigcontext {
	int sig;
	siginfo_t info;
	struct k_sigaction ka;
}; and turn get_signal_to_deliver into
bool get_signal_to_deliver(struct k_sigcontext *ctx, struct pt_regs *regs),
with ctx passed through handle_signal/setup_frame/tracehook_signal_deliver
instead of the same triple shoved into three arguments, in more or less
random order as it's done now.  Perhaps let tracehook_signal_deliver() keep
its type and semantics and introduce
void signal_delivered(struct k_sigcontext *, struct pt_regs *, int stepping)
that would be called by all handle_signal() after successful frame setup
and would call tracehook_... itself, after having done block_sigmask()...