On Thu, 15 Dec 2011, Andre Guedes wrote:
Hi Mat,
On Dec 15, 2011, at 4:25 PM, Mat Martineau wrote:
Marcel & Andre -
On Wed, 14 Dec 2011, Marcel Holtmann wrote:
Hi Andre,
This patch adds to hci_core an infra-structure to scan LE devices.
The LE scan is implemented using a work_struct which is enqueued
on hdev->workqueue. The LE scan work sends commands (Set LE Scan
Parameters and Set LE Scan Enable) to the controller and waits for
its results. If commands were executed successfully a timer is set
to disable the ongoing scanning after some amount of time.
so I rather hold off on these until we get the tasklet removal patches
merged. The mgmt processing will then also be done in process context
and we can just sleep. This should make code like this a lot simpler.
While execution on a workqueue can sleep, it's not a good idea to
block for a long time like this patch does. A single-threaded
workqueue (like the hdev workqueue) will not run the next scheduled
work until the current work function returns. If code executing in
a workqueue suspends execution by using a wait queue, like
le_scan_workqueue(), then all other pending work is blocked until
le_scan_workqueue() returns.
Yes, I'm aware of it. This is why we have the schedule_timeout(). Most
of the time, we sleep for a short time since the controller sends the
command result event just after we send the command. If the controller
takes too long or simply doesn't send the command event we timeout
and abort the le scan work. This way we don't starve other works
enqueued on hdev->workqueue.
I did some work to verify what I'm asserting about workqueue
serialization in a single-threaded workqueue, and I still think I'm on
the right track for our current use of workqueues.
My initial understanding was based on what I had read in Linux Device
Drivers (3rd ed), which dates back to 2.6.10. Workqueues have since
been updated to use a thread pool (http://lwn.net/Articles/355700/),
but our hdev workqueue is still serialized. From that LWN article:
"Code which requires that workqueue tasks be executed in the order in
which they are submitted can use a WQ_UNBOUND workqueue with
max_active set to one." Look at process_one_work() in workqueue.c,
the submitted work is run in the middle ("f(work);"), and the next
work is not started until cwq_dec_nr_in_flight() is called at the end
-- after f(work) returns.
So, using schedule_timeout() doesn't help, unless we switch away
from the single threaded workqueue. I'll reply to David's email on
that point.
Do you mean "block for a long time" the time we wait for the command
result or the timeout?
Both.
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
