[RFC 1/1 linux-next] DRIVERS: replace current->state by set_current_state()

[RFC 1/1 linux-next] DRIVERS: replace current->state by set_current_state()

[Fabian Frederick]

replace remaining direct access to current->state by slower
helper function in drivers branch.
Some of them could be optimized later using __set_current_state().

Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
 drivers/block/swim.c                       |  6 +++---
 drivers/isdn/hardware/mISDN/hfcpci.c       |  2 +-
 drivers/macintosh/via-pmu.c                |  4 ++--
 drivers/media/common/saa7146/saa7146_vbi.c |  4 ++--
 drivers/net/usb/hso.c                      |  2 +-
 drivers/net/wan/cosa.c                     | 12 ++++++------
 drivers/net/wireless/airo.c                |  2 +-
 drivers/tty/serial/serial_core.c           |  2 +-
 8 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/block/swim.c b/drivers/block/swim.c
index b5afd49..e9bb759 100644
--- a/drivers/block/swim.c
+++ b/drivers/block/swim.c
@@ -331,7 +331,7 @@ static inline void swim_motor(struct swim __iomem *base,
 			swim_select(base, RELAX);
 			if (swim_readbit(base, MOTOR_ON))
 				break;
-			current->state = TASK_INTERRUPTIBLE;
+			set_current_state(TASK_INTERRUPTIBLE);
 			schedule_timeout(1);
 		}
 	} else if (action == OFF) {
@@ -350,7 +350,7 @@ static inline void swim_eject(struct swim __iomem *base)
 		swim_select(base, RELAX);
 		if (!swim_readbit(base, DISK_IN))
 			break;
-		current->state = TASK_INTERRUPTIBLE;
+		set_current_state(TASK_INTERRUPTIBLE);
 		schedule_timeout(1);
 	}
 	swim_select(base, RELAX);
@@ -374,7 +374,7 @@ static inline int swim_step(struct swim __iomem *base)
 
 	for (wait = 0; wait < HZ; wait++) {
 
-		current->state = TASK_INTERRUPTIBLE;
+		set_current_state(TASK_INTERRUPTIBLE);
 		schedule_timeout(1);
 
 		swim_select(base, RELAX);
diff --git a/drivers/isdn/hardware/mISDN/hfcpci.c b/drivers/isdn/hardware/mISDN/hfcpci.c
index 3c92780..ff48da6 100644
--- a/drivers/isdn/hardware/mISDN/hfcpci.c
+++ b/drivers/isdn/hardware/mISDN/hfcpci.c
@@ -1755,7 +1755,7 @@ init_card(struct hfc_pci *hc)
 		enable_hwirq(hc);
 		spin_unlock_irqrestore(&hc->lock, flags);
 		/* Timeout 80ms */
-		current->state = TASK_UNINTERRUPTIBLE;
+		set_current_state(TASK_UNINTERRUPTIBLE);
 		schedule_timeout((80 * HZ) / 1000);
 		printk(KERN_INFO "HFC PCI: IRQ %d count %d\n",
 		       hc->irq, hc->irqcnt);
diff --git a/drivers/macintosh/via-pmu.c b/drivers/macintosh/via-pmu.c
index dee88e5..b0ab88e 100644
--- a/drivers/macintosh/via-pmu.c
+++ b/drivers/macintosh/via-pmu.c
@@ -2109,7 +2109,7 @@ pmu_read(struct file *file, char __user *buf,
 
 	spin_lock_irqsave(&pp->lock, flags);
 	add_wait_queue(&pp->wait, &wait);
-	current->state = TASK_INTERRUPTIBLE;
+	set_current_statet(TASK_INTERRUPTIBLE);
 
 	for (;;) {
 		ret = -EAGAIN;
@@ -2138,7 +2138,7 @@ pmu_read(struct file *file, char __user *buf,
 		schedule();
 		spin_lock_irqsave(&pp->lock, flags);
 	}
-	current->state = TASK_RUNNING;
+	set_current_state(TASK_RUNNING);
 	remove_wait_queue(&pp->wait, &wait);
 	spin_unlock_irqrestore(&pp->lock, flags);
 	
diff --git a/drivers/media/common/saa7146/saa7146_vbi.c b/drivers/media/common/saa7146/saa7146_vbi.c
index 1e71e37..cb6e906 100644
--- a/drivers/media/common/saa7146/saa7146_vbi.c
+++ b/drivers/media/common/saa7146/saa7146_vbi.c
@@ -95,7 +95,7 @@ static int vbi_workaround(struct saa7146_dev *dev)
 
 		/* prepare to wait to be woken up by the irq-handler */
 		add_wait_queue(&vv->vbi_wq, &wait);
-		current->state = TASK_INTERRUPTIBLE;
+		set_current_state(TASK_INTERRUPTIBLE);
 
 		/* start rps1 to enable workaround */
 		saa7146_write(dev, RPS_ADDR1, dev->d_rps1.dma_handle);
@@ -106,7 +106,7 @@ static int vbi_workaround(struct saa7146_dev *dev)
 		DEB_VBI("brs bug workaround %d/1\n", i);
 
 		remove_wait_queue(&vv->vbi_wq, &wait);
-		current->state = TASK_RUNNING;
+		set_current_state(TASK_RUNNING);
 
 		/* disable rps1 irqs */
 		SAA7146_IER_DISABLE(dev,MASK_28);
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index 9cdfb3f..855c7c4 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -1594,7 +1594,7 @@ hso_wait_modem_status(struct hso_serial *serial, unsigned long arg)
 		}
 		cprev = cnow;
 	}
-	current->state = TASK_RUNNING;
+	set_current_state(TASK_RUNNING);
 	remove_wait_queue(&tiocmget->waitq, &wait);
 
 	return ret;
diff --git a/drivers/net/wan/cosa.c b/drivers/net/wan/cosa.c
index 83c39e2..605af38 100644
--- a/drivers/net/wan/cosa.c
+++ b/drivers/net/wan/cosa.c
@@ -806,21 +806,21 @@ static ssize_t cosa_read(struct file *file,
 	spin_lock_irqsave(&cosa->lock, flags);
 	add_wait_queue(&chan->rxwaitq, &wait);
 	while (!chan->rx_status) {
-		current->state = TASK_INTERRUPTIBLE;
+		set_current_state(TASK_INTERRUPTIBLE);
 		spin_unlock_irqrestore(&cosa->lock, flags);
 		schedule();
 		spin_lock_irqsave(&cosa->lock, flags);
 		if (signal_pending(current) && chan->rx_status == 0) {
 			chan->rx_status = 1;
 			remove_wait_queue(&chan->rxwaitq, &wait);
-			current->state = TASK_RUNNING;
+			set_current_state(TASK_RUNNING);
 			spin_unlock_irqrestore(&cosa->lock, flags);
 			mutex_unlock(&chan->rlock);
 			return -ERESTARTSYS;
 		}
 	}
 	remove_wait_queue(&chan->rxwaitq, &wait);
-	current->state = TASK_RUNNING;
+	set_current_state(TASK_RUNNING);
 	kbuf = chan->rxdata;
 	count = chan->rxsize;
 	spin_unlock_irqrestore(&cosa->lock, flags);
@@ -890,14 +890,14 @@ static ssize_t cosa_write(struct file *file,
 	spin_lock_irqsave(&cosa->lock, flags);
 	add_wait_queue(&chan->txwaitq, &wait);
 	while (!chan->tx_status) {
-		current->state = TASK_INTERRUPTIBLE;
+		set_current_state(TASK_INTERRUPTIBLE);
 		spin_unlock_irqrestore(&cosa->lock, flags);
 		schedule();
 		spin_lock_irqsave(&cosa->lock, flags);
 		if (signal_pending(current) && chan->tx_status == 0) {
 			chan->tx_status = 1;
 			remove_wait_queue(&chan->txwaitq, &wait);
-			current->state = TASK_RUNNING;
+			set_current_state(TASK_RUNNING);
 			chan->tx_status = 1;
 			spin_unlock_irqrestore(&cosa->lock, flags);
 			up(&chan->wsem);
@@ -905,7 +905,7 @@ static ssize_t cosa_write(struct file *file,
 		}
 	}
 	remove_wait_queue(&chan->txwaitq, &wait);
-	current->state = TASK_RUNNING;
+	set_current_state(TASK_RUNNING);
 	up(&chan->wsem);
 	spin_unlock_irqrestore(&cosa->lock, flags);
 	kfree(kbuf);
diff --git a/drivers/net/wireless/airo.c b/drivers/net/wireless/airo.c
index e71a2ce..6aadbfc 100644
--- a/drivers/net/wireless/airo.c
+++ b/drivers/net/wireless/airo.c
@@ -3111,7 +3111,7 @@ static int airo_thread(void *data) {
 				}
 				break;
 			}
-			current->state = TASK_RUNNING;
+			set_current_state(TASK_RUNNING);
 			remove_wait_queue(&ai->thr_wait, &wait);
 			locked = 1;
 		}
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 6a1055a..410f188 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -1119,7 +1119,7 @@ uart_wait_modem_status(struct uart_state *state, unsigned long arg)
 		cprev = cnow;
 	}
 
-	current->state = TASK_RUNNING;
+	set_current_state(TASK_RUNNING);
 	remove_wait_queue(&port->delta_msr_wait, &wait);
 
 	return ret;
-- 
2.1.0

Re: [RFC 1/1 linux-next] DRIVERS: replace current->state by set_current_state()

[Peter Zijlstra]

On Thu, Feb 19, 2015 at 09:21:51PM +0100, Fabian Frederick wrote:
> replace remaining direct access to current->state by slower
> helper function in drivers branch.
> Some of them could be optimized later using __set_current_state().

This changelog needs help. Going by this you just make the drivers worse
(slower).

The reason for using set_current_state() is not that its slower, its
because its actually correct; as opposed to direct assignments which are
prone to races and therefore buggy.

Also, since all were direct assignments you can immediately use
__set_current_state() for TASK_RUNNING; code will not be broken more by
that and relying on the barrier for TASK_RUNNING is obscure and rare
anyhow.

So sure, the idea is good, but please, write a coherent changelog to go
with it.

Re: [RFC 1/1 linux-next] DRIVERS: replace current->state by set_current_state()

[Fabian Frederick]

> On 20 February 2015 at 09:56 Peter Zijlstra <peterz@infradead.org> wrote:
>
>
> On Thu, Feb 19, 2015 at 09:21:51PM +0100, Fabian Frederick wrote:
> > replace remaining direct access to current->state by slower
> > helper function in drivers branch.
> > Some of them could be optimized later using __set_current_state().
>
> This changelog needs help. Going by this you just make the drivers worse
> (slower).
>
> The reason for using set_current_state() is not that its slower, its
> because its actually correct; as opposed to direct assignments which are
> prone to races and therefore buggy.
>
> Also, since all were direct assignments you can immediately use
> __set_current_state() for TASK_RUNNING; code will not be broken more by
> that and relying on the barrier for TASK_RUNNING is obscure and rare
> anyhow.
>
> So sure, the idea is good, but please, write a coherent changelog to go
> with it.

Thanks a lot for explanation Peter, I meant set_current_state was slower than
__set_current_state. I hope changelog is ok in the patchset.
 
Regards,
Fabian