On Sun, Aug 21, 2005 at 03:52:27PM -0400, Jeff Garzik wrote: > Tejun Heo wrote: > >02_libata_add-locking-to-ata_scsi_error.patch > > > > SCSI EH can start before ata_qc_complete is completely > > complete. so, latter part of ata_qc_complete can run > > side-by-side with ->eng_timeout(), interfering EH. > > > > This patch makes ata_scsi_error() to grab and release host_set > > lock before invoking ->eng_timeout(). > > > > Note: host_failed decrementing and eh_cmd_q banging are moved > > above ->eng_timeout() invocation such that they're done while > > holding the lock. > > I think it's better to follow a path that sets a "EH owns this port now" > flag inside spinlock, > > spin_lock_irqsave() > ap->in_eh = 1; > spin_unlock_irqrestore() > > and then check that flag in ata_qc_complete() > > if (qc->ap->in_eh) > return; /* do nothing else with command */ > > so that for the case of a late interrupt, the interrupt handler should > acknowledge the interrupt, but not actually touch the ata_queued_cmd state.
Hi! I think above is larger subject than what this patch deals with and should be dealt with when we revamp EH (implementing in_eh will certainly involve other changes, I think). So, I think, for short term, either Albert's patch[1][2] or above one should go in to plug ATA_QCFLAG_ACTIVE race. I think if Albert's patch goes in, above patch is not necessary, as the only race we currently have is ATAPI EH case and clearing ATA_QCFLAG_ACTIVE is the only thing we do after ->complete_fn() in such cases. Thanks. [1] http://marc.theaimsgroup.com/?l=linux-ide&m=112417360223374&w=2 [2] Well, this referencing thing is neat. ;-) -- tejun - To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
