Re: [PATCH] IDE: Fix Power Management request race on resume

Re: [PATCH] IDE: Fix Power Management request race on resume

[cb-lkml]

I applied this patch to 2.6.0-test5 and still have this problem:

http://marc.theaimsgroup.com/?l=linux-kernel&m=106218353005043&w=2

Charlie

Re: [PATCH] IDE: Fix Power Management request race on resume

[Benjamin Herrenschmidt]

On Wed, 2003-09-10 at 11:04, cb-lkml@fish.zetnet.co.uk wrote:
> I applied this patch to 2.6.0-test5 and still have this problem:
> 
> http://marc.theaimsgroup.com/?l=linux-kernel&m=106218353005043&w=2

There are a couple of other fixes pending though I don't thing
they are related to your problem. Do you have a slave drive on
this channel ? What driver are you using for the host controller ?
Does it have a dma_check() function ?

Ben.

Re: [PATCH] IDE: Fix Power Management request race on resume

[cb-lkml]

On Wed, Sep 10, 2003 at 11:16:15AM +0200, Benjamin Herrenschmidt wrote:
> On Wed, 2003-09-10 at 11:04, cb-lkml@fish.zetnet.co.uk wrote:
> > I applied this patch to 2.6.0-test5 and still have this problem:
> > 
> > http://marc.theaimsgroup.com/?l=linux-kernel&m=106218353005043&w=2
> 
> There are a couple of other fixes pending though I don't thing
> they are related to your problem. Do you have a slave drive on
> this channel ? What driver are you using for the host controller ?
> Does it have a dma_check() function ?

It's a laptop, so there's no slave drive.

The host controller is a PIIX4, using CONFIG_BLK_DEV_PIIX. (Presumably
drivers/ide/pci/piix.c)

$ grep dma_check piix.c 
        hwif->ide_dma_check = &piix_config_drive_xfer_rate;

Charlie

Re: [PATCH] IDE: Fix Power Management request race on resume

[Benjamin Herrenschmidt]

On Wed, 2003-09-10 at 11:25, cb-lkml@fish.zetnet.co.uk wrote:
> On Wed, Sep 10, 2003 at 11:16:15AM +0200, Benjamin Herrenschmidt wrote:
> > On Wed, 2003-09-10 at 11:04, cb-lkml@fish.zetnet.co.uk wrote:
> > > I applied this patch to 2.6.0-test5 and still have this problem:
> > > 
> > > http://marc.theaimsgroup.com/?l=linux-kernel&m=106218353005043&w=2
> > 
> > There are a couple of other fixes pending though I don't thing
> > they are related to your problem. Do you have a slave drive on
> > this channel ? What driver are you using for the host controller ?
> > Does it have a dma_check() function ?
> 
> It's a laptop, so there's no slave drive.

Some laptops have the CD/DVD as a slave
> 
> The host controller is a PIIX4, using CONFIG_BLK_DEV_PIIX. (Presumably
> drivers/ide/pci/piix.c)
> 
> $ grep dma_check piix.c 
>         hwif->ide_dma_check = &piix_config_drive_xfer_rate;

Can you put some printk in there to verify it's properly called on
wakeup and that it actually does something ?

Note that problem may be unrelated to the IDE driver, it could also
be some APIC problem messing with interrupts...

Ben.

[PATCH] IDE: Fix Power Management request race on resume

[Benjamin Herrenschmidt]

Hi Linus !

The current IDE Power Management code I wrote has a race on wakeup
when the master device got resumed, it may take a request. At this
point, a PM resume request to a slave device of the same hwgroup
would clear hwgroup->rq and cause an Oops when the master device
request completes.

This patch fixes it. Due to the context in which PM resume requests
are sent, just not clearing hwgroup->rq for these is enough.

Please apply, this patch was already tested & discussed with Bart.

I also removed a useless debug message in the PM code that was
actually misleading (people though it indicated a problem while it
didn't, it's really useless) and fix a typo in a comment.

Cheers,
Ben.

diff -urN linux-2.5/drivers/ide/ide-io.c linuxppc-2.5-benh/drivers/ide/ide-io.c
--- linux-2.5/drivers/ide/ide-io.c	2003-09-09 20:15:34.000000000 +0200
+++ linuxppc-2.5-benh/drivers/ide/ide-io.c	2003-09-09 20:31:23.000000000 +0200
@@ -928,13 +928,10 @@
 		 * 
 		 * We let requests forced at head of queue with ide-preempt
 		 * though. I hope that doesn't happen too much, hopefully not
-		 * unless the subdriver triggers such a thing in it's own PM
+		 * unless the subdriver triggers such a thing in its own PM
 		 * state machine.
 		 */
 		if (drive->blocked && !blk_pm_request(rq) && !(rq->flags & REQ_PREEMPT)) {
-#ifdef DEBUG_PM
-			printk("%s: a request made it's way while we are power managing...\n", drive->name);
-#endif
 			/* We clear busy, there should be no pending ATA command at this point. */
 			hwgroup->busy = 0;
 			break;
@@ -1417,8 +1414,9 @@
 	}
 
 	spin_lock_irqsave(&ide_lock, flags);
-	if (action == ide_preempt || action == ide_head_wait) {
+	if (action == ide_preempt)
 		hwgroup->rq = NULL;
+	if (action == ide_preempt || action == ide_head_wait) {
 		where = ELEVATOR_INSERT_FRONT;
 		rq->flags |= REQ_PREEMPT;
 	}