Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> writes: > On Thu, May 13, 2021 at 9:55 PM Eric W. Biederman <ebiederm@xxxxxxxxxxxx> wrote: >> >> Please pull the for-v5.13-rc2 branch from the git tree: > > I really don't like this tree. > > The immediate cause for "no" is the silly > > #if IS_ENABLED(CONFIG_SPARC) > > and > > #if IS_ENABLED(CONFIG_ALPHA) > > code in kernel/signal.c. It has absolutely zero business being there, > when those architectures have a perfectly fine arch/*/kernel/signal.c > file where that code would make much more sense *WITHOUT* any odd > preprocessor games. The code is generic it just happens those functions are only used on sparc and alpha. Further I really want to make filling out siginfo_t happen in dedicated functions as much as possible in kernel/signal.c. The probably of getting it wrong without a helper functions is very strong. As the code I am fixing demonstrates. The IS_ENABLED(arch) is mostly there so we can delete the code if/when the architectures are retired in another decade or so. > But there are other oddities too, like the new > > send_sig_fault_trapno(SIGFPE, si_code, (void __user *) regs->pc, > 0, current); > > in the alpha code, which fundamentally seems bogus: using > send_sig_fault_trapno() with a '0' for trapno seems entirely > incorrect, since the *ONLY* point of that function is to set si_trapno > to something non-zero. > > So it would seem that a plain send_sig_fault() without that 0 would be > the right thing to do. As it happens the floating point emulation code on alpha is inconsistent with the non floating point emulation code. When using real floating point hardware SIGFPE on alpha always set si_trapno. The floating point emulation code does not look like it has ever set si_trapno. I continued to used send_sig_fault_trapno to point out that inconsistency. If alpha floating point emulation was in active use I expect we would care enough to put something other than 0 in there. > This also mixes in a lot of other stuff than just the fixes. Which > would have been ok during the merge window, but I'm definitely not > happy about it now. If the breakage that came with SIGTRAP TRAP_PERF had not been discovered during the merge window I would not be sending this now. It took a little time to dig to the bottom, then the code needed just a little extra time to sit, so there were not surprises. As for mixing things, I am not quite certain what you are referring to. All of the changes relate to keeping people from shooting themselves in the foot with when using siginfo. The most noise comes from send_sig_fault vs send_sig_fault_trapno, and force_sig_fault vs force_sig_fault_trapno. That is fundamental to the siginfo fix as it is there to ensure that is safe to treat si_trapno as an ordinary _sigfault union member. Which in turns makes alpha and sparc no longer special with respect to _sigfault, just a little eccentric. I will concede that renaming SIL_PERF_EVENT to SIL_FAULT_PERF_EVENT is unnecessary, but it certainly makes it clear that we are dealing with _sigfault and not some other part of siginfo_t. Eric
