If hci_req_run returns error, we erroneously leave the current
process in TASK_INTERRUPTABLE state. If we leave the process in
TASK_INTERRUPTABLE and it is preempted, this process will never
be scheduled again.
This patch fixes this issue by moving the preparation for scheduling
(add to waitqueue and set process state) to just after the hci_req_run
call.
Signed-off-by: Andre Guedes <[email protected]>
Acked-by: Johan Hedberg <[email protected]>
---
net/bluetooth/hci_core.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index b6d44a2..a3e3c96 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -95,15 +95,11 @@ static int __hci_req_sync(struct hci_dev *hdev,
hdev->req_status = HCI_REQ_PEND;
- add_wait_queue(&hdev->req_wait_q, &wait);
- set_current_state(TASK_INTERRUPTIBLE);
-
func(&req, opt);
err = hci_req_run(&req, hci_req_sync_complete);
if (err < 0) {
hdev->req_status = 0;
- remove_wait_queue(&hdev->req_wait_q, &wait);
/* req_run will fail if the request did not add any
* commands to the queue, something that can happen when
* a request with conditionals doesn't trigger any
@@ -113,6 +109,9 @@ static int __hci_req_sync(struct hci_dev *hdev,
return 0;
}
+ add_wait_queue(&hdev->req_wait_q, &wait);
+ set_current_state(TASK_INTERRUPTIBLE);
+
schedule_timeout(timeout);
remove_wait_queue(&hdev->req_wait_q, &wait);
--
1.8.1.2
Hi Andre,
* Andre Guedes <[email protected]> [2013-03-08 11:20:18 -0300]:
> If req->err is set, there is no point in queueing the HCI command
> in HCI request command queue since it won't be sent anyway.
>
> Signed-off-by: Andre Guedes <[email protected]>
> Acked-by: Johan Hedberg <[email protected]>
> ---
> net/bluetooth/hci_core.c | 6 ++++++
> 1 file changed, 6 insertions(+)
All patches have been applied to bluetooth-next. Thanks.
Gustavo
If req->err is set, there is no point in queueing the HCI command
in HCI request command queue since it won't be sent anyway.
Signed-off-by: Andre Guedes <[email protected]>
Acked-by: Johan Hedberg <[email protected]>
---
net/bluetooth/hci_core.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index f14c41f..a49c445 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2536,6 +2536,12 @@ void hci_req_add(struct hci_request *req, u16 opcode, u32 plen, void *param)
BT_DBG("%s opcode 0x%4.4x plen %d", hdev->name, opcode, plen);
+ /* If an error occured during request building, there is no point in
+ * queueing the HCI command. We can simply return.
+ */
+ if (req->err)
+ return;
+
skb = hci_prepare_cmd(hdev, opcode, plen, param);
if (!skb) {
BT_ERR("%s no memory for command (opcode 0x%4.4x)",
--
1.8.1.2
Since no one checks the returning value of hci_req_add and HCI
request errors are now handled in hci_req_run, we can make hci_
req_add returning void.
Signed-off-by: Andre Guedes <[email protected]>
Acked-by: Johan Hedberg <[email protected]>
---
include/net/bluetooth/hci_core.h | 2 +-
net/bluetooth/hci_core.c | 6 ++----
2 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 332ee50..d6c3256 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1051,7 +1051,7 @@ struct hci_request {
void hci_req_init(struct hci_request *req, struct hci_dev *hdev);
int hci_req_run(struct hci_request *req, hci_req_complete_t complete);
-int hci_req_add(struct hci_request *req, u16 opcode, u32 plen, void *param);
+void hci_req_add(struct hci_request *req, u16 opcode, u32 plen, void *param);
void hci_req_cmd_complete(struct hci_dev *hdev, u16 opcode, u8 status);
void hci_req_cmd_status(struct hci_dev *hdev, u16 opcode, u8 status);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index fa72aff..f14c41f 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2529,7 +2529,7 @@ int hci_send_cmd(struct hci_dev *hdev, __u16 opcode, __u32 plen, void *param)
}
/* Queue a command to an asynchronous HCI request */
-int hci_req_add(struct hci_request *req, u16 opcode, u32 plen, void *param)
+void hci_req_add(struct hci_request *req, u16 opcode, u32 plen, void *param)
{
struct hci_dev *hdev = req->hdev;
struct sk_buff *skb;
@@ -2541,15 +2541,13 @@ int hci_req_add(struct hci_request *req, u16 opcode, u32 plen, void *param)
BT_ERR("%s no memory for command (opcode 0x%4.4x)",
hdev->name, opcode);
req->err = -ENOMEM;
- return -ENOMEM;
+ return;
}
if (skb_queue_empty(&req->cmd_q))
bt_cb(skb)->req.start = true;
skb_queue_tail(&req->cmd_q, skb);
-
- return 0;
}
/* Get data from the previously sent command */
--
1.8.1.2
Since hci_req_run will be returning more than one error code, we
should check its returning value in __hci_req_sync.
Signed-off-by: Andre Guedes <[email protected]>
Acked-by: Johan Hedberg <[email protected]>
---
net/bluetooth/hci_core.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 5f625e0..dc76dcf 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -100,13 +100,16 @@ static int __hci_req_sync(struct hci_dev *hdev,
err = hci_req_run(&req, hci_req_sync_complete);
if (err < 0) {
hdev->req_status = 0;
- /* req_run will fail if the request did not add any
- * commands to the queue, something that can happen when
- * a request with conditionals doesn't trigger any
- * commands to be sent. This is normal behavior and
- * should not trigger an error return.
+
+ /* ENODATA means the HCI request command queue is empty.
+ * This can happen when a request with conditionals doesn't
+ * trigger any commands to be sent. This is normal behavior
+ * and should not trigger an error return.
*/
- return 0;
+ if (err == -ENODATA)
+ return 0;
+
+ return err;
}
add_wait_queue(&hdev->req_wait_q, &wait);
--
1.8.1.2
When we are building a HCI request with more the one HCI command
and one of the hci_req_add calls fail, we should have some cleanup
routine so the HCI commands already queued on HCI request can be
deleted. Otherwise, we will face some memory leaks issues.
This patch implements the HCI request error handling which is the
following: If a hci_req_add fails, we save the error code in hci_
request. Once hci_req_run is called, we verify the error field. If
it is different from zero, we delete all HCI commands already queued
and return the error code.
Signed-off-by: Andre Guedes <[email protected]>
Acked-by: Johan Hedberg <[email protected]>
---
include/net/bluetooth/hci_core.h | 5 +++++
net/bluetooth/hci_core.c | 13 ++++++++++++-
2 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 3a9cbf2..332ee50 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1042,6 +1042,11 @@ int hci_unregister_cb(struct hci_cb *hcb);
struct hci_request {
struct hci_dev *hdev;
struct sk_buff_head cmd_q;
+
+ /* If something goes wrong when building the HCI request, the error
+ * value is stored in this field.
+ */
+ int err;
};
void hci_req_init(struct hci_request *req, struct hci_dev *hdev);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index dc76dcf..fa72aff 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2443,6 +2443,7 @@ void hci_req_init(struct hci_request *req, struct hci_dev *hdev)
{
skb_queue_head_init(&req->cmd_q);
req->hdev = hdev;
+ req->err = 0;
}
int hci_req_run(struct hci_request *req, hci_req_complete_t complete)
@@ -2453,6 +2454,14 @@ int hci_req_run(struct hci_request *req, hci_req_complete_t complete)
BT_DBG("length %u", skb_queue_len(&req->cmd_q));
+ /* If an error occured during request building, remove all HCI
+ * commands queued on the HCI request queue.
+ */
+ if (req->err) {
+ skb_queue_purge(&req->cmd_q);
+ return req->err;
+ }
+
/* Do not allow empty requests */
if (skb_queue_empty(&req->cmd_q))
return -ENODATA;
@@ -2529,7 +2538,9 @@ int hci_req_add(struct hci_request *req, u16 opcode, u32 plen, void *param)
skb = hci_prepare_cmd(hdev, opcode, plen, param);
if (!skb) {
- BT_ERR("%s no memory for command", hdev->name);
+ BT_ERR("%s no memory for command (opcode 0x%4.4x)",
+ hdev->name, opcode);
+ req->err = -ENOMEM;
return -ENOMEM;
}
--
1.8.1.2
In case the HCI request queue is empty, hci_req_run should return
ENODATA instead of EINVAL. This way, hci_req_run returns a more
meaningful error value.
Signed-off-by: Andre Guedes <[email protected]>
Acked-by: Johan Hedberg <[email protected]>
---
net/bluetooth/hci_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index a3e3c96..5f625e0 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2452,7 +2452,7 @@ int hci_req_run(struct hci_request *req, hci_req_complete_t complete)
/* Do not allow empty requests */
if (skb_queue_empty(&req->cmd_q))
- return -EINVAL;
+ return -ENODATA;
skb = skb_peek_tail(&req->cmd_q);
bt_cb(skb)->req.complete = complete;
--
1.8.1.2