Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org BA31AC433CA
From:   Kalle Valo <kvalo@codeaurora.org>
To:     Felix Fietkau <nbd@nbd.name>
Cc:     linux-wireless@vger.kernel.org
Subject: Re: [PATCH 04/11] mt76: add utility functions for deferring work to a kernel thread
References: <20200908211756.15998-1-nbd@nbd.name>
        <20200908211756.15998-4-nbd@nbd.name>
Date:   Fri, 11 Sep 2020 08:15:05 +0000
In-Reply-To: <20200908211756.15998-4-nbd@nbd.name> (Felix Fietkau's message of
        "Tue, 8 Sep 2020 23:17:49 +0200")
Message-ID: <010101747c3b7518-86af82e8-24bd-41f9-b7e3-8fb181e9d926-000000@us-west-2.amazonses.com>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
Feedback-ID: 1.us-west-2.CZuq2qbDmUIuT3qdvXlRHZZCpfZqZ4GtG9v3VKgRyF0=:AmazonSES
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

Felix Fietkau <nbd@nbd.name> 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@nbd.name>

[...]

> --- 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?

-- 
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches