Re: [patch V2 3/3] signal: Allow tasks to cache one sigqueue struct

[Thomas Gleixner]

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