Re: [RFC] Bluetooth: Use struct delayed_work for HCI command timeout

[Alexander Holler <holler@ahsoftware.de>]

Am 05.06.2014 12:53, schrieb Marcel Holtmann:
> Hi Alexander,
>
>>>> This is an experimental patch that converts hdev->cmd_timer from
>>>> struct timer_list to struct delayed_work.
>>>
>>> I don't know what this patch should change.
>>>
>>> If I understand it correctly, a workqueue is used instead of a timer.
>>> But besides that nothing else was changed. So instead of a timer, a work
>>> now kills the hci-cmd-task and posts an error. And that is exactly what
>>> happened here. So after one (failed) try, I've gone back to use my 2
>>> small patches.
>>
>> I assume the reasoning for the patch was to get the load of system queue somehow into the timeout. But that can't work. E.g. if the queue is already very busy when the hci command is (scheduled to) send and the (delayed) timeout is put on the queue (too), then it might happen that the command is send and immediately afterwards the timeout happens. Or the response from the dongle is already in the queue but happens after the timeout happens.
>
> the main reason is that we have to move away from timer_list usage since we want to fully run outside of the interrupt context. It seems that the cmd_timeout was forgotten when we did the switch to workqueue processing. I did not expect that this change will fix anything right away, but it is the ground work to actually move into simpler locking which could potentially an issue for certain delays and unexpected interactions with the USB subsystem.
>
> Keep in mind that we currently are using 3 workqueues. The system wide one, and two per HCI controller. I am pretty certain we need to investigate our general HCI packet processing vs the actual HCI command/event processing. For the HCI command/event processing we do rely on this being single threaded and ordered, but for the general HCI packet processing, we might can go multi-threaded and allow work to move from one CPU/core to another.
 >
> Increasing the timeout like you did is just fixing a symptom. We need to figure out the root cause why a system takes so long to report results of the HCI command and/or what it wastes the time. USB might be a bottle neck here for sure, but then this might needs fixing as well.
>
> Johan, did you have a look at my patch and see if it is generally sound. Should we include it and see how good (or bad) it is behaving in general. From there on we can optimize or workqueue usage.

Increasing the timeout isn't fixing a symptom, the timeout itself is the 
problem. It is arbitrary short and getting a reasonable value for such 
is almost impossible on non-realtime-systems like Linux. Even on my 
fastest machine with plenty RAM, I'm able to pause almost everything for 
some seconds by switching kernel versions in my git kernel repo.

But I'm much more concerned about the fact, that the timeout masks 
almost all in-kernel errors too. So one never can be sure what the 
reason for the timeout is. That's why I've ended up with the first 
patch. Using that patch I can at least be sure that I don't have to 
search for a deadlock.

Regards,

Alexander Holler