Re: [PATCH] signal: Allow tasks to cache one sigqueue struct (again).

[Sebastian Andrzej Siewior]

On 2022-11-29 12:06:22 [-0800], Linus Torvalds wrote:
> In other words, this is *really* meant as not a good patch, but as a
> "can we please go in this direction instead".
> 
> And yes, maybe some other "free this siginfo" paths should *also* do
> that "try to add it to the cache, and if so, you're done" after doing
> the accounting. So this is probably all mis-guided, but I ended up
> wanting to just see how an alternative approach would look, and this
> feels a lot safer to me.

That one case which was not covered by the sighand lock was clearly an
oversight. It should have been under the sighand. The commit description
said so.

> We already have that SIGQUEUE_PREALLOC case, why not use some of the
> same logic for the cached entry. Sure, it bypasses the rlimit, but
> what else is new?

You need set sigqueue_cache to NULL in copy_process().

The SIGQUEUE_PREALLOC signals work slightly different. It is allocated
once at timer_create(2) time and can send a signal multiple times before
finally removed by timer_delete(2). So the signal delivery is, in a way,
already cached. The caching here for timer_create(2) does not work as
__send_signal_locked() (where you grab the cached struct) isn't used by
that code using SIGQUEUE_PREALLOC.
Instead it allocates its sigqueue via sigqueue_alloc() while not holding
the sighand lock.
So after a timer_delete(2), the next signal it receives (which is not
from a posix timer/ SIGQUEUE_PREALLOC) would be from the cache.

>               Linus

Sebastian