mvsas: fix default can_queue

PATCH In review James Bottomley 3 participants 14 messages Feb 29, 2008 – Mar 7, 2008 SCSI

Share & tools

James Bottomley

author maintainer

2008-02-29 18:02 UTC #0 lore raw

PATCH 1/1 mvsas: fix default can_queue

I noticed that the current marvell sas driver wasn't performing very well. It turns out that it's setting can_queue not in the SCSI host, but in its own internal data structure, meaning it's always operating with a global queue depth of one. This patch raises it to what the code seemed to be intending ... although I think can_queue should be MVS_CHIP_SLOT_SZ - 1 (without the divide by two)?

The good news is that with this change, I'm getting a respectable throughput on the fio hammer test; plus zapping random phy resets across the disk triggers error handler recovery correctly (so far).

I'm having less happy results with a SATAPI DVD ... it looks like the initial IDENTIFY goes across just fine, but that we stall on the other SCSI commands ... I'm still investigating this one.

James

---

drivers/scsi/mvsas.c +1 -2

index 5ec0665..a128f67 100755
@@ -618,7 +618,6 @@ struct mvs_info {
 	struct mvs_phy		phy[MVS_MAX_PHYS];
 	struct mvs_port		port[MVS_MAX_PHYS];
 
-	u32			can_queue;	/* per adapter */
 	u32			tag_out;	/*Get*/
 	u32			tag_in;		/*Give*/
 };

@@ -2150,7 +2149,7 @@ static struct mvs_info *__devinit mvs_alloc(struct pci_dev *pdev,
 	mvi->sas.num_phys = chip->n_phy;
 	mvi->sas.lldd_max_execute_num = MVS_CHIP_SLOT_SZ - 1;
 	mvi->sas.lldd_queue_size = MVS_QUEUE_SIZE;
-	mvi->can_queue = (MVS_CHIP_SLOT_SZ >> 1) - 1;
+	mvi->shost->can_queue = (MVS_CHIP_SLOT_SZ >> 1) - 1;
 	mvi->sas.lldd_ha = mvi;
 	mvi->sas.core.shost = mvi->shost;

James Bottomley

author maintainer

2008-03-03 00:42 UTC #1 lore raw

Quoted 7 lines · James Bottomley

I noticed that the current marvell sas driver wasn't performing very well. It turns out that it's setting can_queue not in the SCSI host, but in its own internal data structure, meaning it's always operating with a global queue depth of one. This patch raises it to what the code seemed to be intending ... although I think can_queue should be MVS_CHIP_SLOT_SZ - 1 (without the divide by two)?

Quoted 7 lines

The good news is that with this change, I'm getting a respectable throughput on the fio hammer test; plus zapping random phy resets across the disk triggers error handler recovery correctly (so far).
I'm having less happy results with a SATAPI DVD ... it looks like the initial IDENTIFY goes across just fine, but that we stall on the other SCSI commands ... I'm still investigating this one.

Actually, I've run into another problem with this patch applied. It looks like NCQ fails with ATA disks. What I see is that I/O goes fine until I get more than one command outstanding to the device, then the device stops responding. I can keep the I/O flowing if I clamp the device queue depth at 1. SAS disks seem to be fine ... I can get multiple outstanding commands to them correctly serviced.

James

Ke Wei

2008-03-03 08:17 UTC #2 lore raw

Quoted 17 lines · James Bottomley

Quoted 7 lines · James Bottomley
I noticed that the current marvell sas driver wasn't performing very well. It turns out that it's setting can_queue not in the SCSI host, but in its own internal data structure, meaning it's always operating with a global queue depth of one. This patch raises it to what the code seemed to be intending ... although I think can_queue should be MVS_CHIP_SLOT_SZ - 1 (without the divide by two)?
Quoted 7 lines
The good news is that with this change, I'm getting a respectable throughput on the fio hammer test; plus zapping random phy resets across the disk triggers error handler recovery correctly (so far).
I'm having less happy results with a SATAPI DVD ... it looks like the initial IDENTIFY goes across just fine, but that we stall on the other SCSI commands ... I'm still investigating this one.

Quoted 6 lines

Actually, I've run into another problem with this patch applied. It looks like NCQ fails with ATA disks. What I see is that I/O goes fine until I get more than one command outstanding to the device, then the device stops responding. I can keep the I/O flowing if I clamp the device queue depth at 1. SAS disks seem to be fine ... I can get multiple outstanding commands to them correctly serviced.

Yes, I have to say that testing failed when I plugged SATA and SAS disk. Sometimes "insmod mvsas" will cause the system to hang. Only look good if can_queue is set to 1. I will investigate this case.

Quoted 3 lines

I'm having less happy results with a SATAPI DVD ... it looks like the initial IDENTIFY goes across just fine, but that we stall on the other SCSI commands ... I'm still investigating this one.

I think we need set BLIST_NOREPORTLUN or some other flags (see scsi_devinfo.h) about new some ATAPI device.When calling scsi_report_lun_scan , it will bypass REPORT_LUNS command.

Signature · 2 lines

Best Regards,
Ke Wei

James Bottomley

author maintainer

2008-03-03 14:59 UTC #3 lore raw

Quoted 26 lines · Ke Wei

Quoted 17 lines · James Bottomley
Quoted 7 lines · James Bottomley
I noticed that the current marvell sas driver wasn't performing very well. It turns out that it's setting can_queue not in the SCSI host, but in its own internal data structure, meaning it's always operating with a global queue depth of one. This patch raises it to what the code seemed to be intending ... although I think can_queue should be MVS_CHIP_SLOT_SZ - 1 (without the divide by two)?
Quoted 7 lines
The good news is that with this change, I'm getting a respectable throughput on the fio hammer test; plus zapping random phy resets across the disk triggers error handler recovery correctly (so far).
I'm having less happy results with a SATAPI DVD ... it looks like the initial IDENTIFY goes across just fine, but that we stall on the other SCSI commands ... I'm still investigating this one.
Quoted 6 lines
Actually, I've run into another problem with this patch applied. It looks like NCQ fails with ATA disks. What I see is that I/O goes fine until I get more than one command outstanding to the device, then the device stops responding. I can keep the I/O flowing if I clamp the device queue depth at 1. SAS disks seem to be fine ... I can get multiple outstanding commands to them correctly serviced.

Quoted 3 lines

Yes, I have to say that testing failed when I plugged SATA and SAS disk. Sometimes "insmod mvsas" will cause the system to hang. Only look good if can_queue is set to 1. I will investigate this case.

Thanks. For the NCQ case, it does look like turning NCQ off makes the disk work fine, so I'd suspect some issue with NCQ handling.

Quoted 7 lines

Quoted 3 lines
I'm having less happy results with a SATAPI DVD ... it looks like the initial IDENTIFY goes across just fine, but that we stall on the other SCSI commands ... I'm still investigating this one.
I think we need set BLIST_NOREPORTLUN or some other flags (see scsi_devinfo.h) about new some ATAPI device.When calling scsi_report_lun_scan , it will bypass REPORT_LUNS command.

It doesn't seem to be anything the DVD does ... it works fine with the aic94xx controller doing SATAPI (it sends the correct reply to REPORT LUNS). It looks like the first hang comes at around the second or third Test Unit Ready.

Traces seem to show IDENTIFY_PACKET, INQUIRY, INQUIRY, TUR, TUR (hang) and then every following command hangs, but I'll try to instrument more accurate tracing.

James

James Bottomley

author maintainer

2008-03-03 16:40 UTC #4 lore raw

Quoted 27 lines · James Bottomley

Quoted 26 lines · Ke Wei
Quoted 23 lines · James Bottomley
Quoted 7 lines · James Bottomley
I noticed that the current marvell sas driver wasn't performing very well. It turns out that it's setting can_queue not in the SCSI host, but in its own internal data structure, meaning it's always operating with a global queue depth of one. This patch raises it to what the code seemed to be intending ... although I think can_queue should be MVS_CHIP_SLOT_SZ - 1 (without the divide by two)?
Quoted 7 lines
The good news is that with this change, I'm getting a respectable throughput on the fio hammer test; plus zapping random phy resets across the disk triggers error handler recovery correctly (so far).
I'm having less happy results with a SATAPI DVD ... it looks like the initial IDENTIFY goes across just fine, but that we stall on the other SCSI commands ... I'm still investigating this one.
Actually, I've run into another problem with this patch applied. It looks like NCQ fails with ATA disks. What I see is that I/O goes fine until I get more than one command outstanding to the device, then the device stops responding. I can keep the I/O flowing if I clamp the device queue depth at 1. SAS disks seem to be fine ... I can get multiple outstanding commands to them correctly serviced.

Quoted 6 lines

Quoted 3 lines
Yes, I have to say that testing failed when I plugged SATA and SAS disk. Sometimes "insmod mvsas" will cause the system to hang. Only look good if can_queue is set to 1. I will investigate this case.
Thanks. For the NCQ case, it does look like turning NCQ off makes the disk work fine, so I'd suspect some issue with NCQ handling.

As a work around until the NCQ issue is fixed, this will allow can_queue to be raised while still forcing all ATA devices to the non-NCQ case.

Also, can can_queue be set to MVS_CHIP_SLOT_SZ - 1? It seems like a reasonable default.

James

---

drivers/scsi/mvsas.c +22 -1

index 5ec0665..425231f 100755
@@ -37,6 +37,8 @@
 #include <linux/dma-mapping.h>
 #include <linux/ctype.h>
 #include <scsi/libsas.h>
+#include <scsi/scsi_tcq.h>
+#include <scsi/sas_ata.h>
 #include <asm/io.h>
 
 #define DRV_NAME	"mvsas"

@@ -654,12 +655,31 @@ static const struct mvs_chip_info mvs_chips[] = {
 	[chip_6480] =		{ 8, 32, 10 },
 };
 
+static int mvsas_slave_configure(struct scsi_device *sdev)
+{
+	struct domain_device *dev = sdev_to_domain_dev(sdev);
+	int ret = sas_slave_configure(sdev);
+
+	if (ret)
+		return ret;
+
+	if (dev_is_sata(dev)) {
+		struct ata_port *ap = dev->sata_dev.ap;
+		struct ata_device *adev = ap->link.device;
+
+		/* clamp at no NCQ for the time being */
+		adev->flags |= ATA_DFLAG_NCQ_OFF;
+		scsi_adjust_queue_depth(sdev, MSG_SIMPLE_TAG, 1);
+	}
+	return 0;
+}
+
 static struct scsi_host_template mvs_sht = {
 	.module			= THIS_MODULE,
 	.name			= DRV_NAME,
 	.queuecommand		= sas_queuecommand,
 	.target_alloc		= sas_target_alloc,
-	.slave_configure	= sas_slave_configure,
+	.slave_configure	= mvsas_slave_configure,
 	.slave_destroy		= sas_slave_destroy,
 	.scan_finished		= mvs_scan_finished,
 	.scan_start		= mvs_scan_start,

James Bottomley

author maintainer

2008-03-05 02:07 UTC #5 lore raw

In reply to James Bottomley

Quoted 42 lines · James Bottomley

Quoted 26 lines · Ke Wei
Quoted 17 lines · James Bottomley
Quoted 7 lines · James Bottomley
I noticed that the current marvell sas driver wasn't performing very well. It turns out that it's setting can_queue not in the SCSI host, but in its own internal data structure, meaning it's always operating with a global queue depth of one. This patch raises it to what the code seemed to be intending ... although I think can_queue should be MVS_CHIP_SLOT_SZ - 1 (without the divide by two)?
Quoted 7 lines
The good news is that with this change, I'm getting a respectable throughput on the fio hammer test; plus zapping random phy resets across the disk triggers error handler recovery correctly (so far).
I'm having less happy results with a SATAPI DVD ... it looks like the initial IDENTIFY goes across just fine, but that we stall on the other SCSI commands ... I'm still investigating this one.
Quoted 6 lines
Actually, I've run into another problem with this patch applied. It looks like NCQ fails with ATA disks. What I see is that I/O goes fine until I get more than one command outstanding to the device, then the device stops responding. I can keep the I/O flowing if I clamp the device queue depth at 1. SAS disks seem to be fine ... I can get multiple outstanding commands to them correctly serviced.
Quoted 3 lines
Yes, I have to say that testing failed when I plugged SATA and SAS disk. Sometimes "insmod mvsas" will cause the system to hang. Only look good if can_queue is set to 1. I will investigate this case.
Thanks. For the NCQ case, it does look like turning NCQ off makes the disk work fine, so I'd suspect some issue with NCQ handling.
Quoted 7 lines
Quoted 3 lines
I'm having less happy results with a SATAPI DVD ... it looks like the initial IDENTIFY goes across just fine, but that we stall on the other SCSI commands ... I'm still investigating this one.
I think we need set BLIST_NOREPORTLUN or some other flags (see scsi_devinfo.h) about new some ATAPI device.When calling scsi_report_lun_scan , it will bypass REPORT_LUNS command.

Quoted 8 lines

It doesn't seem to be anything the DVD does ... it works fine with the aic94xx controller doing SATAPI (it sends the correct reply to REPORT LUNS). It looks like the first hang comes at around the second or third Test Unit Ready.
Traces seem to show IDENTIFY_PACKET, INQUIRY, INQUIRY, TUR, TUR (hang) and then every following command hangs, but I'll try to instrument more accurate tracing.

OK, I instrumented more ... you're right, the first failing command is REPORT_LUNS. The failure isn't because the DVD doesn't accept the command, but because it gets errored and we fail to report back the error data.

What I see is the mvsas driver returning RXQ_ERR, so the device is trying to terminate the transaction with an error code. Unfortunately, when it sees this code, mvsas does nothing at all, leaving the request to time out and be aborted (even through it already finished).

I can plumb it in ... it looks like we should also be doing is calling mvs_slot_complete(), but this still isn't quite correct ... it just sets SAM_STAT_CHECK_COND ... it needs to collect the ATA error code somehow.

James

James Bottomley

author maintainer

2008-03-05 21:02 UTC #6 lore raw

Quoted 57 lines · James Bottomley

Quoted 42 lines · James Bottomley
Quoted 26 lines · Ke Wei
Quoted 17 lines · James Bottomley
Quoted 7 lines · James Bottomley
I noticed that the current marvell sas driver wasn't performing very well. It turns out that it's setting can_queue not in the SCSI host, but in its own internal data structure, meaning it's always operating with a global queue depth of one. This patch raises it to what the code seemed to be intending ... although I think can_queue should be MVS_CHIP_SLOT_SZ - 1 (without the divide by two)?
Quoted 7 lines
The good news is that with this change, I'm getting a respectable throughput on the fio hammer test; plus zapping random phy resets across the disk triggers error handler recovery correctly (so far).
I'm having less happy results with a SATAPI DVD ... it looks like the initial IDENTIFY goes across just fine, but that we stall on the other SCSI commands ... I'm still investigating this one.
Quoted 6 lines
Actually, I've run into another problem with this patch applied. It looks like NCQ fails with ATA disks. What I see is that I/O goes fine until I get more than one command outstanding to the device, then the device stops responding. I can keep the I/O flowing if I clamp the device queue depth at 1. SAS disks seem to be fine ... I can get multiple outstanding commands to them correctly serviced.
Quoted 3 lines
Yes, I have to say that testing failed when I plugged SATA and SAS disk. Sometimes "insmod mvsas" will cause the system to hang. Only look good if can_queue is set to 1. I will investigate this case.
Thanks. For the NCQ case, it does look like turning NCQ off makes the disk work fine, so I'd suspect some issue with NCQ handling.
Quoted 7 lines
Quoted 3 lines
I'm having less happy results with a SATAPI DVD ... it looks like the initial IDENTIFY goes across just fine, but that we stall on the other SCSI commands ... I'm still investigating this one.
I think we need set BLIST_NOREPORTLUN or some other flags (see scsi_devinfo.h) about new some ATAPI device.When calling scsi_report_lun_scan , it will bypass REPORT_LUNS command.
Quoted 8 lines
It doesn't seem to be anything the DVD does ... it works fine with the aic94xx controller doing SATAPI (it sends the correct reply to REPORT LUNS). It looks like the first hang comes at around the second or third Test Unit Ready.
Traces seem to show IDENTIFY_PACKET, INQUIRY, INQUIRY, TUR, TUR (hang) and then every following command hangs, but I'll try to instrument more accurate tracing.
OK, I instrumented more ... you're right, the first failing command is REPORT_LUNS. The failure isn't because the DVD doesn't accept the command, but because it gets errored and we fail to report back the error data.

Quoted 8 lines

What I see is the mvsas driver returning RXQ_ERR, so the device is trying to terminate the transaction with an error code. Unfortunately, when it sees this code, mvsas does nothing at all, leaving the request to time out and be aborted (even through it already finished).
I can plumb it in ... it looks like we should also be doing is calling mvs_slot_complete(), but this still isn't quite correct ... it just sets SAM_STAT_CHECK_COND ... it needs to collect the ATA error code somehow.

Just by way of update, the slot is completing with RXQ_ERR set, but RXQ_DONE clear. The mvs_err_info field has TFILE_ERR set (the only set bit) and MVS_INT_STAT_SRS is zero.

I assume the slot processing has halted, and that we need to collect the task file error registers and resume it somehow, but how?

James

Ke Wei

2008-03-06 14:46 UTC #7 lore raw

On Thu, Mar 6, 2008 at 5:02 AM, James Bottomley
<[email protected]> wrote:

Quoted 5 lines

Quoted 5 lines
OK, I instrumented more ... you're right, the first failing command is REPORT_LUNS. The failure isn't because the DVD doesn't accept the command, but because it gets errored and we fail to report back the error data.

First, I noticed that scsi_probe_and_add_lun function has changed since v2.6.17-git4.
It will cause bflagsp to miss BLIST_NOREPORTLUN flag after calling scsi_add_lun. On the other hand, Even though mvsas driver can succeed in reporting back the error data, scsi subsystem also will force device to reset because of wrong REPORTLUN status. But Error again, then reset. Maybe I may write simulation codes for this command.

2.6.17-git4:
/**
* scsi_probe_and_add_lun - probe a LUN, if a LUN is found add it
@@ -880,10 +906,12 @@ static int scsi_probe_and_add_lun(struct
if (scsi_probe_lun(sdev, result, result_len, &bflags))
goto out_free_result;

+ if (bflagsp)

+      *bflagsp = bflags;
    /*
     * result contains valid SCSI INQUIRY data.
     */

- if ((result[0] >> 5) == 3) {
+ if (((result[0] >> 5) == 3) && !(bflags & BLIST_ATTACH_PQ3)) {
/*
* For a Peripheral qualifier 3 (011b), the SCSI
* spec says: The device server is not capable of
@@ -894,9 +922,22 @@ static int scsi_probe_and_add_lun(struct
* logical disk configured at sdev->lun, but there
* is a target id responding.
*/
- SCSI_LOG_SCAN_BUS(3, printk(KERN_INFO
- "scsi scan: peripheral qualifier of 3,"
- " no device added\n"));
+ SCSI_LOG_SCAN_BUS(2, sdev_printk(KERN_INFO, sdev, "scsi scan:"
+ " peripheral qualifier of 3, device not"
+ " added\n"))
+ if (lun == 0) {
+ SCSI_LOG_SCAN_BUS(1, {
+ unsigned char vend[9];
+ unsigned char mod[17];
+
+ sdev_printk(KERN_INFO, sdev,

+               "scsi scan: consider passing scsi_mod."
+               "dev_flags=%s:%s:0x240 or 0x800240\n",
+               scsi_inq_str(vend, result, 8, 16),
+               scsi_inq_str(mod, result, 16, 32));
+         });
+      }
+
       res = SCSI_SCAN_TARGET_PRESENT;
       goto out_free_result;
    }
@@ -920,8 +961,6 @@ static int scsi_probe_and_add_lun(struct
          sdev->lockable = 0;
          scsi_unlock_floptical(sdev, result);
       }
-      if (bflagsp)
-         *bflagsp = bflags;
    }

Quoted 5 lines

As a work around until the NCQ issue is fixed, this will allow can_queue to be raised while still forcing all ATA devices to the non-NCQ case.
Also, can can_queue be set to MVS_CHIP_SLOT_SZ - 1? It seems like a reasonable default.

I also test failed. By far, can_queue only be set to 1. I will keep on investigating.

Quoted 3 lines

The good news is that with this change, I'm getting a respectable throughput on the fio hammer test; plus zapping random phy resets across the disk triggers error handler recovery correctly (so far).

I fixed it because of a register mistake. And I'm testing for the follow patch.

Signature · 2 lines

Best Regards,
Ke Wei

James Bottomley

author maintainer

2008-03-06 15:52 UTC #8 lore raw

Quoted 10 lines · Ke Wei

On Thu, Mar 6, 2008 at 5:02 AM, James Bottomley
<[email protected]> wrote:
1.
Quoted 5 lines
Quoted 5 lines
OK, I instrumented more ... you're right, the first failing command is REPORT_LUNS. The failure isn't because the DVD doesn't accept the command, but because it gets errored and we fail to report back the error data.

Quoted 7 lines

First, I noticed that scsi_probe_and_add_lun function has changed since v2.6.17-git4.
It will cause bflagsp to miss BLIST_NOREPORTLUN flag after calling scsi_add_lun. On the other hand, Even though mvsas driver can succeed in reporting back the error data, scsi subsystem also will force device to reset because of wrong REPORTLUN status. But Error again, then reset. Maybe I may write simulation codes for this command.

But this isn't the problem.

I'll look into the mid layer changes and see if we have a problem. However, this device does respond correctly (with an error) to REPORT_LUNS (however, at the moment, for me, the current setup is a great test of ATA error handling).

When I have it on the aic94xx it terminates the command and sends back a Register D2H FIS with

error = 0x54 (sense key ILLEGAL REQUEST, ABRT)
status = 0x51 (DRDY, SERV, CHK)

So the device does a correct termination for an unsupported command. The problem seems to be that mvsas isn't terminating and picking up the register D2H fis properly. Which, in turn seems to be connected with the way it's not really processing a RXQ_ERR return.

This is a perfectly proper termination, and the scan routine will proceed normally without invoking any error handling (it's aware of the conflict between SPC and MMC regarding REPORT_LUNS).

James

James Bottomley

author maintainer

2008-03-06 17:44 UTC #9 lore raw

Quoted 32 lines · James Bottomley

Quoted 10 lines · Ke Wei
On Thu, Mar 6, 2008 at 5:02 AM, James Bottomley
<[email protected]> wrote:
1.
Quoted 5 lines
Quoted 5 lines
OK, I instrumented more ... you're right, the first failing command is REPORT_LUNS. The failure isn't because the DVD doesn't accept the command, but because it gets errored and we fail to report back the error data.
Quoted 7 lines
First, I noticed that scsi_probe_and_add_lun function has changed since v2.6.17-git4.
It will cause bflagsp to miss BLIST_NOREPORTLUN flag after calling scsi_add_lun. On the other hand, Even though mvsas driver can succeed in reporting back the error data, scsi subsystem also will force device to reset because of wrong REPORTLUN status. But Error again, then reset. Maybe I may write simulation codes for this command.
But this isn't the problem.
I'll look into the mid layer changes and see if we have a problem. However, this device does respond correctly (with an error) to REPORT_LUNS (however, at the moment, for me, the current setup is a great test of ATA error handling).
When I have it on the aic94xx it terminates the command and sends back a Register D2H FIS with
error = 0x54 (sense key ILLEGAL REQUEST, ABRT)
status = 0x51 (DRDY, SERV, CHK)

Quoted 8 lines

So the device does a correct termination for an unsupported command. The problem seems to be that mvsas isn't terminating and picking up the register D2H fis properly. Which, in turn seems to be connected with the way it's not really processing a RXQ_ERR return.
This is a perfectly proper termination, and the scan routine will proceed normally without invoking any error handling (it's aware of the conflict between SPC and MMC regarding REPORT_LUNS).

Actually, I looked in your mvi->rx_fis area where the D2H FIS should be and there seems to be a correct one collected. The problem is that if the slot returns RXQ_ERR without any other flags set (other than the slot number), the driver does nothing. Shouldn't the slot be completed and the D2H FIS returned so we can then do a request sense?

James

Jeff Garzik

2008-03-06 17:59 UTC #10 lore raw

Quoted 39 lines · James Bottomley

Quoted 29 lines · James Bottomley
Quoted 8 lines · Ke Wei
On Thu, Mar 6, 2008 at 5:02 AM, James Bottomley
<[email protected]> wrote:
1.
Quoted 4 lines
Quoted 4 lines
OK, I instrumented more ... you're right, the first failing command is REPORT_LUNS. The failure isn't because the DVD doesn't accept the command, but because it gets errored and we fail to report back the error data.
Quoted 7 lines
First, I noticed that scsi_probe_and_add_lun function has changed since v2.6.17-git4.
It will cause bflagsp to miss BLIST_NOREPORTLUN flag after calling scsi_add_lun. On the other hand, Even though mvsas driver can succeed in reporting back the error data, scsi subsystem also will force device to reset because of wrong REPORTLUN status. But Error again, then reset. Maybe I may write simulation codes for this command.
But this isn't the problem.
I'll look into the mid layer changes and see if we have a problem. However, this device does respond correctly (with an error) to REPORT_LUNS (however, at the moment, for me, the current setup is a great test of ATA error handling).
When I have it on the aic94xx it terminates the command and sends back a Register D2H FIS with
error = 0x54 (sense key ILLEGAL REQUEST, ABRT)
status = 0x51 (DRDY, SERV, CHK)
Quoted 8 lines
So the device does a correct termination for an unsupported command. The problem seems to be that mvsas isn't terminating and picking up the register D2H fis properly. Which, in turn seems to be connected with the way it's not really processing a RXQ_ERR return.
This is a perfectly proper termination, and the scan routine will proceed normally without invoking any error handling (it's aware of the conflict between SPC and MMC regarding REPORT_LUNS).

Quoted 5 lines

Actually, I looked in your mvi->rx_fis area where the D2H FIS should be and there seems to be a correct one collected. The problem is that if the slot returns RXQ_ERR without any other flags set (other than the slot number), the driver does nothing. Shouldn't the slot be completed and the D2H FIS returned so we can then do a request sense?

That's a good question... Ke?

I wrote that slot completion code according to the docs, which seemed to indicate that nothing associated with a slot could be touched until RXQ_DONE was set.

It seemed analagous to the 'OWN' bit of a DMA ring found in many other hardwares, where software should not touch anything related to the descriptor if that bit is not set.

Jeff

Ke Wei

2008-03-07 10:50 UTC #11 lore raw

On a system with many SAS targets, It appears possible that a scsi_cmnd can time out without ever making it to the SAS LLDD or at the same time that a completion is occurring.
in file sas_scsi_host.c :

               /* Queue up, Direct Mode or Task Collector Mode. */
               if (sas_ha->lldd_max_execute_num < 2)
                               res = i->dft->lldd_execute_task(task,
1, GFP_ATOMIC);
               else
                               res = sas_queue_up(task);

If I set lldd_max_execute_num above 1, I find that libsas couldn't queue a task to the SAS LLDD sometimes.
System will always report: not at initiator: EH_RESET_TIMER. Is queue_thread pending? I will keep investigating.

Signature · 2 lines

Best Regards,
Ke Wei

James Bottomley

author maintainer

2008-03-07 15:03 UTC #12 lore raw

Quoted 10 lines · Ke Wei

On a system with many SAS targets, It appears possible that a scsi_cmnd can time out without ever making it to the SAS LLDD or at the same time that a completion is occurring.
in file sas_scsi_host.c :
               /* Queue up, Direct Mode or Task Collector Mode. */
               if (sas_ha->lldd_max_execute_num < 2)
                               res = i->dft->lldd_execute_task(task,
1, GFP_ATOMIC);
               else
                               res = sas_queue_up(task);

Quoted 4 lines

If I set lldd_max_execute_num above 1, I find that libsas couldn't queue a task to the SAS LLDD sometimes.
System will always report: not at initiator: EH_RESET_TIMER. Is queue_thread pending? I will keep investigating.

Oh, actually, that's the old task collector mode. I keep meaning to strip it out of libsas ... this provides a good excuse. Basically the only valid case is lldd_max_execute_num == 1 since, as you found, anything above that doesn't work.

Are you still seeing timeouts in the lldd_max_execute_num == 1 case? And if so, is that still with can_queue == 1?

James

Ke Wei

2008-03-07 15:31 UTC #13 lore raw

Quoted 5 lines · James Bottomley

Are you still seeing timeouts in the lldd_max_execute_num == 1 case? And if so, is that still with can_queue == 1?

Yes , I set lldd_max_execute_num = 1 & can_queue = 1. By far, no timeout event.

Quoted 4 lines

James

Signature · 2 lines

Best Regards,
Ke Wei