On Sun, 1 Dec 2002, Manfred Spraul wrote:
> The bad thing is that you use __add_wait_queue() and call the wait queue
> locking functions yourself. This is not needed. The only function that
> was permitted to do that is sleep_on(), and that will die soon. I've
> quoted sleep_on as a reminder to kill that code in kernel/sched.c.
> Just use add_wait_queue() instead of the internal functions. Or
> prepare_to_wait/finish_wait, that has a slighly lower locking overhead.
Some more explanation. Originally I used sleep_on(). But when it occurred
to not work well on SMP I just make my own version of it with queue-wide
semaphore protecting whole code. Then it started to work, but it was ugly,
again I agree. Now I've made my own function taking wait_event() as a
template: I've removed checking for condition and add process to wait
queue with EXCLUSIVE flag. On uniprocessor it works well - but on SMP I
just a moment before had the same race - (wq were corrupted). When I added
sem IN this function it started to work ok. Why (I mean why it didn't
worked previously - in such a case I can imagine that wait_event would
also produce some races) ?
Now it looks like this:
+void inline wait_exclusive(wait_queue_head_t *wq)
+{
+ wait_queue_t wait;
+ init_waitqueue_entry(&wait, current);
+
+ add_wait_queue_exclusive(wq, &wait);
+ set_current_state(TASK_UNINTERRUPTIBLE);
+
++ up(&queue_sem)
+ schedule();
++ down(&queue_sem);
+
+ current->state = TASK_RUNNING;
+ remove_wait_queue(wq, &wait);
+}
I hope it is acceptable (?)
>
> Btw, could you explain how your message priority implementation works?
>
> If I understand it correctly, wq_add maintains a priority sorted linked
> list. wq_sleep() waits until the process becomes the first entry in the
> priority queue.
> - You use the pid value as the thread identifier - why? Usually the task
> struct pointer is used within the kernel.
Ups :-). Yes it will let me remove one extra field from struct.
> - Is it correct that wq_wakeup wakes up all processes that sleep in
> wq_send, and then the highest priority process continues? What about
> waking up just the highest priority process? Look at the wakeup code in
> ipc/msg.c - it implement message types that way. The sender looks
> through the list of waiting receivers, and directly sends the message to
> the right receiver [pipelined_send()]
Yes it was correct, but useless. Now I wakeup only one process. But note
that we use different algorithm then in msg.c: we haven't "pipelined" s&r.
I have applied previous suggestions & made some improvements - it works
well.
If you have some further notices please send (especially about first
case).
Thanks again
Krzysiek Benedyczak
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Sat Dec 07 2002 - 22:00:14 EST