Re: Need help with libata error handling in libsas
James Bottomley wrote: I keep hearing that we need to convert libsas to use libata's new error handling. Unfortunately, I have very little conception of what that means. Right at the moment, libsas doesn't use any error handling functions of libata at all. I've looked through the libata-eh functions, and I find them frankly incomprehensible. Firstly, let me say what SAS error handling actually does: Let me chime in with what ipr error handling does/can do. The ipr firmware provides two basic SATA error handling methods with some modifiers to each. Cancel All - This cancels all outstanding commands to the device. When issued to an ATA device, this gets escalated by the firmware to an SRST. When issued to an ATAPI device, an ATA NOOP is issued. Reset Device - This command has modifiers to indicate either a soft reset or a hard reset. Currently, the only SATA devices that ipr officially attaches are ATAPI DVD devices. In our testing we've come to the conclusion that trying to use anything but a hard reset for ERP is generally more trouble than it is worth. All of this leads me to conclude, that all libsas needs is to plumb in the ATA equivalent of abort, junk the task query for libata devices and simply proceed, as if the task is held at the target, along the escalating reset path. The new libata-eh is used for more than just EH. It is used for device probing, device revalidation, and power management. It is also woken for all command failures and is where the request sense for ATAPI devices is issued. Device revalidation following reset is also critical for ATA and ATAPI devices. One example of this is some SATA/PATA converter chips lose their DMA xfer settings following a reset and default to PIO mode only. Any DMA transfer that is attempted simply hangs. The other issue is PMP support. The more that gets pushed into libsas, the more libsas needs to know about things such as PMP. -Brian -- Brian King Linux on Power Virtualization IBM Linux Technology Center - 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
[RFC 2/3] [SCSI/libata] libata EH conversion for ipr SAS
Currently, the SAS users of libata are using a mix of the
old and new error handling. This patch introduces a new
API which can be used by both ipr and libsas. It has several
advantages to the current implementation:
1. It uses the new libata EH
2. Device discovery is now driven by libata, which should hopefully
make is easier to support PMP on SAS.
3. SATA rphy's have their own scsi_host, which makes SAS much more
like all the other SATA drivers.
4. It eliminates tying scsi_target object lifetimes to ata_port lifetimes
and introduces a cleaner API.
Signed-off-by: Brian King [EMAIL PROTECTED]
---
linux-2.6-bjking1/drivers/ata/libata-scsi.c | 130 +++-
linux-2.6-bjking1/include/linux/libata.h| 18 +++
2 files changed, 146 insertions(+), 2 deletions(-)
diff -puN drivers/ata/libata-scsi.c~libata_sas_rphy3 drivers/ata/libata-scsi.c
--- linux-2.6/drivers/ata/libata-scsi.c~libata_sas_rphy32007-10-29
11:46:23.0 -0500
+++ linux-2.6-bjking1/drivers/ata/libata-scsi.c 2007-10-29 11:53:14.0
-0500
@@ -3456,7 +3456,10 @@ EXPORT_SYMBOL_GPL(ata_sas_port_alloc);
*/
int ata_sas_port_start(struct ata_port *ap)
{
- return ata_pad_alloc(ap, ap-dev);
+ ap-pad_dma = 0;
+ ap-pad = dma_alloc_coherent(ap-dmadev, ATA_DMA_PAD_BUF_SZ,
+ap-pad_dma, GFP_KERNEL);
+ return (ap-pad == NULL) ? -ENOMEM : 0;
}
EXPORT_SYMBOL_GPL(ata_sas_port_start);
@@ -3474,7 +3477,11 @@ EXPORT_SYMBOL_GPL(ata_sas_port_start);
void ata_sas_port_stop(struct ata_port *ap)
{
- ata_pad_free(ap, ap-dev);
+ if (ap-pad) {
+ dma_free_coherent(ap-dmadev, ATA_DMA_PAD_BUF_SZ, ap-pad,
ap-pad_dma);
+ ap-pad = NULL;
+ ap-pad_dma = 0;
+ }
}
EXPORT_SYMBOL_GPL(ata_sas_port_stop);
@@ -3560,3 +3567,122 @@ int ata_sas_queuecmd(struct scsi_cmnd *c
return rc;
}
EXPORT_SYMBOL_GPL(ata_sas_queuecmd);
+
+static unsigned int ata_rphy_id;
+
+static void ata_sas_rphy_release(struct device *dev)
+{
+ put_device(dev-parent);
+ kfree(dev_to_sata_rphy(dev));
+}
+
+/**
+ * ata_sas_rphy_alloc - Allocate a SATA rphy in a SAS topology
+ * @parent: parent device
+ * @dmadev: device to use when mapping DMA buffers
+ * @pinfo: ATA port info
+ * @privsize: size of additional memory to allocate with ata_sas_rphy
+ *
+ * RETURNS:
+ * Pointer to ata_sas_rphy, NULL if a failure occurs.
+ */
+struct ata_sas_rphy *ata_sas_rphy_alloc(struct device *parent,
+ struct device *dmadev,
+ const struct ata_port_info *pinfo,
+ unsigned int privsize)
+{
+ const struct ata_port_info *apinfo[] = { pinfo, NULL };
+ struct ata_sas_rphy *rphy;
+
+ rphy = kzalloc(sizeof(*rphy) + privsize, GFP_KERNEL);
+ if (!rphy)
+ return NULL;
+
+ rphy-id = ata_rphy_id++;
+ device_initialize(rphy-dev);
+ rphy-dev.parent = get_device(parent);
+ rphy-dev.release = ata_sas_rphy_release;
+ sprintf(rphy-dev.bus_id, ata%u, rphy-id);
+
+ rphy-host = ata_host_alloc_pinfo(rphy-dev, apinfo, 1);
+
+ if (!rphy-host) {
+ kfree(rphy);
+ return NULL;
+ }
+
+ rphy-host-dmadev = dmadev;
+ rphy-host-ports[0]-dmadev = dmadev;
+ transport_setup_device(rphy-dev);
+ return rphy;
+}
+EXPORT_SYMBOL_GPL(ata_sas_rphy_alloc);
+
+/**
+ * ata_sas_rphy_add - Add a SATA rphy to the device hierarchy
+ * @rphy: SATA rphy to add
+ * @sht: SCSI host template
+ *
+ * Adds a SATA rphy to sysfs, allocates scsi hosts, and
+ * scans for devices.
+ *
+ * RETURNS:
+ * 0 on success, non-zero on error
+ */
+int ata_sas_rphy_add(struct ata_sas_rphy *rphy, struct scsi_host_template *sht)
+{
+ int rc = ata_host_start(rphy-host);
+
+ if (rc)
+ return rc;
+
+ rc = device_add(rphy-dev);
+
+ if (rc)
+ return rc;
+
+ rc = ata_host_register(rphy-host, sht);
+
+ if (rc) {
+ device_del(rphy-dev);
+ put_device(rphy-dev);
+ return rc;
+ }
+
+ transport_add_device(rphy-dev);
+ transport_configure_device(rphy-dev);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(ata_sas_rphy_add);
+
+/**
+ * ata_sas_rphy_free - Free a SATA rphy
+ * @rphy: SATA rphy to free
+ *
+ * Note:
+ * This function must only be called on an rphy that has not
+ * sucessfully been added using ata_sas_rphy_add().
+ */
+void ata_sas_rphy_free(struct ata_sas_rphy *rphy)
+{
+ transport_destroy_device(rphy-dev);
+ put_device(rphy-dev);
+}
+EXPORT_SYMBOL_GPL(ata_sas_rphy_free);
+
+/**
+ * ata_sas_rphy_delete - Delete a SATA rphy
+ * @rphy: SATA rphy to delete
+ *
+ */
+void ata_sas_rphy_delete(struct ata_sas_rphy *rphy)
+{
+ struct device *dev
[RFC 0/3] [SCSI/libata] libata EH conversion for ipr SAS
The following three patches convert ipr to use the new libata EH APIs. In the process of doing this, I first looked into implementing this in a similar manner to how libata SAS is done today, which is hooking into target_alloc/target_destroy to allocate/delete sata ports. While I was able to get this working after writing my own eh_strategy_handler, I was doubtful this was the best long term solution. One problem with this implementation I didn't solve was the fact that libata now invokes EH for each and every error received. For some devices, such as optical devices, each not ready response received during a media reload would result in all the attached SAS devices getting quiesced as well. My second approach is the attached patch set. In this series I have created a new libata API which can be used by SAS LLDDs. It introduces an ata_sas_rphy device object which is created/destroyed by the following API: ata_sas_rphy_alloc ata_sas_rphy_add ata_sas_rphy_delete When using this API in ipr, I made ipr's scsi_host the parent device of the SATA rphy. The SATA rphy is then the parent of the allocated scsi_hosts. This means that each SATA rphy in the SAS topology will have its own scsi_host, making SAS *much* more like all the SATA LLDDs in how it uses libata. The only issue I ran into with this implementation is that since each SATA port has its own scsi_host, the adapter cannot rely on scsi core to manage any LLDD or adapter imposed queue depth. To solve this I added some code to ipr. Longer term, block layer queue groups might be another way to do this. I'm still polishing this up, but it is up and running and seems to work with what testing I've done so far. -Brian -- Brian King Linux on Power Virtualization IBM Linux Technology Center - 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
[RFC 1/3] [SCSI/libata] libata EH conversion for ipr SAS
Currently libata uses ap-dev when allocating DMA'able storage on
behalf of the LLDD, or when mapping DMA buffers. This dev struct
is also being used when allocating memory for the ata_port struct
and associated structures. This patch splits these two uses by adding
a dmadev pointer to the ata_port. This allows for ap-dev to be
any arbitrary struct device. This is to be used by the libata SAS
LLDDs.
Signed-off-by: Brian King [EMAIL PROTECTED]
---
linux-2.6-bjking1/drivers/ata/libata-core.c | 13 -
linux-2.6-bjking1/include/linux/libata.h|2 ++
2 files changed, 10 insertions(+), 5 deletions(-)
diff -puN include/linux/libata.h~libata_dmadev include/linux/libata.h
--- linux-2.6/include/linux/libata.h~libata_dmadev 2007-10-29
11:31:39.0 -0500
+++ linux-2.6-bjking1/include/linux/libata.h2007-10-29 11:41:53.0
-0500
@@ -391,6 +391,7 @@ struct ata_ioports {
struct ata_host {
spinlock_t lock;
struct device *dev;
+ struct device *dmadev;
void __iomem * const*iomap;
unsigned intn_ports;
void*private_data;
@@ -601,6 +602,7 @@ struct ata_port {
struct ata_port_stats stats;
struct ata_host *host;
struct device *dev;
+ struct device *dmadev;
void*port_task_data;
struct delayed_work port_task;
diff -puN drivers/ata/libata-core.c~libata_dmadev drivers/ata/libata-core.c
--- linux-2.6/drivers/ata/libata-core.c~libata_dmadev 2007-10-29
11:31:39.0 -0500
+++ linux-2.6-bjking1/drivers/ata/libata-core.c 2007-10-29 11:31:39.0
-0500
@@ -4294,7 +4294,7 @@ void ata_sg_clean(struct ata_queued_cmd
if (qc-flags ATA_QCFLAG_SG) {
if (qc-n_elem)
- dma_unmap_sg(ap-dev, sg, qc-n_elem, dir);
+ dma_unmap_sg(ap-dmadev, sg, qc-n_elem, dir);
/* restore last sg */
sg_last(sg, qc-orig_n_elem)-length += qc-pad_len;
if (pad_buf) {
@@ -4305,7 +4305,7 @@ void ata_sg_clean(struct ata_queued_cmd
}
} else {
if (qc-n_elem)
- dma_unmap_single(ap-dev,
+ dma_unmap_single(ap-dmadev,
sg_dma_address(sg[0]), sg_dma_len(sg[0]),
dir);
/* restore sg */
@@ -4631,7 +4631,7 @@ static int ata_sg_setup_one(struct ata_q
goto skip_map;
}
- dma_address = dma_map_single(ap-dev, qc-buf_virt,
+ dma_address = dma_map_single(ap-dmadev, qc-buf_virt,
sg-length, dir);
if (dma_mapping_error(dma_address)) {
/* restore sg */
@@ -4719,7 +4719,7 @@ static int ata_sg_setup(struct ata_queue
}
dir = qc-dma_dir;
- n_elem = dma_map_sg(ap-dev, sg, pre_n_elem, dir);
+ n_elem = dma_map_sg(ap-dmadev, sg, pre_n_elem, dir);
if (n_elem 1) {
/* restore last sg */
lsg-length += qc-pad_len;
@@ -6335,7 +6335,7 @@ void ata_host_resume(struct ata_host *ho
*/
int ata_port_start(struct ata_port *ap)
{
- struct device *dev = ap-dev;
+ struct device *dev = ap-dmadev;
int rc;
ap-prd = dmam_alloc_coherent(dev, ATA_PRD_TBL_SZ, ap-prd_dma,
@@ -6480,6 +6480,7 @@ struct ata_port *ata_port_alloc(struct a
ap-ctl = ATA_DEVCTL_OBS;
ap-host = host;
ap-dev = host-dev;
+ ap-dmadev = host-dmadev;
ap-last_ctl = 0xFF;
#if defined(ATA_VERBOSE_DEBUG)
@@ -6589,6 +6590,7 @@ struct ata_host *ata_host_alloc(struct d
spin_lock_init(host-lock);
host-dev = dev;
+ host-dmadev = dev;
host-n_ports = max_ports;
/* allocate ports bound to this host */
@@ -6732,6 +6734,7 @@ void ata_host_init(struct ata_host *host
{
spin_lock_init(host-lock);
host-dev = dev;
+ host-dmadev = dev;
host-flags = flags;
host-ops = ops;
}
_
-
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
Re: ipr using libata old-EH
Jeff Garzik wrote: I just noticed that ipr is still using the old reset and error handling methods. Once libata-dev.git (#mv-eh, #new-eh) work is merged, ipr is the last driver that uses old error handling methods. Please look into upgrading the driver to -error_handler(), -freeze(), -thaw(). I'll look into this. -Brian -- Brian King Linux on Power Virtualization IBM Linux Technology Center - 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
Re: [git patches] libata reset sequence update
Jeff Garzik wrote: This has been testing in -mm for a while, but I wanted to send it separated from the main libata update, since it has a chance of breakage. Most notably, a cumulative timeout (deadline) helps the code from diving into overly-long reset sequences. Please pull from 'reset-seq' branch of master.kernel.org:/pub/scm/linux/kernel/git/jgarzik/libata-dev.git reset-seq James, FYI - this means you'll probably want to revert this patch: http://git.kernel.org/gitweb.cgi?p=linux/kernel/git/jejb/scsi-misc-2.6.git;a=commitdiff;h=a6506b8e111c9bf9e430e32725b96c0688416c8e -Brian - 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
Recent ipr breakage (ata_do_eh api change)
I just noticed that the following patch (git-libata-all-ipr-fix) is currently in mainline which results in inducing a compiler warning: http://git.kernel.org/gitweb.cgi?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=120bda35ff8514c937dac6d4e5c7dc6c01c699ac;hp=771b8dad9653d2659e0ffcc237184cb16c317788 This looks like a fix for a compile problem with the libata-all branch. I did some poking around and it looks like the ata_do_eh api change is in Jeff's #all branch, but not in #upstream, so I am assuming the ipr patch mentioned above should be reverted in mainline for now. Thanks, Brian -- Brian King eServer Storage I/O IBM Linux Technology Center - 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
Re: [PATCH 03/12] libata: separate out ata_host_alloc() and ata_host_attach()
Tejun Heo wrote: Association to SCSI host is done via pointer now even for native ATA case, so this should be easier for SAS. What I'm worried about is how EH gets invoked. libata depends on EH to do a lot of things including probing, requesting sense data, etc. How should this work? For SAS, the scsi_host pointer in the ata port is NULL today, since libata is really not managing the scsi host, the LLDD is. I think the initialization model we want for SAS is a little different than the one you are heading towards on SATA. For SAS, I think we just want to be able to alloc/init and delete/destroy a SATA device a they show up on the transport, without tying it to initialization of the ata host. And this set of patches doesn't necessarily prevent that... SAS attached libata port shares EH with the SAS SCSI host, right? How can Right. we connect SAS EH with libata EH and would it be okay for libata EH hold the SCSI EH (thus holding all command execution on the host) to handle ATA exceptions? Currently, ipr calls ata_do_eh from its eh_device_reset_handler function. This seems to work well enough with the testing that I've done, but it would certainly be nice to get to a more layered EH approach, where we could possibly have pluggable error handlers for different device types. Regarding holding all command execution on the host while performing eh, that doesn't seem to be a huge issue from my perspective, not sure if this would have a larger negative impact on others... Generally speaking, we shouldn't be entering eh very often, and it should only be happening if something went wrong. The biggest issue here might be ATAPI devices, since they tend to report more errors during normal running. The request sense for these devices for SAS is done without entering eh today. Would you want to move this into eh as well? Brian -- Brian King eServer Storage I/O IBM Linux Technology Center - 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
Re: [PATCH 03/12] libata: separate out ata_host_alloc() and ata_host_attach()
Tejun Heo wrote: * ipr is now the only user of ata_host_init(). Either kill it by converting ipr to use ata_host_alloc() and friends or rename and move it to libata-scsi.c One of the problems with converting ipr to use ata_host_alloc and friends is that it then forces ipr to tell libata how many SATA ports are possible. On SAS, this number can't really be calculated, since the maximum number of SATA devices which can possibly be cabled to a SAS adapter, particularly with SAS expanders, is a very large number and is not practical for how this is being used in the current implementation. My guess is that aic94xx will have similar issues/concerns. Brian -- Brian King eServer Storage I/O IBM Linux Technology Center - 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
[PATCH 1/1] libata: Initialize nbytes for internal sg commands
Some LLDDs, like ipr, use nbytes and pad_len to determine the total data transfer length of a command. Make sure nbytes gets initialized for internally generated commands. Signed-off-by: Brian King [EMAIL PROTECTED] --- Jeff, This bug is already fixed in #upstream, with Tejun's patch, which does away with nsect, but it is still broken in 2.6.20. This is a resend of my original, 1 line patch. If this doesn't make 2.6.20, it might be a candidate for -stable, since without it, libata is not able to find SATA devices attached with ipr. Thanks, Brian Signed-off-by: Brian King [EMAIL PROTECTED] --- linux-2.6-bjking1/drivers/ata/libata-core.c |1 + 1 files changed, 1 insertion(+) diff -puN drivers/ata/libata-core.c~libata_fix_nbytes drivers/ata/libata-core.c --- linux-2.6/drivers/ata/libata-core.c~libata_fix_nbytes 2007-01-30 11:24:20.0 -0600 +++ linux-2.6-bjking1/drivers/ata/libata-core.c 2007-01-30 11:24:20.0 -0600 @@ -1250,6 +1250,7 @@ unsigned ata_exec_internal_sg(struct ata ata_sg_init(qc, sg, n_elem); qc-nsect = buflen / ATA_SECT_SIZE; + qc-nbytes = buflen; } qc-private_data = wait; _ - 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
Re: [git patches] libata fixes
Jeff Garzik wrote: All fixes for ugly bugs and/or regressions. Brian King (2): libata: Fixup n_elem initialization libata: Initialize qc-pad_len Thanks for pulling this in. There is one patch outstanding preventing ipr SATA from working: http://marc.theaimsgroup.com/?l=linux-idem=116905877706906w=2 Tejun proposed an alternate patch: http://article.gmane.org/gmane.linux.ide/14792 Which you merged into #upstream for 2.6.21. Any chance one of these patches could make its way in 2.6.20 so ipr SATA support works? Thanks, Brian -- Brian King eServer Storage I/O IBM Linux Technology Center - 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
Re: [PATCH 2/3] libata: Initialize nbytes for internal sg commands
Brian King wrote: Tejun Heo wrote: Brian King wrote: Some LLDDs, like ipr, use nbytes and pad_len to determine the total data transfer length of a command. Make sure nbytes gets initialized for internally generated commands. I think it's better to apply the following patch instead of this one. http://article.gmane.org/gmane.linux.ide/14792 Works for me. I loaded that patch in place of this one and verified it solved my problem as well. I still need the other two patches in this series, however. Jeff, Do you plan to pull this patch series into upstream-fixes? Without these three patches (or 1 3 plus Tejun's patch referenced above), ipr SATA is broken in 2.6.20. Thanks, Brian -- Brian King eServer Storage I/O IBM Linux Technology Center - 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
Re: [PATCH 2/3] libata: Initialize nbytes for internal sg commands
Tejun Heo wrote: Brian King wrote: Some LLDDs, like ipr, use nbytes and pad_len to determine the total data transfer length of a command. Make sure nbytes gets initialized for internally generated commands. I think it's better to apply the following patch instead of this one. http://article.gmane.org/gmane.linux.ide/14792 Works for me. I loaded that patch in place of this one and verified it solved my problem as well. I still need the other two patches in this series, however. Thanks, Brian -- Brian King eServer Storage I/O IBM Linux Technology Center - 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
[PATCH 2/3] libata: Initialize nbytes for internal sg commands
Some LLDDs, like ipr, use nbytes and pad_len to determine the total data transfer length of a command. Make sure nbytes gets initialized for internally generated commands. Signed-off-by: Brian King [EMAIL PROTECTED] --- linux-2.6-bjking1/drivers/ata/libata-core.c |1 + 1 files changed, 1 insertion(+) diff -puN drivers/ata/libata-core.c~libata_fix_nbytes drivers/ata/libata-core.c --- linux-2.6/drivers/ata/libata-core.c~libata_fix_nbytes 2007-01-17 11:15:17.0 -0600 +++ linux-2.6-bjking1/drivers/ata/libata-core.c 2007-01-17 11:15:17.0 -0600 @@ -1250,6 +1250,7 @@ unsigned ata_exec_internal_sg(struct ata ata_sg_init(qc, sg, n_elem); qc-nsect = buflen / ATA_SECT_SIZE; + qc-nbytes = buflen; } qc-private_data = wait; _ - 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
[PATCH 1/3] libata: Fixup n_elem initialization
Fixup the inialization of qc-n_elem. It currently gets
initialized to 1 for commands that do not transfer any data.
Fix this by initializing n_elem to 0 and only setting to 1
in ata_scsi_qc_new when there is data to transfer. This fixes
some problems seen with SATA devices attached to ipr adapters.
Signed-off-by: Brian King [EMAIL PROTECTED]
---
linux-2.6-bjking1/drivers/ata/libata-scsi.c |2 +-
linux-2.6-bjking1/include/linux/libata.h|1 +
2 files changed, 2 insertions(+), 1 deletion(-)
diff -puN drivers/ata/libata-scsi.c~libata_fixup_nelem drivers/ata/libata-scsi.c
--- linux-2.6/drivers/ata/libata-scsi.c~libata_fixup_nelem 2007-01-17
08:35:27.0 -0600
+++ linux-2.6-bjking1/drivers/ata/libata-scsi.c 2007-01-17 08:35:27.0
-0600
@@ -372,7 +372,7 @@ struct ata_queued_cmd *ata_scsi_qc_new(s
if (cmd-use_sg) {
qc-__sg = (struct scatterlist *) cmd-request_buffer;
qc-n_elem = cmd-use_sg;
- } else {
+ } else if (cmd-request_bufflen) {
qc-__sg = qc-sgent;
qc-n_elem = 1;
}
diff -puN include/linux/libata.h~libata_fixup_nelem include/linux/libata.h
--- linux-2.6/include/linux/libata.h~libata_fixup_nelem 2007-01-17
08:35:27.0 -0600
+++ linux-2.6-bjking1/include/linux/libata.h2007-01-17 11:12:49.0
-0600
@@ -1149,6 +1149,7 @@ static inline void ata_qc_reinit(struct
qc-cursect = qc-cursg = qc-cursg_ofs = 0;
qc-nsect = 0;
qc-nbytes = qc-curbytes = 0;
+ qc-n_elem = 0;
qc-err_mask = 0;
ata_tf_init(qc-dev, qc-tf);
_
-
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
[PATCH 3/3] libata: Initialize qc-pad_len
Initialize qc-pad_len for each new command. This ensures that pad_len is not set to a stale value for zero data length commands. Signed-off-by: Brian King [EMAIL PROTECTED] --- linux-2.6-bjking1/include/linux/libata.h |1 + 1 files changed, 1 insertion(+) diff -puN include/linux/libata.h~libata_init_pad_len include/linux/libata.h --- linux-2.6/include/linux/libata.h~libata_init_pad_len2007-01-17 11:15:30.0 -0600 +++ linux-2.6-bjking1/include/linux/libata.h2007-01-17 11:15:30.0 -0600 @@ -1151,6 +1151,7 @@ static inline void ata_qc_reinit(struct qc-nbytes = qc-curbytes = 0; qc-n_elem = 0; qc-err_mask = 0; + qc-pad_len = 0; ata_tf_init(qc-dev, qc-tf); _ - 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
Re: [PATCH] libata: initialize qc-dma_dir to DMA_NONE
Jeff Garzik wrote: Brian King wrote: ACK Does this response mean that you've tested it, and successfully verified your problem is gone? Yes, I have tested it, but all my problems are not gone with this one patch. This fixes the problem I was seeing where the data direction was set incorrectly, but I still have some other issues I am working through. I'll be sending out additional patches shortly. Brian -- Brian King eServer Storage I/O IBM Linux Technology Center - 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
Re: ipr SATA problems in 2.6.20
James Bottomley wrote: On Tue, 2007-01-16 at 17:45 -0500, Jeff Garzik wrote: Tejun recently updated the CDB length areas of the code. I bet it's either a bug somewhere in there, or the SCSI layer isn't passing us proper command lengths in a case or two. Additional traces (starting with SCSI command, before it hits libata) would be helpful. Actually, this looks like a potential bug in atapi_xlat(): it doesn't set qc-dma_dir. Shouldn't it be setting it from qc-scsicmd-sc_data_direction like ata_scsi_translate() does? I think we are OK here since atapi_xlate is only ever called by ata_scsi_translate. Brian -- Brian King eServer Storage I/O IBM Linux Technology Center - 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
Re: ipr SATA problems in 2.6.20
Brian King wrote: James Bottomley wrote: On Tue, 2007-01-16 at 17:45 -0500, Jeff Garzik wrote: Tejun recently updated the CDB length areas of the code. I bet it's either a bug somewhere in there, or the SCSI layer isn't passing us proper command lengths in a case or two. Additional traces (starting with SCSI command, before it hits libata) would be helpful. Actually, this looks like a potential bug in atapi_xlat(): it doesn't set qc-dma_dir. Shouldn't it be setting it from qc-scsicmd-sc_data_direction like ata_scsi_translate() does? I think we are OK here since atapi_xlate is only ever called by ata_scsi_translate. I spoke too soon... ata_scsi_translate only sets up dma_dir if its a read or write, which means it never gets initialized if the direction is DMA_NONE. Brian -- Brian King eServer Storage I/O IBM Linux Technology Center - 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
Re: [PATCH] libata: initialize qc-dma_dir to DMA_NONE
ACK
Tejun Heo wrote:
libata didn't used to init qc-dma_dir to any specific value on qc
initialization and command translation path didn't set qc-dma_dir if
the command doesn't need data transfer. This made non-data commands
to have random qc-dma_dir.
This usually doesn't cause problem because LLDs usually check
qc-protocol first and look at qc-dma_dir iff the command needs data
transfer but this doesn't hold for all LLDs.
It might be worthwhile to rename qc-dma_dir to qc-data_dir as we use
the field to tag data direction for both PIO and DMA protocols.
This problem has been spotted by James Bottomley.
Signed-off-by: Tejun Heo [EMAIL PROTECTED]
Cc: James Bottomley [EMAIL PROTECTED]
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 7cfc18f..925ad7f 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -1139,6 +1139,7 @@ static inline void ata_tf_init(struct ata_device *dev,
struct ata_taskfile *tf)
static inline void ata_qc_reinit(struct ata_queued_cmd *qc)
{
+ qc-dma_dir = DMA_NONE;
qc-__sg = NULL;
qc-flags = 0;
qc-cursect = qc-cursg = qc-cursg_ofs = 0;
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Brian King
eServer Storage I/O
IBM Linux Technology Center
-
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
