Re: [PATCH] libata: disable_irq() during polling IDENTIFY (take 2)

2007-05-08 Thread Tejun Heo
Alan Cox wrote: On Tue, 08 May 2007 19:30:24 +0800 Albert Lee [EMAIL PROTECTED] wrote: Problem: Kernel got irq 5: nobody cared when using libata + polling IDENTIFY + Promise 20275 adapter + Benq DW1620 drive. Detail message available in bug 8441

Re: [PATCH] libata: disable_irq() during polling IDENTIFY (take 2)

2007-05-08 Thread Tejun Heo
Hello, Alan Cox wrote: Oh well, that's how we do PIO data transfer when not polling anyway. We kick HSM directly from the interrupt handler and do PIO inside the interrupt handler. The change made here is to make polling PIO act the same way. Which is making a horrendous problem worse

Re: [PATCH] libata: disable_irq() during polling IDENTIFY (take 2)

2007-05-08 Thread Alan Cox
Oh well, that's how we do PIO data transfer when not polling anyway. We kick HSM directly from the interrupt handler and do PIO inside the interrupt handler. The change made here is to make polling PIO act the same way. Which is making a horrendous problem worse than it was before. In PIO

Re: [PATCH] libata: disable_irq() during polling IDENTIFY (take 2)

2007-05-08 Thread Alan Cox
One thing I'm worried about is that somehow I'm thinking there are controllers/devices out there which choke if there PIO data transfer is interrupted (timing-wise). Is this something I just imagined up? There are a couple of problem controllers. They use the ata_data_xfer_noirq method for

Re: [PATCH] libata: disable_irq() during polling IDENTIFY (take 2)

2007-05-08 Thread Alan Cox
So, are you suggesting pushing data transfer or the whole HSM to workqueue? I'm all for that. I just thought it didn't make sense to I was thinking data transfer but HSM itself might work too, I'd not even thought of push that much out. Anyways, if that's the plan, as Albert suggested

Re: [PATCH] libata: disable_irq() during polling IDENTIFY (take 2)

2007-05-08 Thread Tejun Heo
Alan Cox wrote: One thing I'm worried about is that somehow I'm thinking there are controllers/devices out there which choke if there PIO data transfer is interrupted (timing-wise). Is this something I just imagined up? There are a couple of problem controllers. They use the

Re: [PATCH] libata: disable_irq() during polling IDENTIFY (take 2)

2007-05-08 Thread Tejun Heo
Alan Cox wrote: So, are you suggesting pushing data transfer or the whole HSM to workqueue? I'm all for that. I just thought it didn't make sense to I was thinking data transfer but HSM itself might work too, I'd not even thought of push that much out. I thought pushing the whole HSM out

Re: [PATCH] libata: disable_irq() during polling IDENTIFY

2007-05-08 Thread Alan Cox
Oh yes, and also some controllers will corrupt their data stream if you do this during a PIO transfer of a block of ATA data. I also believe that to be true, but don't know exactly which hardware has that issue. Some stuff which needs to block local irqs during the transfer anyway (eg

Re: [PATCH] libata: disable_irq() during polling IDENTIFY (take 2)

2007-05-08 Thread Albert Lee
Tejun Heo wrote: I guess it's about time to listen what Jeff thinks. Albert, if Jeff agrees with pushing HSM or data transfer to workqueue, are you interested in doing that? The HSM is your baby after all. :-) Moving HSM task to the workqueue looks good idea. The mouse cursor of my X

Re: [PATCH] libata: disable_irq() during polling IDENTIFY (take 2)

2007-05-08 Thread Jeff Garzik
Tejun Heo wrote: Albert, if Jeff agrees with pushing HSM or data transfer to workqueue, are you interested in doing that? The HSM is your baby after all. :-) It's always been my desire to do PIO from a workqueue if possible, since it is so slow. Jeff - To unsubscribe from this

Re: [PATCH] libata: disable_irq() during polling IDENTIFY

2007-05-07 Thread Tejun Heo
[cc'ing Bartlomiej and Mark, hi] Hello, Albert. Albert Lee wrote: Problem: Kernel got irq 5: nobody cared when using libata + polling IDENTIFY + Promise 20275 adapter + Benq DW1620 drive. Detail message available in bug 8441 (http://bugzilla.kernel.org/show_bug.cgi?id=8441).

Re: [PATCH] libata: disable_irq() during polling IDENTIFY

2007-05-07 Thread Alan Cox
Yeap, this is how IDE deals with polling commands but I'm not sure how it's supposed to work with PCI IRQ sharing. Bartlomiej, can you enlighten me here? Simple answer: Badly. If you've got the IRQ shared it mucks up the behaviour of the other device especially when its doing PIO. - To

Re: [PATCH] libata: disable_irq() during polling IDENTIFY

2007-05-07 Thread Albert Lee
Tejun Heo wrote: [cc'ing Bartlomiej and Mark, hi] Hello, Albert. Albert Lee wrote: Problem: Kernel got irq 5: nobody cared when using libata + polling IDENTIFY + Promise 20275 adapter + Benq DW1620 drive. Detail message available in bug 8441

Re: [PATCH] libata: disable_irq() during polling IDENTIFY

2007-05-07 Thread Tejun Heo
Hello, Albert. Albert Lee wrote: Tejun Heo wrote: Also, this is a problem for not only IDENTIFY but all polling commands. Yes, other command might also assert INTRQ during polling. However, for the specific BENQ DW1620 drive, only IDENTIFY_PACKET_DEVICE has such behavior; other commands

Re: [PATCH] libata: disable_irq() during polling IDENTIFY

2007-05-07 Thread Tejun Heo
Alan Cox wrote: Yeap, this is how IDE deals with polling commands but I'm not sure how it's supposed to work with PCI IRQ sharing. Bartlomiej, can you enlighten me here? Simple answer: Badly. If you've got the IRQ shared it mucks up the behaviour of the other device especially when its

Re: [PATCH] libata: disable_irq() during polling IDENTIFY

2007-05-07 Thread Albert Lee
Tejun Heo wrote: Hello, Albert. Albert Lee wrote: Tejun Heo wrote: Also, this is a problem for not only IDENTIFY but all polling commands. Yes, other command might also assert INTRQ during polling. However, for the specific BENQ DW1620 drive, only IDENTIFY_PACKET_DEVICE has such behavior;

Re: [PATCH] libata: disable_irq() during polling IDENTIFY

2007-05-07 Thread Tejun Heo
Albert Lee wrote: Good to know it works. With or without nIEN, I think this change is a good thing to have. IRQ handler shouldn't interfere with polling as both acquire lock during operation. This reminds me of a possible issue with the patch: Previously the polling code assumes that the

Re: [PATCH] libata: disable_irq() during polling IDENTIFY

2007-05-07 Thread Alan Cox
This reminds me of a possible issue with the patch: Previously the polling code assumes that the interrupt handler won't interfere with it and the polling code runs without holding ap-lock. However, with the above patch, the interrupt handler might read the Status register when the polling

[PATCH] libata: disable_irq() during polling IDENTIFY

2007-05-06 Thread Albert Lee
Problem: Kernel got irq 5: nobody cared when using libata + polling IDENTIFY + Promise 20275 adapter + Benq DW1620 drive. Detail message available in bug 8441 (http://bugzilla.kernel.org/show_bug.cgi?id=8441). Cause: The Benq DW1620 drive raises INTRQ during polling IDENTIFY PACKET