* [PATCH 1/2] Bluetooth: set sk_sndbuf and sk_rcvbuf properly in ERTM @ 2010-09-27 20:16 Gustavo F. Padovan 2010-09-27 20:16 ` [PATCH 2/2] Bluetooth: Use the proper error value from bt_skb_send_alloc() Gustavo F. Padovan 2010-09-27 21:22 ` [PATCH 1/2] Bluetooth: set sk_sndbuf and sk_rcvbuf properly in ERTM Mat Martineau 0 siblings, 2 replies; 4+ messages in thread From: Gustavo F. Padovan @ 2010-09-27 20:16 UTC (permalink / raw) To: linux-bluetooth Setting both this value to MPS * TxWin * 1.2 guarantees that we are reserving space to fit the whole txwindow in the memory, and that sendmsg() will block when the transmission window is full avoid overloading the system memory. I don't have a strong reason about the 1.2 constant in the account, we can do another tests in the future and change that value. Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi> --- net/bluetooth/l2cap_core.c | 10 +++++++++- 1 files changed, 9 insertions(+), 1 deletions(-) diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c index 44aa034..1e2ab05 100644 --- a/net/bluetooth/l2cap_core.c +++ b/net/bluetooth/l2cap_core.c @@ -3129,9 +3129,17 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hdr l2cap_pi(sk)->next_tx_seq = 0; l2cap_pi(sk)->expected_tx_seq = 0; __skb_queue_head_init(TX_QUEUE(sk)); - if (l2cap_pi(sk)->mode == L2CAP_MODE_ERTM) + if (l2cap_pi(sk)->mode == L2CAP_MODE_ERTM) { l2cap_ertm_init(sk); + sk->sk_sndbuf = (l2cap_pi(sk)->remote_tx_win * 1.2 * + (sizeof(struct l2cap_pinfo) + + l2cap_pi(sk)->mps)); + sk->sk_rcvbuf = (l2cap_pi(sk)->tx_win * 1.2 * + (sizeof(struct l2cap_pinfo) + + l2cap_pi(sk)->remote_mps)); + } + l2cap_chan_ready(sk); } -- 1.7.3 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] Bluetooth: Use the proper error value from bt_skb_send_alloc() 2010-09-27 20:16 [PATCH 1/2] Bluetooth: set sk_sndbuf and sk_rcvbuf properly in ERTM Gustavo F. Padovan @ 2010-09-27 20:16 ` Gustavo F. Padovan 2010-09-27 21:22 ` [PATCH 1/2] Bluetooth: set sk_sndbuf and sk_rcvbuf properly in ERTM Mat Martineau 1 sibling, 0 replies; 4+ messages in thread From: Gustavo F. Padovan @ 2010-09-27 20:16 UTC (permalink / raw) To: linux-bluetooth &err points to the proper error set by bt_skb_send_alloc() when it fails. Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi> --- net/bluetooth/l2cap_core.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c index 1e2ab05..1510812 100644 --- a/net/bluetooth/l2cap_core.c +++ b/net/bluetooth/l2cap_core.c @@ -1537,7 +1537,7 @@ static inline int l2cap_skbuff_fromiovec(struct sock *sk, struct msghdr *msg, in *frag = bt_skb_send_alloc(sk, count, msg->msg_flags & MSG_DONTWAIT, &err); if (!*frag) - return -EFAULT; + return err; if (memcpy_fromiovec(skb_put(*frag, count), msg->msg_iov, count)) return -EFAULT; @@ -1563,7 +1563,7 @@ static struct sk_buff *l2cap_create_connless_pdu(struct sock *sk, struct msghdr skb = bt_skb_send_alloc(sk, count + hlen, msg->msg_flags & MSG_DONTWAIT, &err); if (!skb) - return ERR_PTR(-ENOMEM); + return ERR_PTR(err); /* Create L2CAP header */ lh = (struct l2cap_hdr *) skb_put(skb, L2CAP_HDR_SIZE); @@ -1592,7 +1592,7 @@ static struct sk_buff *l2cap_create_basic_pdu(struct sock *sk, struct msghdr *ms skb = bt_skb_send_alloc(sk, count + hlen, msg->msg_flags & MSG_DONTWAIT, &err); if (!skb) - return ERR_PTR(-ENOMEM); + return ERR_PTR(err); /* Create L2CAP header */ lh = (struct l2cap_hdr *) skb_put(skb, L2CAP_HDR_SIZE); @@ -1629,7 +1629,7 @@ static struct sk_buff *l2cap_create_iframe_pdu(struct sock *sk, struct msghdr *m skb = bt_skb_send_alloc(sk, count + hlen, msg->msg_flags & MSG_DONTWAIT, &err); if (!skb) - return ERR_PTR(-ENOMEM); + return ERR_PTR(err); /* Create L2CAP header */ lh = (struct l2cap_hdr *) skb_put(skb, L2CAP_HDR_SIZE); -- 1.7.3 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] Bluetooth: set sk_sndbuf and sk_rcvbuf properly in ERTM 2010-09-27 20:16 [PATCH 1/2] Bluetooth: set sk_sndbuf and sk_rcvbuf properly in ERTM Gustavo F. Padovan 2010-09-27 20:16 ` [PATCH 2/2] Bluetooth: Use the proper error value from bt_skb_send_alloc() Gustavo F. Padovan @ 2010-09-27 21:22 ` Mat Martineau 2010-09-29 0:32 ` Gustavo F. Padovan 1 sibling, 1 reply; 4+ messages in thread From: Mat Martineau @ 2010-09-27 21:22 UTC (permalink / raw) To: Gustavo F. Padovan; +Cc: linux-bluetooth Gustavo - On Mon, 27 Sep 2010, Gustavo F. Padovan wrote: > Setting both this value to MPS * TxWin * 1.2 guarantees that we are > reserving space to fit the whole txwindow in the memory, and that > sendmsg() will block when the transmission window is full avoid > overloading the system memory. > I don't have a strong reason about the 1.2 constant in the account, we > can do another tests in the future and change that value. > > Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi> > --- > net/bluetooth/l2cap_core.c | 10 +++++++++- > 1 files changed, 9 insertions(+), 1 deletions(-) > > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c > index 44aa034..1e2ab05 100644 > --- a/net/bluetooth/l2cap_core.c > +++ b/net/bluetooth/l2cap_core.c > @@ -3129,9 +3129,17 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hdr > l2cap_pi(sk)->next_tx_seq = 0; > l2cap_pi(sk)->expected_tx_seq = 0; > __skb_queue_head_init(TX_QUEUE(sk)); > - if (l2cap_pi(sk)->mode == L2CAP_MODE_ERTM) > + if (l2cap_pi(sk)->mode == L2CAP_MODE_ERTM) { > l2cap_ertm_init(sk); > > + sk->sk_sndbuf = (l2cap_pi(sk)->remote_tx_win * 1.2 * > + (sizeof(struct l2cap_pinfo) + > + l2cap_pi(sk)->mps)); > + sk->sk_rcvbuf = (l2cap_pi(sk)->tx_win * 1.2 * > + (sizeof(struct l2cap_pinfo) + > + l2cap_pi(sk)->remote_mps)); > + } > + > l2cap_chan_ready(sk); > } > > -- > 1.7.3 I think sizeof(struct sk_buff) would be better than sizeof(struct l2cap_pinfo), since these limits apply to data buffers, not per-socket overhead. The 1.2 constant would need to be increased if we allow ERTM MPS bigger than the HCI MTU, since there would be multiple sk_buffs per PDU. However, the calculation could be updated when those MPS changes are made. It would also help to enforce some limits: SOCK_MIN_SNDBUF < sk->sk_sndbuf < sysctl_wmem_max SOCK_MIN_RCVBUF < sk->sk_rcvbuf < sysctl_rmem_max Regards, -- Mat Martineau Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] Bluetooth: set sk_sndbuf and sk_rcvbuf properly in ERTM 2010-09-27 21:22 ` [PATCH 1/2] Bluetooth: set sk_sndbuf and sk_rcvbuf properly in ERTM Mat Martineau @ 2010-09-29 0:32 ` Gustavo F. Padovan 0 siblings, 0 replies; 4+ messages in thread From: Gustavo F. Padovan @ 2010-09-29 0:32 UTC (permalink / raw) To: Mat Martineau; +Cc: linux-bluetooth Hi Mat, * Mat Martineau <mathewm@codeaurora.org> [2010-09-27 14:22:07 -0700]: > > Gustavo - > > On Mon, 27 Sep 2010, Gustavo F. Padovan wrote: > > > Setting both this value to MPS * TxWin * 1.2 guarantees that we are > > reserving space to fit the whole txwindow in the memory, and that > > sendmsg() will block when the transmission window is full avoid > > overloading the system memory. > > I don't have a strong reason about the 1.2 constant in the account, we > > can do another tests in the future and change that value. > > > > Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi> > > --- > > net/bluetooth/l2cap_core.c | 10 +++++++++- > > 1 files changed, 9 insertions(+), 1 deletions(-) > > > > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c > > index 44aa034..1e2ab05 100644 > > --- a/net/bluetooth/l2cap_core.c > > +++ b/net/bluetooth/l2cap_core.c > > @@ -3129,9 +3129,17 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hdr > > l2cap_pi(sk)->next_tx_seq = 0; > > l2cap_pi(sk)->expected_tx_seq = 0; > > __skb_queue_head_init(TX_QUEUE(sk)); > > - if (l2cap_pi(sk)->mode == L2CAP_MODE_ERTM) > > + if (l2cap_pi(sk)->mode == L2CAP_MODE_ERTM) { > > l2cap_ertm_init(sk); > > > > + sk->sk_sndbuf = (l2cap_pi(sk)->remote_tx_win * 1.2 * > > + (sizeof(struct l2cap_pinfo) + > > + l2cap_pi(sk)->mps)); > > + sk->sk_rcvbuf = (l2cap_pi(sk)->tx_win * 1.2 * > > + (sizeof(struct l2cap_pinfo) + > > + l2cap_pi(sk)->remote_mps)); > > + } > > + > > l2cap_chan_ready(sk); > > } > > > > -- > > 1.7.3 Thanks a lot for your comments, I'll rework the patches and then resend. > > I think sizeof(struct sk_buff) would be better than > sizeof(struct l2cap_pinfo), since these limits apply to data buffers, > not per-socket overhead. Sure. > > The 1.2 constant would need to be increased if we allow ERTM MPS > bigger than the HCI MTU, since there would be multiple sk_buffs per > PDU. However, the calculation could be updated when those MPS changes > are made. Yes, we can change that when we start to allow MPS greater than HCI MTU. > > It would also help to enforce some limits: > SOCK_MIN_SNDBUF < sk->sk_sndbuf < sysctl_wmem_max > SOCK_MIN_RCVBUF < sk->sk_rcvbuf < sysctl_rmem_max Sure. -- Gustavo F. Padovan ProFUSION embedded systems - http://profusion.mobi ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-09-29 0:32 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-09-27 20:16 [PATCH 1/2] Bluetooth: set sk_sndbuf and sk_rcvbuf properly in ERTM Gustavo F. Padovan 2010-09-27 20:16 ` [PATCH 2/2] Bluetooth: Use the proper error value from bt_skb_send_alloc() Gustavo F. Padovan 2010-09-27 21:22 ` [PATCH 1/2] Bluetooth: set sk_sndbuf and sk_rcvbuf properly in ERTM Mat Martineau 2010-09-29 0:32 ` Gustavo F. Padovan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).