Messages in this thread | | | Date | Tue, 07 Apr 2015 19:48:01 +0200 | From | Manfred Spraul <> | Subject | Re: [PATCH 3/3] ipc/mqueue: remove STATE_PENDING |
| |
On 04/07/2015 05:03 PM, Sebastian Andrzej Siewior wrote: > This patch moves the wakeup_process() invocation so it is not done under > the info->lock. With this change, the waiter is woken up once it is > "ready" which means its state is STATE_READY and it does not need to loop > on SMP if it is still in STATE_PENDING. > In the timeout case we still need to grab the info->lock to verify the state. > > This change should also avoid the introduction of preempt_disable() in > -RT which avoids a busy-loop which pools for the STATE_PENDING -> STATE_READY > change if the waiter has a higher priority compared to the waker.
> @@ -909,9 +905,8 @@ SYSCALL_DEFINE1(mq_unlink, const char __user *, u_name) > * bypasses the message array and directly hands the message over to the > * receiver. > * The receiver accepts the message and returns without grabbing the queue > - * spinlock. Therefore an intermediate STATE_PENDING state and memory barriers > - * are necessary. The same algorithm is used for sysv semaphores, see > - * ipc/sem.c for more details. > + * spinlock. The same algorithm is used for sysv semaphores, see ipc/sem.c > + * for more details. No. With your change, ipc/sem.c and ipc/msg.c use different algorithms. Please update the comment and describe the new approach:
Current approach: - set pointer to message - STATE_PENDING - wake_up_process() - STATE_READY (now the receiver can continue)
New approach: - set pointer to message - get_task_struct - STATE_READY (now the receiver can continue, e.g. woken up due to an unrelated SIGKILL) - wake_up_process() - put_task_struct()
> + if (r_sender) { > + wake_up_process(r_sender); > + put_task_struct(r_sender); > + } > ret = 0; Could you double-check that it is safe to call wake_up_process on a killed and reaped thread, only with a get_task_struct reference?
And: please test it, too. (patch the kernel so that you can trigger this case).
-- Manfred
| |