Re: [PATCH] I2C update for 2.6.10-rc1

From: Greg KH
Date: Tue Nov 09 2004 - 01:36:13 EST


ChangeSet 1.2014.1.2, 2004/10/22 12:52:13-07:00, ebs@xxxxxxxxxxx

[PATCH] I2C: fix recently introduced race in IBM PPC4xx I2C driver

On Tue, Oct 19, 2004 at 10:21:08PM -0700, Eugene Surovegin wrote:
[snip]
> It looks like this change added race I tried to avoid here.
>
> This code is modeled after __wait_event_interruptible_timeout, where
> "prepare_to_wait" is done _before_ checking completion status. This
> change breaks this, e.g. if IRQ happens right after we check iic->sts,
> but before calling msleep_interruptible(). In this case we'll sleep
> much more than required (seconds instead of microseconds)
>
> Greg, if my analysis is correct, please rollback this change.
>
> Nishanth, I'd be nice if you CC'ed me with this patch, my e-mail is at
> the top of that source file.

Oh, well. I should have used wait_event_interruptible_timeout when I
ported this driver to 2.6.

This patch fixes recently introduced race and also cleans ups some
2.4-ism.


Signed-off-by: Eugene Surovegin <ebs@xxxxxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <greg@xxxxxxxxx>


drivers/i2c/busses/i2c-ibm_iic.c | 16 +++++-----------
1 files changed, 5 insertions(+), 11 deletions(-)


diff -Nru a/drivers/i2c/busses/i2c-ibm_iic.c b/drivers/i2c/busses/i2c-ibm_iic.c
--- a/drivers/i2c/busses/i2c-ibm_iic.c 2004-11-08 18:56:20 -08:00
+++ b/drivers/i2c/busses/i2c-ibm_iic.c 2004-11-08 18:56:20 -08:00
@@ -412,18 +412,12 @@

if (dev->irq >= 0){
/* Interrupt mode */
- wait_queue_t wait;
- init_waitqueue_entry(&wait, current);
-
- add_wait_queue(&dev->wq, &wait);
- if (in_8(&iic->sts) & STS_PT)
- msleep_interruptible(dev->adap.timeout * 1000);
- remove_wait_queue(&dev->wq, &wait);
-
- if (unlikely(signal_pending(current))){
+ ret = wait_event_interruptible_timeout(dev->wq,
+ !(in_8(&iic->sts) & STS_PT), dev->adap.timeout * HZ);
+
+ if (unlikely(ret < 0))
DBG("%d: wait interrupted\n", dev->idx);
- ret = -ERESTARTSYS;
- } else if (unlikely(in_8(&iic->sts) & STS_PT)){
+ else if (unlikely(in_8(&iic->sts) & STS_PT)){
DBG("%d: wait timeout\n", dev->idx);
ret = -ETIMEDOUT;
}

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/