Messages in this thread | | | Date | Wed, 08 Apr 2009 13:40:39 -0700 | From | Tejun Heo <> | Subject | Re: [PATCH 2/2] sata_sil: disable DMA engine in ->freeze |
| |
Hello,
Jeff Garzik wrote: >> Thanks for updating the patch Jeff. I see you spotted the reason why >> I didn't put the code into sil_freeze. I tested your patch and it >> prevented the kernel panic. I now get the following output, which >> seems to be correct to me. >> >> WARNING: at drivers/ata/libata-core.c:5209 ata_qc_complete() > > hum, my patch would indeed trigger this WARN_ON_ONCE() in > ata_qc_complete(): > > if (ap->ops->error_handler) { > struct ata_device *dev = qc->dev; > struct ata_eh_info *ehi = &dev->link->eh_info; > > WARN_ON_ONCE(ap->pflags & ATA_PFLAG_FROZEN); > > Tejun, was that WARN_ON() originally added to detect spurious callers? > Or, completions after the EH started? > > AFAICS, libata still owns the qc's at this point, so it should not be a > problem to complete them when the port is frozen.
Ah... that one. The WARN_ON_ONCE() was added because I was worried that LLD interrupt handler might get activated after the port is frozen and try to complete the commands which now belong to EH. Given that ata_qc_from_tag() returns NULL for any commands which get already marked failed, it's a bit paranoid. Well, I was a bit paranoid when adding new EH the first time, so...
Removing it should be fine at this point, I think, but I'm away from my toys so testing is a bit difficult, so please test the path which triggered the WARN_ON_ONCE() works fine (it should) before removing it.
Thanks.
-- tejun
| |