Messages in this thread | | | From | Thomas Gleixner <> | Subject | Re: [patch V2 3/3] signal: Allow tasks to cache one sigqueue struct | Date | Fri, 12 Mar 2021 20:25:16 +0100 |
| |
On Fri, Mar 12 2021 at 17:18, Oleg Nesterov wrote: > On 03/12, Sebastian Andrzej Siewior wrote: >> >> On 2021-03-11 14:20:39 [+0100], Thomas Gleixner wrote: >> > --- a/kernel/signal.c >> > +++ b/kernel/signal.c >> > @@ -433,7 +433,11 @@ static struct sigqueue * >> > rcu_read_unlock(); >> > >> > if (override_rlimit || likely(sigpending <= task_rlimit(t, RLIMIT_SIGPENDING))) { >> > - q = kmem_cache_alloc(sigqueue_cachep, gfp_flags); >> > + /* Preallocation does not hold sighand::siglock */ >> > + if (sigqueue_flags || !t->sigqueue_cache) >> > + q = kmem_cache_alloc(sigqueue_cachep, gfp_flags); >> > + else >> > + q = xchg(&t->sigqueue_cache, NULL); >> >> Could it happen that two tasks saw t->sigqueue_cache != NULL, the first >> one got the pointer via xchg() and the second got NULL via xchg()? > > It is called with sighand::siglock held, we don't even need xchg().
Yes, it was me being lazy. Lemme open code it as it's actually resulting in a locked instruction.
Thanks,
tglx
| |