Messages in this thread | | | From | Thomas Gleixner <> | Subject | Re: [patch V2 3/3] signal: Allow tasks to cache one sigqueue struct | Date | Tue, 16 Mar 2021 13:36:47 +0100 |
| |
On Sat, Mar 13 2021 at 17:49, Oleg Nesterov wrote: > On 03/12, Thomas Gleixner wrote: >> >> On Fri, Mar 12 2021 at 20:26, Thomas Gleixner wrote: >> > On Fri, Mar 12 2021 at 17:11, Oleg Nesterov wrote: >> >> On 03/11, Thomas Gleixner wrote: >> >>> >> >>> @@ -456,7 +460,12 @@ static void __sigqueue_free(struct sigqu >> >>> return; >> >>> if (atomic_dec_and_test(&q->user->sigpending)) >> >>> free_uid(q->user); >> >>> - kmem_cache_free(sigqueue_cachep, q); >> >>> + >> >>> + /* Cache one sigqueue per task */ >> >>> + if (!current->sigqueue_cache) >> >>> + current->sigqueue_cache = q; >> >>> + else >> >>> + kmem_cache_free(sigqueue_cachep, q); >> >>> } >> >> >> >> This doesn't look right, note that __exit_signal() does >> >> flush_sigqueue(&sig->shared_pending) at the end, after exit_task_sighand() >> >> was already called. >> >> >> >> I'd suggest to not add the new exit_task_sighand() helper and simply free >> >> current->sigqueue_cache at the end of __exit_signal(). >> > >> > Ooops. Thanks for spotting this! >> >> Hrm. >> >> The task which is released is obviously not current, so even if there >> are still sigqueues in shared_pending then they wont end up in the >> released tasks sigqueue_cache. They can only ever end up in >> current->sigqueue_cache. > > The task which is released can be "current" if this task autoreaps. > For example, if its parent ignores SIGCHLD. In this case exit_notify() > does release_task(current).
Bah. Let me stare at it moar.
| |