Linux bluetooth development
 help / color / mirror / Atom feed
* [PATCH 0/2] Fix L2CAP timeout values
@ 2012-01-04 11:10 Andrzej Kaczmarek
  2012-01-04 11:10 ` [PATCH 1/2] Bluetooth: Fix sk_sndtimeo initialization for L2CAP socket Andrzej Kaczmarek
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Andrzej Kaczmarek @ 2012-01-04 11:10 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: kanak.gupta, Andrzej Kaczmarek

Hi,

After moving L2CAP timers to workqueues there is problem with timeout
values which are defined in miliseconds but timer functions works with
jiffies. As a result, timers are unreliable on platforms with CONFIG_HZ
not set to 1000.

Similar problem was fixed before workqueues but now it's apparently
back. This time however I left l2ca_set_timer to use values in jiffies
to avoid confusion and make code less prone to similar issues in
future (I hope).

Also together with patch from Peter Hurley it should fix problems with
SSP and RFCOMM as recently discovered (on platforms with
CONFIG_HZ != 1000).

Andrzej Kaczmarek (2):
  Bluetooth: Fix sk_sndtimeo initialization for L2CAP socket
  Bluetooth: l2cap_set_timer needs jiffies as timeout value

 include/net/bluetooth/l2cap.h |    6 +++---
 net/bluetooth/l2cap_core.c    |   12 ++++++++----
 net/bluetooth/l2cap_sock.c    |    2 +-
 3 files changed, 12 insertions(+), 8 deletions(-)

-- 
on behalf of ST-Ericsson


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] Bluetooth: Fix sk_sndtimeo initialization for L2CAP socket
  2012-01-04 11:10 [PATCH 0/2] Fix L2CAP timeout values Andrzej Kaczmarek
@ 2012-01-04 11:10 ` Andrzej Kaczmarek
  2012-01-04 15:44   ` Marcel Holtmann
  2012-01-04 11:10 ` [PATCH 2/2] Bluetooth: l2cap_set_timer needs jiffies as timeout value Andrzej Kaczmarek
  2012-01-08 20:29 ` [PATCH 0/2] Fix L2CAP timeout values Johan Hedberg
  2 siblings, 1 reply; 6+ messages in thread
From: Andrzej Kaczmarek @ 2012-01-04 11:10 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: kanak.gupta, Andrzej Kaczmarek

sk_sndtime value should be specified in jiffies thus initial value
needs to be converted from miliseconds. Otherwise this timeout is
unreliable when CONFIG_HZ is not set to 1000.

Signed-off-by: Andrzej Kaczmarek <andrzej.kaczmarek@tieto.com>
---
 net/bluetooth/l2cap_sock.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index c61d967..c57027f 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -1002,7 +1002,7 @@ static struct sock *l2cap_sock_alloc(struct net *net, struct socket *sock, int p
 	INIT_LIST_HEAD(&bt_sk(sk)->accept_q);
 
 	sk->sk_destruct = l2cap_sock_destruct;
-	sk->sk_sndtimeo = L2CAP_CONN_TIMEOUT;
+	sk->sk_sndtimeo = msecs_to_jiffies(L2CAP_CONN_TIMEOUT);
 
 	sock_reset_flag(sk, SOCK_ZAPPED);
 
-- 
on behalf of ST-Ericsson


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] Bluetooth: l2cap_set_timer needs jiffies as timeout value
  2012-01-04 11:10 [PATCH 0/2] Fix L2CAP timeout values Andrzej Kaczmarek
  2012-01-04 11:10 ` [PATCH 1/2] Bluetooth: Fix sk_sndtimeo initialization for L2CAP socket Andrzej Kaczmarek
@ 2012-01-04 11:10 ` Andrzej Kaczmarek
  2012-01-04 15:46   ` Marcel Holtmann
  2012-01-08 20:29 ` [PATCH 0/2] Fix L2CAP timeout values Johan Hedberg
  2 siblings, 1 reply; 6+ messages in thread
From: Andrzej Kaczmarek @ 2012-01-04 11:10 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: kanak.gupta, Andrzej Kaczmarek

After moving L2CAP timers to workqueues l2cap_set_timer expects timeout
value to be specified in jiffies but constants defined in miliseconds
are used. This makes timeouts unreliable when CONFIG_HZ is not set to
1000.

__set_chan_timer macro still uses jiffies as input to avoid multiple
conversions from/to jiffies for sk_sndtimeo value which is already
specified in jiffies.

Signed-off-by: Andrzej Kaczmarek <andrzej.kaczmarek@tieto.com>
---
 include/net/bluetooth/l2cap.h |    6 +++---
 net/bluetooth/l2cap_core.c    |   12 ++++++++----
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 9572cbd..b564142 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -626,13 +626,13 @@ static inline void l2cap_clear_timer(struct l2cap_chan *chan,
 #define __set_chan_timer(c, t) l2cap_set_timer(c, &c->chan_timer, (t))
 #define __clear_chan_timer(c) l2cap_clear_timer(c, &c->chan_timer)
 #define __set_retrans_timer(c) l2cap_set_timer(c, &c->retrans_timer, \
-		L2CAP_DEFAULT_RETRANS_TO);
+		msecs_to_jiffies(L2CAP_DEFAULT_RETRANS_TO));
 #define __clear_retrans_timer(c) l2cap_clear_timer(c, &c->retrans_timer)
 #define __set_monitor_timer(c) l2cap_set_timer(c, &c->monitor_timer, \
-		L2CAP_DEFAULT_MONITOR_TO);
+		msecs_to_jiffies(L2CAP_DEFAULT_MONITOR_TO));
 #define __clear_monitor_timer(c) l2cap_clear_timer(c, &c->monitor_timer)
 #define __set_ack_timer(c) l2cap_set_timer(c, &chan->ack_timer, \
-		L2CAP_DEFAULT_ACK_TO);
+		msecs_to_jiffies(L2CAP_DEFAULT_ACK_TO));
 #define __clear_ack_timer(c) l2cap_clear_timer(c, &c->ack_timer)
 
 static inline int __seq_offset(struct l2cap_chan *chan, __u16 seq1, __u16 seq2)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 3ee9bde..97f8549 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -2970,7 +2970,8 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hdr
 
 	default:
 		sk->sk_err = ECONNRESET;
-		__set_chan_timer(chan, L2CAP_DISC_REJ_TIMEOUT);
+		__set_chan_timer(chan,
+				msecs_to_jiffies(L2CAP_DISC_REJ_TIMEOUT));
 		l2cap_send_disconn_req(conn, chan, ECONNRESET);
 		goto done;
 	}
@@ -4478,7 +4479,8 @@ static inline void l2cap_check_encryption(struct l2cap_chan *chan, u8 encrypt)
 	if (encrypt == 0x00) {
 		if (chan->sec_level == BT_SECURITY_MEDIUM) {
 			__clear_chan_timer(chan);
-			__set_chan_timer(chan, L2CAP_ENC_TIMEOUT);
+			__set_chan_timer(chan,
+					msecs_to_jiffies(L2CAP_ENC_TIMEOUT));
 		} else if (chan->sec_level == BT_SECURITY_HIGH)
 			l2cap_chan_close(chan, ECONNREFUSED);
 	} else {
@@ -4546,7 +4548,8 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
 					L2CAP_CONN_REQ, sizeof(req), &req);
 			} else {
 				__clear_chan_timer(chan);
-				__set_chan_timer(chan, L2CAP_DISC_TIMEOUT);
+				__set_chan_timer(chan,
+					msecs_to_jiffies(L2CAP_DISC_TIMEOUT));
 			}
 		} else if (chan->state == BT_CONNECT2) {
 			struct l2cap_conn_rsp rsp;
@@ -4566,7 +4569,8 @@ int l2cap_security_cfm(struct hci_conn *hcon, u8 status, u8 encrypt)
 				}
 			} else {
 				l2cap_state_change(chan, BT_DISCONN);
-				__set_chan_timer(chan, L2CAP_DISC_TIMEOUT);
+				__set_chan_timer(chan,
+					msecs_to_jiffies(L2CAP_DISC_TIMEOUT));
 				res = L2CAP_CR_SEC_BLOCK;
 				stat = L2CAP_CS_NO_INFO;
 			}
-- 
on behalf of ST-Ericsson


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] Bluetooth: Fix sk_sndtimeo initialization for L2CAP socket
  2012-01-04 11:10 ` [PATCH 1/2] Bluetooth: Fix sk_sndtimeo initialization for L2CAP socket Andrzej Kaczmarek
@ 2012-01-04 15:44   ` Marcel Holtmann
  0 siblings, 0 replies; 6+ messages in thread
From: Marcel Holtmann @ 2012-01-04 15:44 UTC (permalink / raw)
  To: Andrzej Kaczmarek; +Cc: linux-bluetooth, kanak.gupta

Hi Andrzej,

> sk_sndtime value should be specified in jiffies thus initial value
> needs to be converted from miliseconds. Otherwise this timeout is
> unreliable when CONFIG_HZ is not set to 1000.
> 
> Signed-off-by: Andrzej Kaczmarek <andrzej.kaczmarek@tieto.com>
> ---
>  net/bluetooth/l2cap_sock.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)

Acked-by: Marcel Holtmann <marcel@holtmann.org>

Regards

Marcel



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] Bluetooth: l2cap_set_timer needs jiffies as timeout value
  2012-01-04 11:10 ` [PATCH 2/2] Bluetooth: l2cap_set_timer needs jiffies as timeout value Andrzej Kaczmarek
@ 2012-01-04 15:46   ` Marcel Holtmann
  0 siblings, 0 replies; 6+ messages in thread
From: Marcel Holtmann @ 2012-01-04 15:46 UTC (permalink / raw)
  To: Andrzej Kaczmarek; +Cc: linux-bluetooth, kanak.gupta

Hi Andrzej,

> After moving L2CAP timers to workqueues l2cap_set_timer expects timeout
> value to be specified in jiffies but constants defined in miliseconds
> are used. This makes timeouts unreliable when CONFIG_HZ is not set to
> 1000.
> 
> __set_chan_timer macro still uses jiffies as input to avoid multiple
> conversions from/to jiffies for sk_sndtimeo value which is already
> specified in jiffies.
> 
> Signed-off-by: Andrzej Kaczmarek <andrzej.kaczmarek@tieto.com>
> ---
>  include/net/bluetooth/l2cap.h |    6 +++---
>  net/bluetooth/l2cap_core.c    |   12 ++++++++----
>  2 files changed, 11 insertions(+), 7 deletions(-)

Acked-by: Marcel Holtmann <marcel@holtmann.org>

Regards

Marcel



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 0/2] Fix L2CAP timeout values
  2012-01-04 11:10 [PATCH 0/2] Fix L2CAP timeout values Andrzej Kaczmarek
  2012-01-04 11:10 ` [PATCH 1/2] Bluetooth: Fix sk_sndtimeo initialization for L2CAP socket Andrzej Kaczmarek
  2012-01-04 11:10 ` [PATCH 2/2] Bluetooth: l2cap_set_timer needs jiffies as timeout value Andrzej Kaczmarek
@ 2012-01-08 20:29 ` Johan Hedberg
  2 siblings, 0 replies; 6+ messages in thread
From: Johan Hedberg @ 2012-01-08 20:29 UTC (permalink / raw)
  To: Andrzej Kaczmarek; +Cc: linux-bluetooth, kanak.gupta

Hi Andrzej,

On Wed, Jan 04, 2012, Andrzej Kaczmarek wrote:
> After moving L2CAP timers to workqueues there is problem with timeout
> values which are defined in miliseconds but timer functions works with
> jiffies. As a result, timers are unreliable on platforms with CONFIG_HZ
> not set to 1000.
> 
> Similar problem was fixed before workqueues but now it's apparently
> back. This time however I left l2ca_set_timer to use values in jiffies
> to avoid confusion and make code less prone to similar issues in
> future (I hope).
> 
> Also together with patch from Peter Hurley it should fix problems with
> SSP and RFCOMM as recently discovered (on platforms with
> CONFIG_HZ != 1000).
> 
> Andrzej Kaczmarek (2):
>   Bluetooth: Fix sk_sndtimeo initialization for L2CAP socket
>   Bluetooth: l2cap_set_timer needs jiffies as timeout value
> 
>  include/net/bluetooth/l2cap.h |    6 +++---
>  net/bluetooth/l2cap_core.c    |   12 ++++++++----
>  net/bluetooth/l2cap_sock.c    |    2 +-
>  3 files changed, 12 insertions(+), 8 deletions(-)

Both patches have been applied to my bluetooth-next tree. Thanks.

Johan

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2012-01-08 20:29 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-01-04 11:10 [PATCH 0/2] Fix L2CAP timeout values Andrzej Kaczmarek
2012-01-04 11:10 ` [PATCH 1/2] Bluetooth: Fix sk_sndtimeo initialization for L2CAP socket Andrzej Kaczmarek
2012-01-04 15:44   ` Marcel Holtmann
2012-01-04 11:10 ` [PATCH 2/2] Bluetooth: l2cap_set_timer needs jiffies as timeout value Andrzej Kaczmarek
2012-01-04 15:46   ` Marcel Holtmann
2012-01-08 20:29 ` [PATCH 0/2] Fix L2CAP timeout values Johan Hedberg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox