Re: [GIT PULL] core/urgent for v5.16-rc6

[Linus Torvalds <torvalds@linux-foundation.org>]

On Sun, Dec 19, 2021 at 9:25 PM Dave Hansen <dave.hansen@intel.com> wrote:
>
> The patch definitely makes the code easier to read.  But, it looks like
> we need to invert the sigaltstack_size_valid() condition from the patch:

Yup, that's just me messign up when moving code around and adding the
second "unlikely()",

> Also, the sigaltstack_lock() lock really is needed over the assignments
> like this:
>
> >                 t->sas_ss_sp = (unsigned long) ss_sp;
> >                 t->sas_ss_size = ss_size;
> >                 t->sas_ss_flags = ss_flags;
> to prevent races with validate_sigaltstack().

Ugh. This code is garbage. Why the hell does it want a lock for
something stupid like this?

That lock should just be removed entirely as pointless. If some thread
has a sigaltstack that is too small, too bad.

We've never done that validation before, why did people think it was a
good idea to add it now?

And when added, why did people think it had to be done so carefully
under a lock?

Sure, go ahead and make it a "be polite, don't let people ask for
xstate features that won't fit an altstack". But at the point where
people noticed it caused lock contention, just give it up, and do the
unlocked version since it has no actual important semantics.

Whatever. I don't care that much, but this all smells like you just
dug your own hole for very questionable causes, and instead of a
"don't do that then" this all is doubling down on a bad idea.

                 Linus