Messages in this thread Patch in this message | | | Date | Mon, 29 May 2006 15:44:02 -0700 | From | Andrew Morton <> | Subject | Re: [PATCH 2.6.17-rc5] ieee1394_core: switch to kthread API |
| |
On Sun, 28 May 2006 18:43:52 +0200 (CEST) Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
We end up with:
:static void queue_packet_complete(struct hpsb_packet *packet) :{ : if (packet->no_waiter) { : hpsb_free_packet(packet); : return; : } : if (packet->complete_routine != NULL) { : skb_queue_tail(&hpsbpkt_queue, packet->skb); : wake_up_process(khpsbpkt_thread); : } : return; :} : :static int hpsbpkt_thread(void *__hi) :{ : struct sk_buff *skb; : struct hpsb_packet *packet; : void (*complete_routine)(void*); : void *complete_data; : : current->flags |= PF_NOFREEZE; : : while (!kthread_should_stop()) { : while ((skb = skb_dequeue(&hpsbpkt_queue)) != NULL) { : packet = (struct hpsb_packet *)skb->data; : : complete_routine = packet->complete_routine; : complete_data = packet->complete_data; : : packet->complete_routine = packet->complete_data = NULL; : : complete_routine(complete_data); : }
There's a race here.
: set_current_state(TASK_INTERRUPTIBLE ); : schedule(); : } : : return 0; :}
If queue_packet_complete() is called on another CPU in that window, there will be a new skb queued and we'll miss the wakeup.
I used skb_peek() in the below fix, but there are other ways, perhaps..
--- devel/drivers/ieee1394/ieee1394_core.c~ieee1394_core-switch-to-kthread-api-fix 2006-05-29 15:42:30.000000000 -0700 +++ devel-akpm/drivers/ieee1394/ieee1394_core.c 2006-05-29 15:42:40.000000000 -0700 @@ -1001,7 +1001,6 @@ void abort_timedouts(unsigned long __opa static struct task_struct *khpsbpkt_thread; static struct sk_buff_head hpsbpkt_queue; - static void queue_packet_complete(struct hpsb_packet *packet) { if (packet->no_waiter) { @@ -1036,10 +1035,11 @@ static int hpsbpkt_thread(void *__hi) complete_routine(complete_data); } - set_current_state(TASK_INTERRUPTIBLE ); - schedule(); + set_current_state(TASK_INTERRUPTIBLE); + if (!skb_peek(&hpsbpkt_queue)) + schedule(); + __set_current_state(TASK_RUNNING); } - return 0; } _ - 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/
| |