Skip to contentSkip to conversation

kthread: remove worker->task self-assignment

PATCH RESEND New Bradley Morgan 4 participants 16 messages Aug 29 – Sep 2

Subsystems (7)
Networking Network drivers Media (V4L/DVB) Scheduler +3 more subsystems
lore
Share & tools
lore

Bradley Morgan

author
#0 lore raw

cover kthread: remove worker->task self-assignment

From: Bradley Morgan <[email protected]>

Resent as a proper series with cover letter. The first send went out as five separate threads instead of one.

kthread_worker_fn() has carried a FIXME for a decade: it assigns worker->task = current because the old kthread_run(kthread_worker_fn) callers never told the worker what task it was. The new kthread_create_worker*() API sets worker->task before the worker even starts, so the self-assignment is dead code.

Four drivers were still on the old API. Convert them, then remove the FIXME.

No functional change intended.

Bradley Morgan (5):
media: ivtv: convert to kthread_run_worker
net: encx24j600: convert to kthread_run_worker
tty: sc16is7xx: convert to kthread_run_worker
cpufreq: schedutil: convert to kthread_create_worker
kthread: remove worker->task self-assignment

 drivers/media/pci/ivtv/ivtv-driver.c    | 11 ++++-------
 drivers/media/pci/ivtv/ivtv-driver.h    |  3 +--
 drivers/net/ethernet/microchip/encx24j600.c | 12 ++++--------
 drivers/tty/serial/sc16is7xx.c        | 17 +++++++----------
 kernel/sched/cpufreq_schedutil.c       | 14 +++++--------
 kernel/kthread.c                      |  7 -------
 6 files changed, 21 insertions(+), 63 deletions(-)
author
#1 lore raw

PATCH 1/5 media: ivtv: convert to kthread_run_worker

From: Bradley Morgan <[email protected]>

Embedding a kthread_worker and running kthread_worker_fn yourself is the old way. Convert ivtv to kthread_run_worker(), dropping the separate irq_worker_task pointer.

This is part of removing the worker->task self-assignment in kthread_worker_fn().

Signed-off-by Bradley Morgan <[email protected]>
---
 drivers/media/pci/ivtv/ivtv-driver.c | 14 ++++++--------
 drivers/media/pci/ivtv/ivtv-driver.h |  3 +--
 2 files changed, 7 insertions(+), 10 deletions(-)
drivers/media/pci/ivtv/ivtv-driver.c +6 -8
index b9ea56ec9593..6f0e0e46deb5 100644
@@ -703,15 +703,13 @@ static int ivtv_init_struct1(struct ivtv *itv)
 	spin_lock_init(&itv->lock);
 	spin_lock_init(&itv->dma_reg_lock);
 
-	kthread_init_worker(&itv->irq_worker);
-	itv->irq_worker_task = kthread_run(kthread_worker_fn, &itv->irq_worker,
-					   "%s", itv->v4l2_dev.name);
-	if (IS_ERR(itv->irq_worker_task)) {
+	itv->irq_worker = kthread_run_worker(0, "%s", itv->v4l2_dev.name);
+	if (IS_ERR(itv->irq_worker)) {
 		IVTV_ERR("Could not create ivtv task\n");
 		return -1;
 	}
 	/* must use the FIFO scheduler as it is realtime sensitive */
-	sched_set_fifo(itv->irq_worker_task);
+	sched_set_fifo(itv->irq_worker->task);
 
 	kthread_init_work(&itv->irq_work, ivtv_irq_work_handler);
 

@@ -1232,7 +1230,7 @@ static int ivtv_probe(struct pci_dev *pdev, const struct pci_device_id *pci_id)
 	v4l2_ctrl_handler_free(&itv->cxhdl.hdl);
 	exit_ivtv_i2c(itv);
 free_worker:
-	kthread_stop(itv->irq_worker_task);
+	kthread_destroy_worker(itv->irq_worker);
 err:
 	if (retval == 0)
 		retval = -ENODEV;

@@ -1372,8 +1370,8 @@ static void ivtv_remove(struct pci_dev *pdev)
 	timer_shutdown_sync(&itv->dma_timer);
 
 	/* Kill irq worker */
-	kthread_flush_worker(&itv->irq_worker);
-	kthread_stop(itv->irq_worker_task);
+	kthread_flush_worker(itv->irq_worker);
+	kthread_destroy_worker(itv->irq_worker);
 
 	ivtv_streams_cleanup(itv);
 	ivtv_udma_free(itv);
drivers/media/pci/ivtv/ivtv-driver.h +1 -2
index 000e8beecc7c..091bc44f13bc 100644
@@ -668,8 +668,7 @@ struct ivtv {
 	/* Interrupts & DMA */
 	u32 irqmask;                    /* active interrupts */
 	u32 irq_rr_idx;                 /* round-robin stream index */
-	struct kthread_worker irq_worker;		/* kthread worker for PIO/YUV/VBI actions */
-	struct task_struct *irq_worker_task;		/* task for irq_worker */
+	struct kthread_worker *irq_worker;		/* kthread worker for PIO/YUV/VBI actions */
 	struct kthread_work irq_work;	/* kthread work entry */
 	spinlock_t dma_reg_lock;        /* lock access to DMA engine registers */
 	int cur_dma_stream;		/* index of current stream doing DMA (-1 if none) */
Signature · 1 line
2.47.3
author
#2 lore raw

PATCH 2/5 net: encx24j600: convert to kthread_run_worker

In reply to Bradley Morgan

From: Bradley Morgan <[email protected]>

Embedding a kthread_worker and running kthread_worker_fn yourself is the old way. Convert encx24j600 to kthread_run_worker(), dropping the separate kworker_task pointer.

This is part of removing the worker->task self-assignment in kthread_worker_fn().

Signed-off-by Bradley Morgan <[email protected]>
---
 drivers/net/ethernet/microchip/encx24j600.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)
drivers/net/ethernet/microchip/encx24j600.c +8 -12
index b011bf5c2305..460189064332 100644
@@ -49,8 +49,7 @@ struct encx24j600_priv {
 	struct mutex              lock; /* device access lock */
 	struct encx24j600_context ctx;
 	struct sk_buff           *tx_skb;
-	struct task_struct       *kworker_task;
-	struct kthread_worker     kworker;
+	struct kthread_worker    *kworker;
 	struct kthread_work       tx_work;
 	struct kthread_work       setrx_work;
 	u16                       next_packet;

@@ -823,7 +822,7 @@ static void encx24j600_set_multicast_list(struct net_device *dev)
 	}
 
 	if (oldfilter != priv->rxfilter)
-		kthread_queue_work(&priv->kworker, &priv->setrx_work);
+		kthread_queue_work(priv->kworker, &priv->setrx_work);
 }
 
 static void encx24j600_hw_tx(struct encx24j600_priv *priv)

@@ -884,7 +883,7 @@ static netdev_tx_t encx24j600_tx(struct sk_buff *skb, struct net_device *dev)
 	/* Remember the skb for deferred processing */
 	priv->tx_skb = skb;
 
-	kthread_queue_work(&priv->kworker, &priv->tx_work);
+	kthread_queue_work(priv->kworker, &priv->tx_work);
 
 	return NETDEV_TX_OK;
 }

@@ -1046,15 +1045,12 @@ static int encx24j600_spi_probe(struct spi_device *spi)
 	/* Initialize the device HW to the consistent state */
 	encx24j600_hw_init(priv);
 
-	kthread_init_worker(&priv->kworker);
 	kthread_init_work(&priv->tx_work, encx24j600_tx_proc);
 	kthread_init_work(&priv->setrx_work, encx24j600_setrx_proc);
 
-	priv->kworker_task = kthread_run(kthread_worker_fn, &priv->kworker,
-					 "encx24j600");
-
-	if (IS_ERR(priv->kworker_task)) {
-		ret = PTR_ERR(priv->kworker_task);
+	priv->kworker = kthread_run_worker(0, "encx24j600");
+	if (IS_ERR(priv->kworker)) {
+		ret = PTR_ERR(priv->kworker);
 		goto out_free;
 	}
 

@@ -1087,7 +1083,7 @@ static int encx24j600_spi_probe(struct spi_device *spi)
 out_unregister:
 	unregister_netdev(priv->ndev);
 out_stop:
-	kthread_stop(priv->kworker_task);
+	kthread_destroy_worker(priv->kworker);
 out_free:
 	free_netdev(ndev);
 

@@ -1100,7 +1096,7 @@ static void encx24j600_spi_remove(struct spi_device *spi)
 	struct encx24j600_priv *priv = dev_get_drvdata(&spi->dev);
 
 	unregister_netdev(priv->ndev);
-	kthread_stop(priv->kworker_task);
+	kthread_destroy_worker(priv->kworker);
 
 	free_netdev(priv->ndev);
 }
Signature · 1 line
2.47.3
author
#3 lore raw

PATCH 3/5 tty: sc16is7xx: convert to kthread_run_worker

In reply to Bradley Morgan

From: Bradley Morgan <[email protected]>

Embedding a kthread_worker and running kthread_worker_fn yourself is the old way. Convert sc16is7xx to kthread_run_worker(), dropping the separate kworker_task pointer.

This is part of removing the worker->task self-assignment in kthread_worker_fn().

Signed-off-by Bradley Morgan <[email protected]>
---
 drivers/tty/serial/sc16is7xx.c | 39 ++++++++++++++++------------------
 1 file changed, 18 insertions(+), 21 deletions(-)
drivers/tty/serial/sc16is7xx.c +18 -21
index 7107a0fb1e7b..2cfc1f290009 100644
@@ -306,8 +306,7 @@ struct sc16is7xx_port {
 	unsigned long			gpio_valid_mask;
 #endif
 	u8				mctrl_mask;
-	struct kthread_worker		kworker;
-	struct task_struct		*kworker_task;
+	struct kthread_worker		*kworker;
 	struct kthread_delayed_work	poll_work;
 	bool				polling;
 	struct sc16is7xx_one		p[];

@@ -434,7 +433,7 @@ static void sc16is7xx_ier_clear(struct uart_port *port, u8 bit)
 	one->config.flags |= SC16IS7XX_RECONF_IER;
 	one->config.ier_mask |= bit;
 	one->config.ier_val &= ~bit;
-	kthread_queue_work(&s->kworker, &one->reg_work);
+	kthread_queue_work(s->kworker, &one->reg_work);
 }
 
 static void sc16is7xx_ier_set(struct uart_port *port, u8 bit)

@@ -447,7 +446,7 @@ static void sc16is7xx_ier_set(struct uart_port *port, u8 bit)
 	one->config.flags |= SC16IS7XX_RECONF_IER;
 	one->config.ier_mask |= bit;
 	one->config.ier_val |= bit;
-	kthread_queue_work(&s->kworker, &one->reg_work);
+	kthread_queue_work(s->kworker, &one->reg_work);
 }
 
 static void sc16is7xx_stop_tx(struct uart_port *port)

@@ -813,7 +812,7 @@ static void sc16is7xx_poll_proc(struct kthread_work *ws)
 	sc16is7xx_irq(0, s);
 
 	/* Setup delay based on SC16IS7XX_POLL_PERIOD_MS */
-	kthread_queue_delayed_work(&s->kworker, &s->poll_work,
+	kthread_queue_delayed_work(s->kworker, &s->poll_work,
 				   msecs_to_jiffies(SC16IS7XX_POLL_PERIOD_MS));
 }
 

@@ -900,7 +899,7 @@ static void sc16is7xx_ms_proc(struct kthread_work *ws)
 		scoped_guard(mutex, &one->lock)
 			sc16is7xx_update_mlines(one);
 
-		kthread_queue_delayed_work(&s->kworker, &one->ms_work, HZ);
+		kthread_queue_delayed_work(s->kworker, &one->ms_work, HZ);
 	}
 }
 

@@ -911,7 +910,7 @@ static void sc16is7xx_enable_ms(struct uart_port *port)
 
 	lockdep_assert_held_once(&port->lock);
 
-	kthread_queue_delayed_work(&s->kworker, &one->ms_work, 0);
+	kthread_queue_delayed_work(s->kworker, &one->ms_work, 0);
 }
 
 static void sc16is7xx_start_tx(struct uart_port *port)

@@ -919,7 +918,7 @@ static void sc16is7xx_start_tx(struct uart_port *port)
 	struct sc16is7xx_port *s = dev_get_drvdata(port->dev);
 	struct sc16is7xx_one *one = to_sc16is7xx_one(port);
 
-	kthread_queue_work(&s->kworker, &one->tx_work);
+	kthread_queue_work(s->kworker, &one->tx_work);
 }
 
 static void sc16is7xx_throttle(struct uart_port *port)

@@ -968,7 +967,7 @@ static void sc16is7xx_set_mctrl(struct uart_port *port, unsigned int mctrl)
 	struct sc16is7xx_one *one = to_sc16is7xx_one(port);
 
 	one->config.flags |= SC16IS7XX_RECONF_MD;
-	kthread_queue_work(&s->kworker, &one->reg_work);
+	kthread_queue_work(s->kworker, &one->reg_work);
 }
 
 static void sc16is7xx_break_ctl(struct uart_port *port, int break_state)

@@ -1098,7 +1097,7 @@ static int sc16is7xx_config_rs485(struct uart_port *port, struct ktermios *termi
 	}
 
 	one->config.flags |= SC16IS7XX_RECONF_RS485;
-	kthread_queue_work(&s->kworker, &one->reg_work);
+	kthread_queue_work(s->kworker, &one->reg_work);
 
 	return 0;
 }

@@ -1159,7 +1158,7 @@ static int sc16is7xx_startup(struct uart_port *port)
 	uart_port_unlock_irqrestore(port, flags);
 
 	if (s->polling)
-		kthread_queue_delayed_work(&s->kworker, &s->poll_work,
+		kthread_queue_delayed_work(s->kworker, &s->poll_work,
 					   msecs_to_jiffies(SC16IS7XX_POLL_PERIOD_MS));
 
 	return 0;

@@ -1186,7 +1185,7 @@ static void sc16is7xx_shutdown(struct uart_port *port)
 	if (s->polling)
 		kthread_cancel_delayed_work_sync(&s->poll_work);
 
-	kthread_flush_worker(&s->kworker);
+	kthread_flush_worker(s->kworker);
 }
 
 static const char *sc16is7xx_type(struct uart_port *port)

@@ -1595,14 +1594,12 @@ int sc16is7xx_probe(struct device *dev, const struct sc16is7xx_devtype *devtype,
 	s->devtype = devtype;
 	dev_set_drvdata(dev, s);
 
-	kthread_init_worker(&s->kworker);
-	s->kworker_task = kthread_run(kthread_worker_fn, &s->kworker,
-				      "sc16is7xx");
-	if (IS_ERR(s->kworker_task)) {
-		ret = PTR_ERR(s->kworker_task);
+	s->kworker = kthread_run_worker(0, "sc16is7xx");
+	if (IS_ERR(s->kworker)) {
+		ret = PTR_ERR(s->kworker);
 		goto out_clk;
 	}
-	sched_set_fifo(s->kworker_task);
+	sched_set_fifo(s->kworker->task);
 
 	ret = sc16is7xx_reset(dev, regmaps[0]);
 	if (ret)

@@ -1676,7 +1673,7 @@ int sc16is7xx_probe(struct device *dev, const struct sc16is7xx_devtype *devtype,
 	}
 
 out_kthread:
-	kthread_stop(s->kworker_task);
+	kthread_destroy_worker(s->kworker);
 
 out_clk:
 	clk_disable_unprepare(s->clk);

@@ -1705,8 +1702,8 @@ void sc16is7xx_remove(struct device *dev)
 	if (s->polling)
 		kthread_cancel_delayed_work_sync(&s->poll_work);
 
-	kthread_flush_worker(&s->kworker);
-	kthread_stop(s->kworker_task);
+	kthread_flush_worker(s->kworker);
+	kthread_destroy_worker(s->kworker);
 
 	clk_disable_unprepare(s->clk);
 }
Signature · 1 line
2.47.3
author
#4 lore raw

PATCH 4/5 cpufreq: schedutil: convert to kthread_create_worker

In reply to Bradley Morgan

From: Bradley Morgan <[email protected]>

Embedding a kthread_worker and running kthread_worker_fn yourself is the old way. Convert cpufreq_schedutil to kthread_create_worker(), dropping the separate thread pointer.

This is part of removing the worker->task self-assignment in kthread_worker_fn().

Signed-off-by Bradley Morgan <[email protected]>
---
 kernel/sched/cpufreq_schedutil.c | 33 ++++++++++++++------------------
 1 file changed, 14 insertions(+), 19 deletions(-)
kernel/sched/cpufreq_schedutil.c +14 -19
index 49ccd6f1c185..5023c24d8a21 100644
@@ -31,8 +31,7 @@ struct sugov_policy {
 	struct			irq_work irq_work;
 	struct			kthread_work work;
 	struct			mutex work_lock;
-	struct			kthread_worker worker;
-	struct task_struct	*thread;
+	struct			kthread_worker *worker;
 	bool			work_in_progress;
 
 	bool			limits_changed;

@@ -586,7 +585,7 @@ static void sugov_irq_work(struct irq_work *irq_work)
 
 	sg_policy = container_of(irq_work, struct sugov_policy, irq_work);
 
-	kthread_queue_work(&sg_policy->worker, &sg_policy->work);
+	kthread_queue_work(sg_policy->worker, &sg_policy->work);
 }
 
 /************************** sysfs interface ************************/

@@ -669,7 +668,6 @@ static void sugov_policy_free(struct sugov_policy *sg_policy)
 
 static int sugov_kthread_create(struct sugov_policy *sg_policy)
 {
-	struct task_struct *thread;
 	struct sched_attr attr = {
 		.size		= sizeof(struct sched_attr),
 		.sched_policy	= SCHED_DEADLINE,

@@ -692,32 +690,29 @@ static int sugov_kthread_create(struct sugov_policy *sg_policy)
 		return 0;
 
 	kthread_init_work(&sg_policy->work, sugov_work);
-	kthread_init_worker(&sg_policy->worker);
-	thread = kthread_create(kthread_worker_fn, &sg_policy->worker,
-				"sugov:%d",
-				cpumask_first(policy->related_cpus));
-	if (IS_ERR(thread)) {
-		pr_err("failed to create sugov thread: %pe\n", thread);
-		return PTR_ERR(thread);
+	sg_policy->worker = kthread_create_worker(0, "sugov:%d",
+						cpumask_first(policy->related_cpus));
+	if (IS_ERR(sg_policy->worker)) {
+		pr_err("failed to create sugov thread: %pe\n", sg_policy->worker);
+		return PTR_ERR(sg_policy->worker);
 	}
 
-	ret = sched_setattr_nocheck(thread, &attr);
+	ret = sched_setattr_nocheck(sg_policy->worker->task, &attr);
 	if (ret) {
-		kthread_stop(thread);
+		kthread_destroy_worker(sg_policy->worker);
 		pr_warn("%s: failed to set SCHED_DEADLINE\n", __func__);
 		return ret;
 	}
 
-	sg_policy->thread = thread;
 	if (policy->dvfs_possible_from_any_cpu)
-		set_cpus_allowed_ptr(thread, policy->related_cpus);
+		set_cpus_allowed_ptr(sg_policy->worker->task, policy->related_cpus);
 	else
-		kthread_bind_mask(thread, policy->related_cpus);
+		kthread_bind_mask(sg_policy->worker->task, policy->related_cpus);
 
 	init_irq_work(&sg_policy->irq_work, sugov_irq_work);
 	mutex_init(&sg_policy->work_lock);
 
-	wake_up_process(thread);
+	wake_up_process(sg_policy->worker->task);
 
 	return 0;
 }

@@ -728,8 +723,8 @@ static void sugov_kthread_stop(struct sugov_policy *sg_policy)
 	if (sg_policy->policy->fast_switch_enabled)
 		return;
 
-	kthread_flush_worker(&sg_policy->worker);
-	kthread_stop(sg_policy->thread);
+	kthread_flush_worker(sg_policy->worker);
+	kthread_destroy_worker(sg_policy->worker);
 	mutex_destroy(&sg_policy->work_lock);
 }
Signature · 1 line
2.47.3
author
#5 lore raw

PATCH 1/1 kthread: remove worker->task self-assignment

In reply to Bradley Morgan

From: Bradley Morgan <[email protected]>

All kthread worker users now create their workers with kthread_create_worker*(), which sets worker->task before the worker starts. The self-assignment in kthread_worker_fn() is dead code now.

Remove it.

Signed-off-by Bradley Morgan <[email protected]>
---
 kernel/kthread.c | 7 -------
 1 file changed, 7 deletions(-)
kernel/kthread.c +0 -7
index 63beb59b7a3d..18bc702e8d2d 100644
@@ -987,13 +987,6 @@ int kthread_worker_fn(void *worker_ptr)
 	struct kthread_worker *worker = worker_ptr;
 	struct kthread_work *work;
 
-	/*
-	 * FIXME: Update the check and remove the assignment when all kthread
-	 * worker users are created using kthread_create_worker*() functions.
-	 */
-	WARN_ON(worker->task && worker->task != current);
-	worker->task = current;
-
 	if (worker->flags & KTW_FREEZABLE)
 		set_freezable();
Signature · 1 line
2.47.3
#6 lore raw

Hi,

Quoted 5 lines · Bradley Morgan

From: Bradley Morgan <[email protected]>

All kthread worker users now create their workers with kthread_create_worker*(), which sets worker->task before the worker starts. The self-assignment in kthread_worker_fn() is dead code now.

How could it be created before?

Quoted 13 lines

Remove it.

Signed-off-by Bradley Morgan <[email protected]>
---
 kernel/kthread.c | 7 -------
 1 file changed, 7 deletions(-)
kernel/kthread.c +0 -0
index 63beb59b7a3d..18bc702e8d2d 100644

kernel/kthread.c int kthread_worker_fn(void *worker_ptr) · line 994

Quoted 8 lines
kernel/kthread.c +0 -5
@@ -987,13 +987,6 @@ int kthread_worker_fn(void *worker_ptr)
 	struct kthread_worker *worker = worker_ptr;
 	struct kthread_work *work;
 
-	/*
-	 * FIXME: Update the check and remove the assignment when all kthread
-	 * worker users are created using kthread_create_worker*() functions.
-	 */
-	WARN_ON(worker->task && worker->task != current);

Should we keep a WARN_ON(worker->task != current) here?

Thanks.

Quoted 7 lines
kernel/kthread.c +0 -2
-	worker->task = current;
-
 	if (worker->flags & KTW_FREEZABLE)
 		set_freezable();
Signature · 1 line
2.47.3
Signature · 2 lines
Frederic Weisbecker
SUSE Labs
maintainer
#7 lore raw
Quoted 27 lines · Frederic Weisbecker

Hi,

Quoted 5 lines · Bradley Morgan

From: Bradley Morgan <[email protected]>

All kthread worker users now create their workers with kthread_create_worker*(), which sets worker->task before the worker starts. The self-assignment in kthread_worker_fn() is dead code now.

How could it be created before?

Quoted 16 lines

Remove it.

Signed-off-by Bradley Morgan <[email protected]>
---
 kernel/kthread.c | 7 -------
 1 file changed, 7 deletions(-)
kernel/kthread.c +0 -0
index 63beb59b7a3d..18bc702e8d2d 100644
@@ -987,13 +987,6 @@ int kthread_worker_fn(void *worker_ptr)
 	struct kthread_worker *worker = worker_ptr;
 	struct kthread_work *work;
Quoted 7 lines

kernel/kthread.c int kthread_worker_fn(void *worker_ptr) · line 994

Quoted 5 lines
kernel/kthread.c +0 -5
-	/*
-	 * FIXME: Update the check and remove the assignment when all kthread
-	 * worker users are created using kthread_create_worker*() functions.
-	 */
-	WARN_ON(worker->task && worker->task != current);

Should we keep a WARN_ON(worker->task != current) here?

I vote yes! Paranoia is good :-)

#8 lore raw
Quoted 36 lines · Peter Zijlstra
Quoted 27 lines · Frederic Weisbecker

Hi,

Quoted 5 lines · Bradley Morgan

From: Bradley Morgan <[email protected]>

All kthread worker users now create their workers with kthread_create_worker*(), which sets worker->task before the worker starts. The self-assignment in kthread_worker_fn() is dead code now.

How could it be created before?

Quoted 16 lines

Remove it.

Signed-off-by Bradley Morgan <[email protected]>
---
 kernel/kthread.c | 7 -------
 1 file changed, 7 deletions(-)
kernel/kthread.c +0 -0
index 63beb59b7a3d..18bc702e8d2d 100644
@@ -987,13 +987,6 @@ int kthread_worker_fn(void *worker_ptr)
 	struct kthread_worker *worker = worker_ptr;
 	struct kthread_work *work;
Quoted 7 lines

kernel/kthread.c int kthread_worker_fn(void *worker_ptr) · line 994

Quoted 5 lines
kernel/kthread.c +0 -5
-	/*
-	 * FIXME: Update the check and remove the assignment when all kthread
-	 * worker users are created using kthread_create_worker*() functions.
-	 */
-	WARN_ON(worker->task && worker->task != current);

Should we keep a WARN_ON(worker->task != current) here?

Quoted 2 lines

I vote yes! Paranoia is good :-)

Yeah, but it *should* be useless. Do you really want me to readd the warn statement? Maybe add some text, like

"worker->task is deprecated"?

--- Thanks!
https://lore.kernel.org/all/[email protected]/

#9 lore raw
Quoted 38 lines · Bradley Morgan
Quoted 36 lines · Peter Zijlstra
Quoted 34 lines · Frederic Weisbecker

Hi,

Quoted 5 lines · Bradley Morgan

From: Bradley Morgan <[email protected]>

All kthread worker users now create their workers with kthread_create_worker*(), which sets worker->task before the worker starts. The self-assignment in kthread_worker_fn() is dead code now.

How could it be created before?

Quoted 13 lines

Remove it.

Signed-off-by Bradley Morgan <[email protected]>
---
 kernel/kthread.c | 7 -------
 1 file changed, 7 deletions(-)
kernel/kthread.c +0 -0
index 63beb59b7a3d..18bc702e8d2d 100644

kernel/kthread.c int kthread_worker_fn(void *worker_ptr) · line 994

Quoted 8 lines
kernel/kthread.c +0 -5
@@ -987,13 +987,6 @@ int kthread_worker_fn(void *worker_ptr)
 	struct kthread_worker *worker = worker_ptr;
 	struct kthread_work *work;
 
-	/*
-	 * FIXME: Update the check and remove the assignment when all kthread
-	 * worker users are created using kthread_create_worker*() functions.
-	 */
-	WARN_ON(worker->task && worker->task != current);

Should we keep a WARN_ON(worker->task != current) here?

Quoted 8 lines
Quoted 2 lines

I vote yes! Paranoia is good :-)

Yeah, but it *should* be useless. Do you really want me to readd the warn statement? Maybe add some text, like

"worker->task is deprecated"?

Btw this is a "something around the lines of", so it's definitely not gonna be this! (Its sorta wrong too)

Quoted 3 lines

--- Thanks!
https://lore.kernel.org/all/[email protected]/

--- Thanks!
https://lore.kernel.org/all/[email protected]/

#10 lore raw
In reply to Bradley Morgan
Quoted 38 lines · Bradley Morgan
Quoted 36 lines · Peter Zijlstra
Quoted 34 lines · Frederic Weisbecker

Hi,

Quoted 5 lines · Bradley Morgan

From: Bradley Morgan <[email protected]>

All kthread worker users now create their workers with kthread_create_worker*(), which sets worker->task before the worker starts. The self-assignment in kthread_worker_fn() is dead code now.

How could it be created before?

Quoted 13 lines

Remove it.

Signed-off-by Bradley Morgan <[email protected]>
---
 kernel/kthread.c | 7 -------
 1 file changed, 7 deletions(-)
kernel/kthread.c +0 -0
index 63beb59b7a3d..18bc702e8d2d 100644

kernel/kthread.c int kthread_worker_fn(void *worker_ptr) · line 994

Quoted 8 lines
kernel/kthread.c +0 -5
@@ -987,13 +987,6 @@ int kthread_worker_fn(void *worker_ptr)
 	struct kthread_worker *worker = worker_ptr;
 	struct kthread_work *work;
 
-	/*
-	 * FIXME: Update the check and remove the assignment when all kthread
-	 * worker users are created using kthread_create_worker*() functions.
-	 */
-	WARN_ON(worker->task && worker->task != current);

Should we keep a WARN_ON(worker->task != current) here?

Quoted 5 lines
Quoted 2 lines

I vote yes! Paranoia is good :-)

Yeah, but it *should* be useless.

Famous last words :-)

Quoted 4 lines

Do you really want me to readd the warn
statement? Maybe add some text, like

"worker->task is deprecated"?

Is it deprecated? We still need that field for wakeup on queue at least. Don't bother with a message, it's only a sanity check.

Thanks.

Quoted 3 lines

--- Thanks!
https://lore.kernel.org/all/[email protected]/

Signature · 2 lines
Frederic Weisbecker
SUSE Labs
#11 lore raw
Quoted 40 lines · Frederic Weisbecker
Quoted 11 lines · Bradley Morgan
Quoted 9 lines · Peter Zijlstra
Quoted 8 lines · Frederic Weisbecker

Hi,

Quoted 5 lines · Bradley Morgan

From: Bradley Morgan <[email protected]>

All kthread worker users now create their workers with kthread_create_worker*(), which sets worker->task before the worker starts. The self-assignment in kthread_worker_fn() is dead code

now.

Quoted 27 lines
Quoted 27 lines
Quoted 26 lines

How could it be created before?

Quoted 13 lines

Remove it.

Signed-off-by Bradley Morgan <[email protected]>
---
 kernel/kthread.c | 7 -------
 1 file changed, 7 deletions(-)
kernel/kthread.c +0 -0
index 63beb59b7a3d..18bc702e8d2d 100644

kernel/kthread.c int kthread_worker_fn(void *worker_ptr) · line 994

Quoted 8 lines
kernel/kthread.c +0 -5
@@ -987,13 +987,6 @@ int kthread_worker_fn(void *worker_ptr)
 	struct kthread_worker *worker = worker_ptr;
 	struct kthread_work *work;
 
-	/*
-	 * FIXME: Update the check and remove the assignment when all kthread
-	 * worker users are created using kthread_create_worker*() functions.
-	 */
-	WARN_ON(worker->task && worker->task != current);

Should we keep a WARN_ON(worker->task != current) here?

Quoted 8 lines
Quoted 5 lines
Quoted 2 lines

I vote yes! Paranoia is good :-)

Yeah, but it *should* be useless.

Famous last words :-)

Hahahaha

Quoted 7 lines
Quoted 4 lines

Do you really want me to readd the warn
statement? Maybe add some text, like

"worker->task is deprecated"?

Is it deprecated? We still need that field for wakeup on queue at least. Don't bother with a message, it's only a sanity check.

Okie, I did convert all the drivers as you can see, if you wanna give it a lookie

Quoted 9 lines

Thanks.

Quoted 3 lines

--- Thanks!

https://lore.kernel.org/all/[email protected]/

--- Thanks!
https://lore.kernel.org/all/[email protected]/

#12 lore raw
Quoted 56 lines · Bradley Morgan
Quoted 40 lines · Frederic Weisbecker
Quoted 11 lines · Bradley Morgan
Quoted 9 lines · Peter Zijlstra
Quoted 8 lines · Frederic Weisbecker

Hi,

Quoted 5 lines · Bradley Morgan

From: Bradley Morgan <[email protected]>

All kthread worker users now create their workers with kthread_create_worker*(), which sets worker->task before the worker starts. The self-assignment in kthread_worker_fn() is dead code

now.

Quoted 27 lines
Quoted 27 lines
Quoted 26 lines

How could it be created before?

Quoted 13 lines

Remove it.

Signed-off-by Bradley Morgan <[email protected]>
---
 kernel/kthread.c | 7 -------
 1 file changed, 7 deletions(-)
kernel/kthread.c +0 -0
index 63beb59b7a3d..18bc702e8d2d 100644

kernel/kthread.c int kthread_worker_fn(void *worker_ptr) · line 994

Quoted 8 lines
kernel/kthread.c +0 -5
@@ -987,13 +987,6 @@ int kthread_worker_fn(void *worker_ptr)
 	struct kthread_worker *worker = worker_ptr;
 	struct kthread_work *work;
 
-	/*
-	 * FIXME: Update the check and remove the assignment when all kthread
-	 * worker users are created using kthread_create_worker*() functions.
-	 */
-	WARN_ON(worker->task && worker->task != current);

Should we keep a WARN_ON(worker->task != current) here?

Quoted 8 lines
Quoted 5 lines
Quoted 2 lines

I vote yes! Paranoia is good :-)

Yeah, but it *should* be useless.

Famous last words :-)

Hahahaha

Quoted 3 lines
Quoted 3 lines

Do you really want me to readd the warn
statement? Maybe add some text, like

Quoted 7 lines
Quoted 4 lines
Quoted 1 line

"worker->task is deprecated"?

Is it deprecated? We still need that field for wakeup on queue at least. Don't bother with a message, it's only a sanity check.

Okie, I did convert all the drivers as you can see, if you wanna give it a lookie

Do you have a link to that?

Thanks.

Quoted 13 lines
Quoted 10 lines

Thanks.

Quoted 3 lines

--- Thanks!

https://lore.kernel.org/all/[email protected]/

--- Thanks!
https://lore.kernel.org/all/[email protected]/

Signature · 2 lines
Frederic Weisbecker
SUSE Labs
#13 lore raw
Quoted 66 lines · Frederic Weisbecker
Quoted 1 line · Bradley Morgan

On 1 September 2026 15:06:51 BST, Frederic Weisbecker

<[email protected]>

Quoted 3 lines
Quoted 2 lines · wrote:
Quoted 1 line · Bradley Morgan

On 1 September 2026 13:49:00 BST, Peter Zijlstra

<[email protected]>

Quoted 9 lines
Quoted 9 lines
Quoted 9 lines
Quoted 8 lines · wrote:
Quoted 7 lines · Frederic Weisbecker

Hi,

Quoted 4 lines · Bradley Morgan

From: Bradley Morgan <[email protected]>

All kthread worker users now create their workers with kthread_create_worker*(), which sets worker->task before the

worker

Quoted 33 lines
Quoted 33 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

starts. The self-assignment in kthread_worker_fn() is dead code

now.

Quoted 31 lines
Quoted 29 lines
Quoted 19 lines

How could it be created before?

Quoted 16 lines

Remove it.

Signed-off-by Bradley Morgan <[email protected]>
---
 kernel/kthread.c | 7 -------
 1 file changed, 7 deletions(-)
kernel/kthread.c +0 -0
index 63beb59b7a3d..18bc702e8d2d 100644
@@ -987,13 +987,6 @@ int kthread_worker_fn(void *worker_ptr)
 	struct kthread_worker *worker = worker_ptr;
 	struct kthread_work *work;
Quoted 7 lines

kernel/kthread.c int kthread_worker_fn(void *worker_ptr) · line 994

Quoted 5 lines
kernel/kthread.c +0 -5
-	/*
-	 * FIXME: Update the check and remove the assignment when all kthread
-	 * worker users are created using kthread_create_worker*() functions.
-	 */
-	WARN_ON(worker->task && worker->task != current);

Should we keep a WARN_ON(worker->task != current) here?

I vote yes! Paranoia is good :-)

Quoted 13 lines
Quoted 4 lines
Quoted 1 line

Yeah, but it *should* be useless.

Famous last words :-)

Hahahaha

Quoted 6 lines
Quoted 4 lines

Do you really want me to readd the warn
statement? Maybe add some text, like

"worker->task is deprecated"?

Is it deprecated? We still need that field for wakeup on queue at

least.

Quoted 2 lines
Quoted 1 line

Don't bother with a message, it's only a sanity check.

Quoted 7 lines
Quoted 1 line

Okie, I did convert all the drivers as you can see, if you wanna give it

a

Quoted 1 line

lookie

Do you have a link to that?

Thanks.

uhh, they should be all in-reply-tos, so if you search for my series in lore, this patch replies to all of the conversions

Quoted 9 lines
Quoted 9 lines
Quoted 8 lines

Thanks.

Quoted 3 lines

--- Thanks!

Quoted 9 lines
Quoted 6 lines

https://lore.kernel.org/all/[email protected]/

Quoted 2 lines

--- Thanks!

https://lore.kernel.org/all/[email protected]/

--- Thanks!
https://lore.kernel.org/all/[email protected]/

#14 lore raw
In reply to Frederic Weisbecker
Quoted 51 lines · Frederic Weisbecker
Quoted 11 lines · Bradley Morgan
Quoted 9 lines · Peter Zijlstra
Quoted 8 lines · Frederic Weisbecker

Hi,

Quoted 5 lines · Bradley Morgan

From: Bradley Morgan <[email protected]>

All kthread worker users now create their workers with kthread_create_worker*(), which sets worker->task before the worker starts. The self-assignment in kthread_worker_fn() is dead code

now.

Quoted 27 lines
Quoted 27 lines
Quoted 26 lines

How could it be created before?

Quoted 13 lines

Remove it.

Signed-off-by Bradley Morgan <[email protected]>
---
 kernel/kthread.c | 7 -------
 1 file changed, 7 deletions(-)
kernel/kthread.c +0 -0
index 63beb59b7a3d..18bc702e8d2d 100644

kernel/kthread.c int kthread_worker_fn(void *worker_ptr) · line 994

Quoted 8 lines
kernel/kthread.c +0 -5
@@ -987,13 +987,6 @@ int kthread_worker_fn(void *worker_ptr)
 	struct kthread_worker *worker = worker_ptr;
 	struct kthread_work *work;
 
-	/*
-	 * FIXME: Update the check and remove the assignment when all kthread
-	 * worker users are created using kthread_create_worker*() functions.
-	 */
-	WARN_ON(worker->task && worker->task != current);

Should we keep a WARN_ON(worker->task != current) here?

Quoted 5 lines
Quoted 2 lines

I vote yes! Paranoia is good :-)

Yeah, but it *should* be useless.

Famous last words :-)

Quoted 3 lines

Do you really want me to readd the warn
statement? Maybe add some text, like

Quoted 6 lines
Quoted 1 line

"worker->task is deprecated"?

Is it deprecated? We still need that field for wakeup on queue at least. Don't bother with a message, it's only a sanity check.

Thanks.

Hi, I hopped on the computer,

Right, so the whole warn on things is good, but I converted all the callers, so it'll never fire.

For OOT users, maybe this'll be beneficial (but yk what their like, theyll remove the warn on)

Guys, what do you think?

Quoted 7 lines
Quoted 4 lines

--- Thanks!

https://lore.kernel.org/all/[email protected]/

--- Thanks!
https://lore.kernel.org/all/[email protected]/

#15 lore raw
Quoted 62 lines · Bradley Morgan
Quoted 51 lines · Frederic Weisbecker
Quoted 11 lines · Bradley Morgan
Quoted 9 lines · Peter Zijlstra
Quoted 8 lines · Frederic Weisbecker

Hi,

Quoted 5 lines · Bradley Morgan

From: Bradley Morgan <[email protected]>

All kthread worker users now create their workers with kthread_create_worker*(), which sets worker->task before the worker starts. The self-assignment in kthread_worker_fn() is dead code

now.

Quoted 27 lines
Quoted 27 lines
Quoted 26 lines

How could it be created before?

Quoted 13 lines

Remove it.

Signed-off-by Bradley Morgan <[email protected]>
---
 kernel/kthread.c | 7 -------
 1 file changed, 7 deletions(-)
kernel/kthread.c +0 -0
index 63beb59b7a3d..18bc702e8d2d 100644

kernel/kthread.c int kthread_worker_fn(void *worker_ptr) · line 994

Quoted 8 lines
kernel/kthread.c +0 -5
@@ -987,13 +987,6 @@ int kthread_worker_fn(void *worker_ptr)
 	struct kthread_worker *worker = worker_ptr;
 	struct kthread_work *work;
 
-	/*
-	 * FIXME: Update the check and remove the assignment when all kthread
-	 * worker users are created using kthread_create_worker*() functions.
-	 */
-	WARN_ON(worker->task && worker->task != current);

Should we keep a WARN_ON(worker->task != current) here?

Quoted 5 lines
Quoted 2 lines

I vote yes! Paranoia is good :-)

Yeah, but it *should* be useless.

Famous last words :-)

Quoted 3 lines

Do you really want me to readd the warn
statement? Maybe add some text, like

Quoted 6 lines
Quoted 1 line

"worker->task is deprecated"?

Is it deprecated? We still need that field for wakeup on queue at least. Don't bother with a message, it's only a sanity check.

Thanks.

Hi, I hopped on the computer,

Quoted 7 lines

Right, so the whole warn on things is good, but I converted all the callers, so it'll never fire.

For OOT users, maybe this'll be beneficial (but yk what their like, theyll remove the warn on)

Guys, what do you think?

The warning is mostly to prevent from future changes to break.

Thanks.

Quoted 11 lines
Quoted 8 lines
Quoted 5 lines

--- Thanks!

https://lore.kernel.org/all/[email protected]/

--- Thanks!
https://lore.kernel.org/all/[email protected]/

Signature · 2 lines
Frederic Weisbecker
SUSE Labs