Re: [PATCH V3 01/14 next] qca_spi: Improve SPI thread creation
From: Jakub Kicinski <kuba@kernel.org>
Date: 2024-01-26 02:36:40
Also in:
lkml
On Wed, 24 Jan 2024 23:31:58 +0100 Stefan Wahren wrote:
The qca_spi driver create/stop the SPI kernel thread in case of netdev_open/close. This isn't optimal because there is no need for such an expensive operation. So improve this by moving create/stop of the SPI kernel into the init/uninit ops. The open/close ops could just 'park/unpark' the SPI kernel thread.
What's the concern? I don't think that creating a thread is all expensive. And we shouldn't have a thread sitting around when the interface isn't use. I mean - if you ask me what's better a small chance that the creation will fail at open or having a parked and unused thread when device is down - I'd pick the former.. But I may well be missing the point.
quoted hunk ↗ jump to hunk
@@ -825,6 +813,7 @@ static int qcaspi_netdev_init(struct net_device *dev) { struct qcaspi *qca = netdev_priv(dev); + struct task_struct *thread; dev->mtu = QCAFRM_MAX_MTU; dev->type = ARPHRD_ETHER;@@ -848,6 +837,15 @@ qcaspi_netdev_init(struct net_device *dev) return -ENOBUFS; } + thread = kthread_create(qcaspi_spi_thread, qca, "%s", dev->name); + if (IS_ERR(thread)) { + netdev_err(dev, "%s: unable to start kernel thread.\n", + QCASPI_DRV_NAME); + return PTR_ERR(thread);
I'm 90% sure this leaks resources on failure, too. Rest of the series LGTM!