| Message ID | 1351501298-3716-6-git-send-email-aaron.lu@intel.com |
|---|---|
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
On Mon, Oct 29, 2012 at 05:01:32PM +0800, Aaron Lu wrote: > ata_exec_internal will preempt the ata link's active_tag and ata port's > qc_active flags, this is OK for error recovery, but if normal code path > wants to use ata_exec_internal, there is a problem: we need to check if > it is OK to issue a new command with the help of port_ops->defer. > > In ZPODD, I'll need to find out the loading mechanism of the ODD by > issuing a GET_CONFIGURATION command. And this command may very well > race with commands issued from SCSI layer. So instead of preempt the > current command, defer the new command if it's not OK to issue it, as > it is always wrong to issue a non-NCQ command when there is command(s) > in processing. Why not do the discovery from EH? Thanks.
On 10/29/2012 11:20 PM, Tejun Heo wrote: > On Mon, Oct 29, 2012 at 05:01:32PM +0800, Aaron Lu wrote: >> ata_exec_internal will preempt the ata link's active_tag and ata port's >> qc_active flags, this is OK for error recovery, but if normal code path >> wants to use ata_exec_internal, there is a problem: we need to check if >> it is OK to issue a new command with the help of port_ops->defer. >> >> In ZPODD, I'll need to find out the loading mechanism of the ODD by >> issuing a GET_CONFIGURATION command. And this command may very well >> race with commands issued from SCSI layer. So instead of preempt the >> current command, defer the new command if it's not OK to issue it, as >> it is always wrong to issue a non-NCQ command when there is command(s) >> in processing. > > Why not do the discovery from EH? Do you mean set a device level EH flag and then schedule EH to do the discovery? Thanks, Aaron -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello, On Tue, Oct 30, 2012 at 11:00:16AM +0800, Aaron Lu wrote: > > Why not do the discovery from EH? > > Do you mean set a device level EH flag and then schedule EH to do the > discovery? Yeah, something like that or why can't it be done while probing the device in the first place? Thanks.
On 10/30/2012 11:01 AM, Tejun Heo wrote: > Hello, > > On Tue, Oct 30, 2012 at 11:00:16AM +0800, Aaron Lu wrote: >>> Why not do the discovery from EH? >> >> Do you mean set a device level EH flag and then schedule EH to do the >> discovery? > > Yeah, something like that or why can't it be done while probing the > device in the first place? I'm not aware of a place to store such ODD specific information when probing the device. I'm currently storing the loading mech type in structure zpodd, which gets created after the corresponding SCSI device gets created in ata_scsi_scan_host, so at the probe time, the zpodd structure does not exist yet. And the reason I create the zpodd sturcture this late is that, it is only created when the ODD together the platform is ZPODD capable, and to find out if this platform is ZPODD capbale, ACPI binding has to occur first, and ACPI binding happens when SCSI device is added to the device tree. Thanks, Aaron -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello, On Tue, Oct 30, 2012 at 11:09:21AM +0800, Aaron Lu wrote: > I'm not aware of a place to store such ODD specific information when > probing the device. You can always add some fields. :) > I'm currently storing the loading mech type in structure zpodd, which > gets created after the corresponding SCSI device gets created in > ata_scsi_scan_host, so at the probe time, the zpodd structure does not > exist yet. And the reason I create the zpodd sturcture this late is > that, it is only created when the ODD together the platform is ZPODD > capable, and to find out if this platform is ZPODD capbale, ACPI binding > has to occur first, and ACPI binding happens when SCSI device is added > to the device tree. Hmm... I see. Which ACPI binding is it? The ATA ACPI binding happens during probing. It's a different one, I presume? Thanks.
On 11/01/2012 05:52 AM, Tejun Heo wrote: > Hello, > > On Tue, Oct 30, 2012 at 11:09:21AM +0800, Aaron Lu wrote: >> I'm not aware of a place to store such ODD specific information when >> probing the device. > > You can always add some fields. :) OK. My concern is that, such information is only useful to ZPODD capable device+platforms, so checking this loading mechanism thing for all ATAPI devices during probe time doesn't seem a good idea. > >> I'm currently storing the loading mech type in structure zpodd, which >> gets created after the corresponding SCSI device gets created in >> ata_scsi_scan_host, so at the probe time, the zpodd structure does not >> exist yet. And the reason I create the zpodd sturcture this late is >> that, it is only created when the ODD together the platform is ZPODD >> capable, and to find out if this platform is ZPODD capbale, ACPI binding >> has to occur first, and ACPI binding happens when SCSI device is added >> to the device tree. > > Hmm... I see. Which ACPI binding is it? The ATA ACPI binding happens > during probing. It's a different one, I presume? Since commit 6b66d95895c149cbc04d4fac5a2f5477c543a8ae: libata: bind the Linux device tree to the ACPI device tree ACPI binding happens when SCSI devices are added to the device tree. The ata port/device software structure does not have a acpi_handle field anymore. Thanks, Aaron -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello, Aaron. On Thu, Nov 01, 2012 at 10:35:10AM +0800, Aaron Lu wrote: > > You can always add some fields. :) > > OK. My concern is that, such information is only useful to ZPODD > capable device+platforms, so checking this loading mechanism thing for > all ATAPI devices during probe time doesn't seem a good idea. Hmmm.. but it's not like querying acpi is high cost or anything. Maybe I'm missing something but if it can be simpler that way, please do so by all means. I don't care whether you add some extra fields or some processing overhead during probing. It doesn't really matter. > > Hmm... I see. Which ACPI binding is it? The ATA ACPI binding happens > > during probing. It's a different one, I presume? > > Since commit 6b66d95895c149cbc04d4fac5a2f5477c543a8ae: > libata: bind the Linux device tree to the ACPI device tree > ACPI binding happens when SCSI devices are added to the device tree. The > ata port/device software structure does not have a acpi_handle field > anymore. Please bear with me. I haven't paid much attention to zpodd, so it probably is my ignorance but at least the ATA <-> ACPI association happens during probing by calling ata_acpi_on_devcfg() from ata_dev_configure(), and it pretty much has to happen then because _SDD/_GTF should be executed after hardreset which happens during probing. So, yeah, I'm confused. Thanks.
On 11/02/2012 12:03 AM, Tejun Heo wrote: > Hello, Aaron. > > On Thu, Nov 01, 2012 at 10:35:10AM +0800, Aaron Lu wrote: >>> You can always add some fields. :) >> >> OK. My concern is that, such information is only useful to ZPODD >> capable device+platforms, so checking this loading mechanism thing for >> all ATAPI devices during probe time doesn't seem a good idea. > > Hmmm.. but it's not like querying acpi is high cost or anything. > Maybe I'm missing something but if it can be simpler that way, please > do so by all means. I don't care whether you add some extra fields or > some processing overhead during probing. It doesn't really matter. OK, thanks for the suggestion. > >>> Hmm... I see. Which ACPI binding is it? The ATA ACPI binding happens >>> during probing. It's a different one, I presume? >> >> Since commit 6b66d95895c149cbc04d4fac5a2f5477c543a8ae: >> libata: bind the Linux device tree to the ACPI device tree >> ACPI binding happens when SCSI devices are added to the device tree. The >> ata port/device software structure does not have a acpi_handle field >> anymore. > > Please bear with me. I haven't paid much attention to zpodd, so it > probably is my ignorance but at least the ATA <-> ACPI association > happens during probing by calling ata_acpi_on_devcfg() from > ata_dev_configure(), and it pretty much has to happen then because > _SDD/_GTF should be executed after hardreset which happens during > probing. So, yeah, I'm confused. Oh, yes, ACPI handle can be retrieved any time by quering some ACPI method. And during probe time, it is done that way, while the binding happens much later... But the ACPI handle can indeed be fetched at that time :-) So all the conditions I need to test ZPODD support is ready during probe time and I'll move the check loading mechanism code there, thanks for your suggestion. Thanks, Aaron -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index 611050d..95fb7b8 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -1557,6 +1557,7 @@ unsigned ata_exec_internal_sg(struct ata_device *dev, unsigned long flags; unsigned int err_mask; int rc; + bool eh_in_recover; spin_lock_irqsave(ap->lock, flags); @@ -1588,14 +1589,21 @@ unsigned ata_exec_internal_sg(struct ata_device *dev, qc->dev = dev; ata_qc_reinit(qc); - preempted_tag = link->active_tag; - preempted_sactive = link->sactive; - preempted_qc_active = ap->qc_active; - preempted_nr_active_links = ap->nr_active_links; - link->active_tag = ATA_TAG_POISON; - link->sactive = 0; - ap->qc_active = 0; - ap->nr_active_links = 0; + eh_in_recover = ap->pflags & ATA_PFLAG_EH_IN_PROGRESS; + if (eh_in_recover) { + preempted_tag = link->active_tag; + preempted_sactive = link->sactive; + preempted_qc_active = ap->qc_active; + preempted_nr_active_links = ap->nr_active_links; + link->active_tag = ATA_TAG_POISON; + link->sactive = 0; + ap->qc_active = 0; + ap->nr_active_links = 0; + } else if (ap->ops->qc_defer && ap->ops->qc_defer(qc)) { + ata_qc_free(qc); + spin_unlock_irqrestore(ap->lock, flags); + return -EBUSY; + } /* prepare & issue qc */ qc->tf = *tf; @@ -1687,10 +1695,12 @@ unsigned ata_exec_internal_sg(struct ata_device *dev, err_mask = qc->err_mask; ata_qc_free(qc); - link->active_tag = preempted_tag; - link->sactive = preempted_sactive; - ap->qc_active = preempted_qc_active; - ap->nr_active_links = preempted_nr_active_links; + if (eh_in_recover) { + link->active_tag = preempted_tag; + link->sactive = preempted_sactive; + ap->qc_active = preempted_qc_active; + ap->nr_active_links = preempted_nr_active_links; + } spin_unlock_irqrestore(ap->lock, flags);
ata_exec_internal will preempt the ata link's active_tag and ata port's qc_active flags, this is OK for error recovery, but if normal code path wants to use ata_exec_internal, there is a problem: we need to check if it is OK to issue a new command with the help of port_ops->defer. In ZPODD, I'll need to find out the loading mechanism of the ODD by issuing a GET_CONFIGURATION command. And this command may very well race with commands issued from SCSI layer. So instead of preempt the current command, defer the new command if it's not OK to issue it, as it is always wrong to issue a non-NCQ command when there is command(s) in processing. So ata_exec_internal is modified to check if it is in eh recovery environment, and if yes, act as before; if not, check if this command should be defered with the help of port_ops->defer. Signed-off-by: Aaron Lu <aaron.lu@intel.com> --- drivers/ata/libata-core.c | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-)