Felix Fietkau <nbd@xxxxxxxx> writes:
> On 2020-09-11 10:15, Kalle Valo wrote:
>> Felix Fietkau <nbd@xxxxxxxx> writes:
>>
>>> In order to avoid keeping work like tx scheduling pinned to the CPU it was
>>> scheduled from, it makes sense to switch from tasklets to kernel threads.
>>>
>>> Signed-off-by: Felix Fietkau <nbd@xxxxxxxx>
>>
>> [...]
>>
>>> --- a/drivers/net/wireless/mediatek/mt76/util.c
>>> +++ b/drivers/net/wireless/mediatek/mt76/util.c
>>> @@ -110,4 +110,32 @@ int mt76_get_min_avg_rssi(struct mt76_dev *dev, bool ext_phy)
>>> }
>>> EXPORT_SYMBOL_GPL(mt76_get_min_avg_rssi);
>>>
>>> +int __mt76_worker_fn(void *ptr)
>>> +{
>>> + struct mt76_worker *w = ptr;
>>> +
>>> + while (!kthread_should_stop()) {
>>> + set_current_state(TASK_INTERRUPTIBLE);
>>> +
>>> + if (kthread_should_park()) {
>>> + kthread_parkme();
>>> + continue;
>>> + }
>>> +
>>> + if (!test_and_clear_bit(MT76_WORKER_SCHEDULED, &w->state)) {
>>> + schedule();
>>> + continue;
>>> + }
>>> +
>>> + set_bit(MT76_WORKER_RUNNING, &w->state);
>>> + set_current_state(TASK_RUNNING);
>>> + w->fn(w);
>>> + cond_resched();
>>> + clear_bit(MT76_WORKER_RUNNING, &w->state);
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +EXPORT_SYMBOL_GPL(__mt76_worker_fn);
>>
>> So how is this better than, for example,
>> create_singlethread_workqueue()? And if this is better, shouldn't it be
>> part of workqueue.h instead of every driver reinventing the wheel?
>
> Unlike a workqueue, this one only allows one fixed worker function to be
> executed by the worker thread. Because of that, there is less locking
> and less code for scheduling involved.
> In fact, the function that schedules the worker is small enough that
> it's just a simple inline function.
> The difference matters in this case, because the tx worker is scheduled
> very often in a hot path.
> I don't think it fits into workqueue.h (because of the lack of
> separation between workqueue and work struct), and I don't know if other
> drivers need this, so let's keep it in mt76 and only move to a generic
> API if we find another user for it.
Ok, fair enough. But please add this info to the commit log so the
reasoning is properly documented.
--
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
