[Bluez-devel] Re: [UPDATE PATCH 18/39] net/core: use wait_event_timeout()

Marcel Holtmann
2005-01-22T17:57:28Z

Hi Nishanth,

> Description: Use wait_event_timeout() instead of custom wait-queue code.
> The current code uses TASK_INTERRUPTIBLE but only cares about timing out and the
> wq event taking place (does not actively do anything in response to signals), so
> wait_event_timeout() should be ok.
>
> Signed-off-by: Nishanth Aravamudan <nacc@us.ibm.com>
>
>
> --- 2.6.11-rc1-kj-v/net/bluetooth/hidp/core.c 2005-01-15 16:55:44.000000000 -0800
> +++ 2.6.11-rc1-kj/net/bluetooth/hidp/core.c 2005-01-21 11:07:11.000000000 -0800
> @@ -36,6 +36,7 @@
> #include <linux/ioctl.h>
> #include <linux/file.h>
> #include <linux/init.h>
> +#include <linux/wait.h>
> #include <net/sock.h>
>
> #include <linux/input.h>
> @@ -325,7 +326,6 @@ static int hidp_session(void *arg)
> struct sk_buff *skb;
> int vendor = 0x0000, product = 0x0000;
> wait_queue_t ctrl_wait, intr_wait;
> - unsigned long timeo = HZ;
>
> BT_DBG("session %p", session);
>
> @@ -370,28 +370,14 @@ static int hidp_session(void *arg)
>
> hidp_del_timer(session);
>
> - if (intr_sk->sk_state != BT_CONNECTED) {
> - init_waitqueue_entry(&ctrl_wait, current);
> - add_wait_queue(ctrl_sk->sk_sleep, &ctrl_wait);
> - while (timeo && ctrl_sk->sk_state != BT_CLOSED) {
> - set_current_state(TASK_INTERRUPTIBLE);
> - timeo = schedule_timeout(timeo);
> - }
> - set_current_state(TASK_RUNNING);
> - remove_wait_queue(ctrl_sk->sk_sleep, &ctrl_wait);
> - timeo = HZ;
> - }
> + if (intr_sk->sk_state != BT_CONNECTED)
> + wait_event_timeout(*(ctrl_sk->sk_sleep),
> + (ctrl_sk->sk_state == BT_CLOSED), HZ);

and this does not make it look better. For me this is too much pointer
stuff here. I think the interface of wait_event_timeout() is wrong. It
should take a pointer to wait_queue_head_t.