* [PATCH 0/5] libsas fixes discovered via isci-atapi enabling effort
@ 2011-09-20 22:10 Dan Williams
2011-09-20 22:10 ` [PATCH 1/5] libsas: disable scanning lun > 0 on ata devices Dan Williams
` (5 more replies)
0 siblings, 6 replies; 21+ messages in thread
From: Dan Williams @ 2011-09-20 22:10 UTC (permalink / raw)
To: linux-ide, linux-scsi; +Cc: jgarzik, JBottomley
While enabling atapi device support for isci we encountered a condition
whereby multiple srX devices (one for every possible LUN) were exported
by the midlayer. Looking at other drivers showed that they had a local
workaround for this and other conditions in custom
->slave_{alloc|configure} methods. Rather than add custom workaround
for isci this patchset uplevels those workarounds to libsas proper.
Patch 3/5 is tagged RFC as I do not know the original history behind
hard limiting libsas end devices to a queue_depth of 32. At least mvsas
has been bumping this up to 64 on its own.
---
Dan Williams (5):
libsas: disable scanning lun > 0 on ata devices
libsas: fix ->change_queue_{depth|type} for sata devices
libsas: dynamic queue depth
mvsas: remove mvs_slave_{alloc|configure}
pm8001: remove pm8001_slave_{alloc|configure}
drivers/ata/libata-core.c | 1 +
drivers/ata/libata-scsi.c | 44 +++++++++++++++++++++++------------
drivers/scsi/libsas/sas_scsi_host.c | 42 +++++++++++++++++++--------------
drivers/scsi/mvsas/mv_init.c | 4 ++-
drivers/scsi/mvsas/mv_sas.c | 30 ------------------------
drivers/scsi/mvsas/mv_sas.h | 2 --
drivers/scsi/pm8001/pm8001_init.c | 4 ++-
drivers/scsi/pm8001/pm8001_sas.c | 31 +------------------------
drivers/scsi/pm8001/pm8001_sas.h | 2 --
drivers/scsi/scsi_transport_sas.c | 10 ++++++--
include/linux/libata.h | 2 ++
11 files changed, 69 insertions(+), 103 deletions(-)
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 1/5] libsas: disable scanning lun > 0 on ata devices
2011-09-20 22:10 [PATCH 0/5] libsas fixes discovered via isci-atapi enabling effort Dan Williams
@ 2011-09-20 22:10 ` Dan Williams
2011-09-20 22:15 ` Christoph Hellwig
` (2 more replies)
2011-09-20 22:10 ` [PATCH 2/5] libsas: fix ->change_queue_{depth|type} for sata devices Dan Williams
` (4 subsequent siblings)
5 siblings, 3 replies; 21+ messages in thread
From: Dan Williams @ 2011-09-20 22:10 UTC (permalink / raw)
To: linux-ide, linux-scsi
Cc: Luben Tuikov, jgarzik, JBottomley, Jack Wang, Xiangliang Yu,
Mark Salyzyn, Darrick Wong
Currently mvsas and pm8001 have custom ->slave_alloc implementations to
achieve this. Uplevel it for all libsas drivers as isci encounters problems
with atapi devices when scanning past lun0.
Just do what Darrick suggested [1], and limit the scan for ata devices.
[1] http://marc.info/?l=linux-scsi&m=116604101119861&w=2
Cc: Darrick Wong <djwong@us.ibm.com>
Cc: Xiangliang Yu <yuxiangl@marvell.com>
Cc: Jack Wang <jack_wang@usish.com>
Cc: Mark Salyzyn <msalyzyn@us.xyratex.com>
Cc: Luben Tuikov <ltuikov@yahoo.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/scsi/scsi_transport_sas.c | 10 ++++++++--
1 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/scsi_transport_sas.c b/drivers/scsi/scsi_transport_sas.c
index c6fcf76..9d9330a 100644
--- a/drivers/scsi/scsi_transport_sas.c
+++ b/drivers/scsi/scsi_transport_sas.c
@@ -1545,8 +1545,14 @@ int sas_rphy_add(struct sas_rphy *rphy)
if (identify->device_type == SAS_END_DEVICE &&
rphy->scsi_target_id != -1) {
- scsi_scan_target(&rphy->dev, 0,
- rphy->scsi_target_id, SCAN_WILD_CARD, 0);
+ int lun;
+
+ if (identify->target_port_protocols & SAS_PROTOCOL_SSP)
+ lun = SCAN_WILD_CARD;
+ else
+ lun = 0;
+
+ scsi_scan_target(&rphy->dev, 0, rphy->scsi_target_id, lun, 0);
}
return 0;
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 2/5] libsas: fix ->change_queue_{depth|type} for sata devices
2011-09-20 22:10 [PATCH 0/5] libsas fixes discovered via isci-atapi enabling effort Dan Williams
2011-09-20 22:10 ` [PATCH 1/5] libsas: disable scanning lun > 0 on ata devices Dan Williams
@ 2011-09-20 22:10 ` Dan Williams
2011-09-20 22:16 ` Christoph Hellwig
` (2 more replies)
2011-09-20 22:10 ` [RFC PATCH 3/5] libsas: dynamic queue depth Dan Williams
` (3 subsequent siblings)
5 siblings, 3 replies; 21+ messages in thread
From: Dan Williams @ 2011-09-20 22:10 UTC (permalink / raw)
To: linux-ide, linux-scsi
Cc: jgarzik, JBottomley, Jack Wang, Tejun Heo, Xiangliang Yu, Mark Salyzyn
Pass queue_depth change requests to libata, and prevent queue_type
changes for ATA devices.
Otherwise:
1/ we do not honor the libata specific restrictions on the queue depth
2/ libsas drivers that do not set sdev->tagged_supported are unable to
change the queue_depth of ata devices via sysfs
Cc: Tejun Heo <tj@kernel.org>
Cc: Xiangliang Yu <yuxiangl@marvell.com>
Cc: Jack Wang <jack_wang@usish.com>
Cc: Mark Salyzyn <msalyzyn@us.xyratex.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/ata/libata-core.c | 1 +
drivers/ata/libata-scsi.c | 44 +++++++++++++++++++++++------------
drivers/scsi/libsas/sas_scsi_host.c | 11 ++++++++-
include/linux/libata.h | 2 ++
4 files changed, 42 insertions(+), 16 deletions(-)
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 4a3a5ae..d26c7f4 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -6713,6 +6713,7 @@ EXPORT_SYMBOL_GPL(ata_scsi_queuecmd);
EXPORT_SYMBOL_GPL(ata_scsi_slave_config);
EXPORT_SYMBOL_GPL(ata_scsi_slave_destroy);
EXPORT_SYMBOL_GPL(ata_scsi_change_queue_depth);
+EXPORT_SYMBOL_GPL(__ata_change_queue_depth);
EXPORT_SYMBOL_GPL(sata_scr_valid);
EXPORT_SYMBOL_GPL(sata_scr_read);
EXPORT_SYMBOL_GPL(sata_scr_write);
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 46d087f..19ba770 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -1215,25 +1215,15 @@ void ata_scsi_slave_destroy(struct scsi_device *sdev)
}
/**
- * ata_scsi_change_queue_depth - SCSI callback for queue depth config
- * @sdev: SCSI device to configure queue depth for
- * @queue_depth: new queue depth
- * @reason: calling context
- *
- * This is libata standard hostt->change_queue_depth callback.
- * SCSI will call into this callback when user tries to set queue
- * depth via sysfs.
+ * __ata_change_queue_depth - helper for ata_scsi_change_queue_depth
*
- * LOCKING:
- * SCSI layer (we don't care)
+ * libsas and libata have different approaches for associating a sdev to
+ * its ata_port.
*
- * RETURNS:
- * Newly configured queue depth.
*/
-int ata_scsi_change_queue_depth(struct scsi_device *sdev, int queue_depth,
- int reason)
+int __ata_change_queue_depth(struct ata_port *ap, struct scsi_device *sdev,
+ int queue_depth, int reason)
{
- struct ata_port *ap = ata_shost_to_port(sdev->host);
struct ata_device *dev;
unsigned long flags;
@@ -1269,6 +1259,30 @@ int ata_scsi_change_queue_depth(struct scsi_device *sdev, int queue_depth,
}
/**
+ * ata_scsi_change_queue_depth - SCSI callback for queue depth config
+ * @sdev: SCSI device to configure queue depth for
+ * @queue_depth: new queue depth
+ * @reason: calling context
+ *
+ * This is libata standard hostt->change_queue_depth callback.
+ * SCSI will call into this callback when user tries to set queue
+ * depth via sysfs.
+ *
+ * LOCKING:
+ * SCSI layer (we don't care)
+ *
+ * RETURNS:
+ * Newly configured queue depth.
+ */
+int ata_scsi_change_queue_depth(struct scsi_device *sdev, int queue_depth,
+ int reason)
+{
+ struct ata_port *ap = ata_shost_to_port(sdev->host);
+
+ return __ata_change_queue_depth(ap, sdev, queue_depth, reason);
+}
+
+/**
* ata_scsi_start_stop_xlat - Translate SCSI START STOP UNIT command
* @qc: Storage for translated ATA taskfile
*
diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c
index eeba76c..4c6923c 100644
--- a/drivers/scsi/libsas/sas_scsi_host.c
+++ b/drivers/scsi/libsas/sas_scsi_host.c
@@ -831,8 +831,13 @@ void sas_slave_destroy(struct scsi_device *scsi_dev)
int sas_change_queue_depth(struct scsi_device *scsi_dev, int new_depth,
int reason)
{
+ struct domain_device *dev = sdev_to_domain_dev(scsi_dev);
int res = min(new_depth, SAS_MAX_QD);
+ if (dev_is_sata(dev))
+ return __ata_change_queue_depth(dev->sata_dev.ap, scsi_dev,
+ new_depth, reason);
+
if (reason != SCSI_QDEPTH_DEFAULT)
return -EOPNOTSUPP;
@@ -840,7 +845,6 @@ int sas_change_queue_depth(struct scsi_device *scsi_dev, int new_depth,
scsi_adjust_queue_depth(scsi_dev, scsi_get_tag_type(scsi_dev),
res);
else {
- struct domain_device *dev = sdev_to_domain_dev(scsi_dev);
sas_printk("device %llx LUN %x queue depth changed to 1\n",
SAS_ADDR(dev->sas_addr),
scsi_dev->lun);
@@ -853,6 +857,11 @@ int sas_change_queue_depth(struct scsi_device *scsi_dev, int new_depth,
int sas_change_queue_type(struct scsi_device *scsi_dev, int qt)
{
+ struct domain_device *dev = sdev_to_domain_dev(scsi_dev);
+
+ if (dev_is_sata(dev))
+ return -EINVAL;
+
if (!scsi_dev->tagged_supported)
return 0;
diff --git a/include/linux/libata.h b/include/linux/libata.h
index efd6f98..23fa829 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -1052,6 +1052,8 @@ extern int ata_scsi_slave_config(struct scsi_device *sdev);
extern void ata_scsi_slave_destroy(struct scsi_device *sdev);
extern int ata_scsi_change_queue_depth(struct scsi_device *sdev,
int queue_depth, int reason);
+extern int __ata_change_queue_depth(struct ata_port *ap, struct scsi_device *sdev,
+ int queue_depth, int reason);
extern struct ata_device *ata_dev_pair(struct ata_device *adev);
extern int ata_do_set_mode(struct ata_link *link, struct ata_device **r_failed_dev);
extern void ata_scsi_port_error_handler(struct Scsi_Host *host, struct ata_port *ap);
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [RFC PATCH 3/5] libsas: dynamic queue depth
2011-09-20 22:10 [PATCH 0/5] libsas fixes discovered via isci-atapi enabling effort Dan Williams
2011-09-20 22:10 ` [PATCH 1/5] libsas: disable scanning lun > 0 on ata devices Dan Williams
2011-09-20 22:10 ` [PATCH 2/5] libsas: fix ->change_queue_{depth|type} for sata devices Dan Williams
@ 2011-09-20 22:10 ` Dan Williams
2011-09-20 22:18 ` Christoph Hellwig
` (2 more replies)
2011-09-20 22:10 ` [PATCH 4/5] mvsas: remove mvs_slave_{alloc|configure} Dan Williams
` (2 subsequent siblings)
5 siblings, 3 replies; 21+ messages in thread
From: Dan Williams @ 2011-09-20 22:10 UTC (permalink / raw)
To: linux-ide, linux-scsi
Cc: Dave Jiang, Luben Tuikov, jgarzik, JBottomley, Jack Wang,
Xiangliang Yu, Mark Salyzyn, Christoph Hellwig
The queue-depth for libsas-attached devices initializes to 32 and can
only be increased manually via sysfs to a max of 64, while mpt2sas
attached devices initialize to 254 and dynamically float via the
midlayer ->change_queue_depth interface.
No performance regression was observed with this change on the isci
driver.
Cc: Christoph Hellwig <hch@lst.de>
Cc: Mark Salyzyn <msalyzyn@us.xyratex.com>
Cc: Xiangliang Yu <yuxiangl@marvell.com>
Cc: Jack Wang <jack_wang@usish.com>
Cc: Mark Salyzyn <msalyzyn@us.xyratex.com>
Cc: Luben Tuikov <ltuikov@yahoo.com>
Tested-by: Dave Jiang <dave.jiang@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/scsi/libsas/sas_scsi_host.c | 39 ++++++++++++++++-------------------
1 files changed, 18 insertions(+), 21 deletions(-)
diff --git a/drivers/scsi/libsas/sas_scsi_host.c b/drivers/scsi/libsas/sas_scsi_host.c
index 4c6923c..fe08ef0 100644
--- a/drivers/scsi/libsas/sas_scsi_host.c
+++ b/drivers/scsi/libsas/sas_scsi_host.c
@@ -784,8 +784,7 @@ int sas_target_alloc(struct scsi_target *starget)
return 0;
}
-#define SAS_DEF_QD 32
-#define SAS_MAX_QD 64
+#define SAS_DEF_QD 256
int sas_slave_configure(struct scsi_device *scsi_dev)
{
@@ -828,31 +827,29 @@ void sas_slave_destroy(struct scsi_device *scsi_dev)
dev->sata_dev.ap->link.device[0].class = ATA_DEV_NONE;
}
-int sas_change_queue_depth(struct scsi_device *scsi_dev, int new_depth,
- int reason)
+int sas_change_queue_depth(struct scsi_device *sdev, int depth, int reason)
{
- struct domain_device *dev = sdev_to_domain_dev(scsi_dev);
- int res = min(new_depth, SAS_MAX_QD);
+ struct domain_device *dev = sdev_to_domain_dev(sdev);
if (dev_is_sata(dev))
- return __ata_change_queue_depth(dev->sata_dev.ap, scsi_dev,
- new_depth, reason);
-
- if (reason != SCSI_QDEPTH_DEFAULT)
+ return __ata_change_queue_depth(dev->sata_dev.ap, sdev, depth,
+ reason);
+
+ switch (reason) {
+ case SCSI_QDEPTH_DEFAULT:
+ case SCSI_QDEPTH_RAMP_UP:
+ if (!sdev->tagged_supported)
+ depth = 1;
+ scsi_adjust_queue_depth(sdev, scsi_get_tag_type(sdev), depth);
+ break;
+ case SCSI_QDEPTH_QFULL:
+ scsi_track_queue_full(sdev, depth);
+ break;
+ default:
return -EOPNOTSUPP;
-
- if (scsi_dev->tagged_supported)
- scsi_adjust_queue_depth(scsi_dev, scsi_get_tag_type(scsi_dev),
- res);
- else {
- sas_printk("device %llx LUN %x queue depth changed to 1\n",
- SAS_ADDR(dev->sas_addr),
- scsi_dev->lun);
- scsi_adjust_queue_depth(scsi_dev, 0, 1);
- res = 1;
}
- return res;
+ return depth;
}
int sas_change_queue_type(struct scsi_device *scsi_dev, int qt)
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 4/5] mvsas: remove mvs_slave_{alloc|configure}
2011-09-20 22:10 [PATCH 0/5] libsas fixes discovered via isci-atapi enabling effort Dan Williams
` (2 preceding siblings ...)
2011-09-20 22:10 ` [RFC PATCH 3/5] libsas: dynamic queue depth Dan Williams
@ 2011-09-20 22:10 ` Dan Williams
2011-09-22 2:26 ` Xiangliang Yu
2011-09-20 22:11 ` [PATCH 5/5] pm8001: remove pm8001_slave_{alloc|configure} Dan Williams
2011-09-21 1:13 ` [PATCH 0/5] libsas fixes discovered via isci-atapi enabling effort Jack Wang
5 siblings, 1 reply; 21+ messages in thread
From: Dan Williams @ 2011-09-20 22:10 UTC (permalink / raw)
To: linux-ide, linux-scsi; +Cc: Xiangliang Yu, jgarzik, JBottomley
libsas now handles:
1/ limiting ata scanning to lun0
2/ maximizing the queue_depth of sas devices (up to 256, mvsas only
supports 64)
3/ changes to /sys/block/<sdX>/device/queue_depth for ata devices
Cc: Xiangliang Yu <yuxiangl@marvell.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/scsi/mvsas/mv_init.c | 4 ++--
drivers/scsi/mvsas/mv_sas.c | 30 ------------------------------
drivers/scsi/mvsas/mv_sas.h | 2 --
3 files changed, 2 insertions(+), 34 deletions(-)
diff --git a/drivers/scsi/mvsas/mv_init.c b/drivers/scsi/mvsas/mv_init.c
index 4e9af66..052861b 100644
--- a/drivers/scsi/mvsas/mv_init.c
+++ b/drivers/scsi/mvsas/mv_init.c
@@ -59,7 +59,7 @@ static struct scsi_host_template mvs_sht = {
.name = DRV_NAME,
.queuecommand = sas_queuecommand,
.target_alloc = sas_target_alloc,
- .slave_configure = mvs_slave_configure,
+ .slave_configure = sas_slave_configure,
.slave_destroy = sas_slave_destroy,
.scan_finished = mvs_scan_finished,
.scan_start = mvs_scan_start,
@@ -74,7 +74,7 @@ static struct scsi_host_template mvs_sht = {
.use_clustering = ENABLE_CLUSTERING,
.eh_device_reset_handler = sas_eh_device_reset_handler,
.eh_bus_reset_handler = sas_eh_bus_reset_handler,
- .slave_alloc = mvs_slave_alloc,
+ .slave_alloc = sas_slave_alloc,
.target_destroy = sas_target_destroy,
.ioctl = sas_ioctl,
.shost_attrs = mvst_host_attrs,
diff --git a/drivers/scsi/mvsas/mv_sas.c b/drivers/scsi/mvsas/mv_sas.c
index 4196eee..f2ac01f 100644
--- a/drivers/scsi/mvsas/mv_sas.c
+++ b/drivers/scsi/mvsas/mv_sas.c
@@ -276,36 +276,6 @@ static void mvs_bytes_dmaed(struct mvs_info *mvi, int i)
PORTE_BYTES_DMAED);
}
-int mvs_slave_alloc(struct scsi_device *scsi_dev)
-{
- struct domain_device *dev = sdev_to_domain_dev(scsi_dev);
- if (dev_is_sata(dev)) {
- /* We don't need to rescan targets
- * if REPORT_LUNS request is failed
- */
- if (scsi_dev->lun > 0)
- return -ENXIO;
- scsi_dev->tagged_supported = 1;
- }
-
- return sas_slave_alloc(scsi_dev);
-}
-
-int mvs_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)) {
- sas_change_queue_depth(sdev,
- MVS_QUEUE_SIZE,
- SCSI_QDEPTH_DEFAULT);
- }
- return 0;
-}
-
void mvs_scan_start(struct Scsi_Host *shost)
{
int i, j;
diff --git a/drivers/scsi/mvsas/mv_sas.h b/drivers/scsi/mvsas/mv_sas.h
index 44d7885..21ee8b0 100644
--- a/drivers/scsi/mvsas/mv_sas.h
+++ b/drivers/scsi/mvsas/mv_sas.h
@@ -459,8 +459,6 @@ int mvs_phy_control(struct asd_sas_phy *sas_phy, enum phy_func func,
void *funcdata);
void __devinit mvs_set_sas_addr(struct mvs_info *mvi, int port_id,
u32 off_lo, u32 off_hi, u64 sas_addr);
-int mvs_slave_alloc(struct scsi_device *scsi_dev);
-int mvs_slave_configure(struct scsi_device *sdev);
void mvs_scan_start(struct Scsi_Host *shost);
int mvs_scan_finished(struct Scsi_Host *shost, unsigned long time);
int mvs_queue_command(struct sas_task *task, const int num,
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 5/5] pm8001: remove pm8001_slave_{alloc|configure}
2011-09-20 22:10 [PATCH 0/5] libsas fixes discovered via isci-atapi enabling effort Dan Williams
` (3 preceding siblings ...)
2011-09-20 22:10 ` [PATCH 4/5] mvsas: remove mvs_slave_{alloc|configure} Dan Williams
@ 2011-09-20 22:11 ` Dan Williams
2011-09-21 1:13 ` [PATCH 0/5] libsas fixes discovered via isci-atapi enabling effort Jack Wang
5 siblings, 0 replies; 21+ messages in thread
From: Dan Williams @ 2011-09-20 22:11 UTC (permalink / raw)
To: linux-ide, linux-scsi; +Cc: jgarzik, JBottomley, Jack Wang
libsas handles:
1/ limiting ata scanning to lun0
2/ changes to /sys/block/<sdX>/device/queue_depth for ata devices
libata handles turning off ncq globally via kernel command line
(libata.force=noncq) or sysfs (echo 1 >
/sys/block/<sdX>/device/queue_depth). A lldd specific compile option is
not necessary.
Cc: Jack Wang <jack_wang@usish.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/scsi/pm8001/pm8001_init.c | 4 ++--
drivers/scsi/pm8001/pm8001_sas.c | 31 +------------------------------
drivers/scsi/pm8001/pm8001_sas.h | 2 --
3 files changed, 3 insertions(+), 34 deletions(-)
diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c
index 172cefb..c21a216 100644
--- a/drivers/scsi/pm8001/pm8001_init.c
+++ b/drivers/scsi/pm8001/pm8001_init.c
@@ -61,7 +61,7 @@ static struct scsi_host_template pm8001_sht = {
.name = DRV_NAME,
.queuecommand = sas_queuecommand,
.target_alloc = sas_target_alloc,
- .slave_configure = pm8001_slave_configure,
+ .slave_configure = sas_slave_configure,
.slave_destroy = sas_slave_destroy,
.scan_finished = pm8001_scan_finished,
.scan_start = pm8001_scan_start,
@@ -76,7 +76,7 @@ static struct scsi_host_template pm8001_sht = {
.use_clustering = ENABLE_CLUSTERING,
.eh_device_reset_handler = sas_eh_device_reset_handler,
.eh_bus_reset_handler = sas_eh_bus_reset_handler,
- .slave_alloc = pm8001_slave_alloc,
+ .slave_alloc = sas_slave_alloc,
.target_destroy = sas_target_destroy,
.ioctl = sas_ioctl,
.shost_attrs = pm8001_host_attrs,
diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c
index 7dbbf8b..9068f12 100644
--- a/drivers/scsi/pm8001/pm8001_sas.c
+++ b/drivers/scsi/pm8001/pm8001_sas.c
@@ -216,20 +216,6 @@ int pm8001_phy_control(struct asd_sas_phy *sas_phy, enum phy_func func,
return rc;
}
-int pm8001_slave_alloc(struct scsi_device *scsi_dev)
-{
- struct domain_device *dev = sdev_to_domain_dev(scsi_dev);
- if (dev_is_sata(dev)) {
- /* We don't need to rescan targets
- * if REPORT_LUNS request is failed
- */
- if (scsi_dev->lun > 0)
- return -ENXIO;
- scsi_dev->tagged_supported = 1;
- }
- return sas_slave_alloc(scsi_dev);
-}
-
/**
* pm8001_scan_start - we should enable all HBA phys by sending the phy_start
* command to HBA.
@@ -314,22 +300,7 @@ static int pm8001_task_prep_ssp(struct pm8001_hba_info *pm8001_ha,
{
return PM8001_CHIP_DISP->ssp_io_req(pm8001_ha, ccb);
}
-int pm8001_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)) {
- #ifdef PM8001_DISABLE_NCQ
- struct ata_port *ap = dev->sata_dev.ap;
- struct ata_device *adev = ap->link.device;
- adev->flags |= ATA_DFLAG_NCQ_OFF;
- scsi_adjust_queue_depth(sdev, MSG_SIMPLE_TAG, 1);
- #endif
- }
- return 0;
-}
+
/* Find the local port id that's attached to this device */
static int sas_find_local_port_id(struct domain_device *dev)
{
diff --git a/drivers/scsi/pm8001/pm8001_sas.h b/drivers/scsi/pm8001/pm8001_sas.h
index b97c8ab..93959fe 100644
--- a/drivers/scsi/pm8001/pm8001_sas.h
+++ b/drivers/scsi/pm8001/pm8001_sas.h
@@ -471,8 +471,6 @@ void pm8001_ccb_task_free(struct pm8001_hba_info *pm8001_ha,
struct sas_task *task, struct pm8001_ccb_info *ccb, u32 ccb_idx);
int pm8001_phy_control(struct asd_sas_phy *sas_phy, enum phy_func func,
void *funcdata);
-int pm8001_slave_alloc(struct scsi_device *scsi_dev);
-int pm8001_slave_configure(struct scsi_device *sdev);
void pm8001_scan_start(struct Scsi_Host *shost);
int pm8001_scan_finished(struct Scsi_Host *shost, unsigned long time);
int pm8001_queue_command(struct sas_task *task, const int num,
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 1/5] libsas: disable scanning lun > 0 on ata devices
2011-09-20 22:10 ` [PATCH 1/5] libsas: disable scanning lun > 0 on ata devices Dan Williams
@ 2011-09-20 22:15 ` Christoph Hellwig
2011-09-21 12:28 ` Mark Salyzyn
2011-09-22 2:29 ` Jeff Garzik
2 siblings, 0 replies; 21+ messages in thread
From: Christoph Hellwig @ 2011-09-20 22:15 UTC (permalink / raw)
To: Dan Williams
Cc: linux-ide, linux-scsi, Luben Tuikov, jgarzik, JBottomley,
Jack Wang, Xiangliang Yu, Mark Salyzyn, Darrick Wong
On Tue, Sep 20, 2011 at 03:10:19PM -0700, Dan Williams wrote:
> Currently mvsas and pm8001 have custom ->slave_alloc implementations to
> achieve this. Uplevel it for all libsas drivers as isci encounters problems
> with atapi devices when scanning past lun0.
>
> Just do what Darrick suggested [1], and limit the scan for ata devices.
>
> [1] http://marc.info/?l=linux-scsi&m=116604101119861&w=2
Looks reasonable.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/5] libsas: fix ->change_queue_{depth|type} for sata devices
2011-09-20 22:10 ` [PATCH 2/5] libsas: fix ->change_queue_{depth|type} for sata devices Dan Williams
@ 2011-09-20 22:16 ` Christoph Hellwig
2011-09-21 12:27 ` Mark Salyzyn
2011-09-22 2:30 ` Jeff Garzik
2 siblings, 0 replies; 21+ messages in thread
From: Christoph Hellwig @ 2011-09-20 22:16 UTC (permalink / raw)
To: Dan Williams
Cc: linux-ide, linux-scsi, jgarzik, JBottomley, Jack Wang, Tejun Heo,
Xiangliang Yu, Mark Salyzyn
Looks good.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC PATCH 3/5] libsas: dynamic queue depth
2011-09-20 22:10 ` [RFC PATCH 3/5] libsas: dynamic queue depth Dan Williams
@ 2011-09-20 22:18 ` Christoph Hellwig
2011-09-21 1:17 ` Jack Wang
2011-09-21 12:37 ` Mark Salyzyn
2 siblings, 0 replies; 21+ messages in thread
From: Christoph Hellwig @ 2011-09-20 22:18 UTC (permalink / raw)
To: Dan Williams
Cc: linux-ide, linux-scsi, Dave Jiang, Luben Tuikov, jgarzik,
JBottomley, Jack Wang, Xiangliang Yu, Mark Salyzyn,
Christoph Hellwig
On Tue, Sep 20, 2011 at 03:10:46PM -0700, Dan Williams wrote:
> The queue-depth for libsas-attached devices initializes to 32 and can
> only be increased manually via sysfs to a max of 64, while mpt2sas
> attached devices initialize to 254 and dynamically float via the
> midlayer ->change_queue_depth interface.
>
> No performance regression was observed with this change on the isci
> driver.
Looks reasonable to me.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/5] libsas fixes discovered via isci-atapi enabling effort
2011-09-20 22:10 [PATCH 0/5] libsas fixes discovered via isci-atapi enabling effort Dan Williams
` (4 preceding siblings ...)
2011-09-20 22:11 ` [PATCH 5/5] pm8001: remove pm8001_slave_{alloc|configure} Dan Williams
@ 2011-09-21 1:13 ` Jack Wang
5 siblings, 0 replies; 21+ messages in thread
From: Jack Wang @ 2011-09-21 1:13 UTC (permalink / raw)
To: 'Dan Williams', linux-ide, linux-scsi; +Cc: jgarzik, JBottomley
Hi Dan
Thanks for doing this, I'm OK with the patchset.
Reviewed-by: Jack Wang <jack_wang@usish.com>
[PATCH 0/5] libsas fixes discovered via isci-atapi enabling effort
>
> While enabling atapi device support for isci we encountered a condition
> whereby multiple srX devices (one for every possible LUN) were exported
> by the midlayer. Looking at other drivers showed that they had a local
> workaround for this and other conditions in custom
> ->slave_{alloc|configure} methods. Rather than add custom workaround
> for isci this patchset uplevels those workarounds to libsas proper.
>
> Patch 3/5 is tagged RFC as I do not know the original history behind
> hard limiting libsas end devices to a queue_depth of 32. At least mvsas
> has been bumping this up to 64 on its own.
>
> ---
>
> Dan Williams (5):
> libsas: disable scanning lun > 0 on ata devices
> libsas: fix ->change_queue_{depth|type} for sata devices
> libsas: dynamic queue depth
> mvsas: remove mvs_slave_{alloc|configure}
> pm8001: remove pm8001_slave_{alloc|configure}
>
>
> drivers/ata/libata-core.c | 1 +
> drivers/ata/libata-scsi.c | 44
> +++++++++++++++++++++++------------
> drivers/scsi/libsas/sas_scsi_host.c | 42
> +++++++++++++++++++--------------
> drivers/scsi/mvsas/mv_init.c | 4 ++-
> drivers/scsi/mvsas/mv_sas.c | 30 ------------------------
> drivers/scsi/mvsas/mv_sas.h | 2 --
> drivers/scsi/pm8001/pm8001_init.c | 4 ++-
> drivers/scsi/pm8001/pm8001_sas.c | 31 +------------------------
> drivers/scsi/pm8001/pm8001_sas.h | 2 --
> drivers/scsi/scsi_transport_sas.c | 10 ++++++--
> include/linux/libata.h | 2 ++
> 11 files changed, 69 insertions(+), 103 deletions(-)
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [RFC PATCH 3/5] libsas: dynamic queue depth
2011-09-20 22:10 ` [RFC PATCH 3/5] libsas: dynamic queue depth Dan Williams
2011-09-20 22:18 ` Christoph Hellwig
@ 2011-09-21 1:17 ` Jack Wang
2011-09-21 12:37 ` Mark Salyzyn
2 siblings, 0 replies; 21+ messages in thread
From: Jack Wang @ 2011-09-21 1:17 UTC (permalink / raw)
To: 'Dan Williams', linux-ide, linux-scsi
Cc: 'Dave Jiang', 'Luben Tuikov',
jgarzik, JBottomley, 'Xiangliang Yu',
'Mark Salyzyn', 'Christoph Hellwig'
I suppose to prepare a similar patch, thanks for ding this.
[RFC PATCH 3/5] libsas: dynamic queue depth
>
> The queue-depth for libsas-attached devices initializes to 32 and can
> only be increased manually via sysfs to a max of 64, while mpt2sas
> attached devices initialize to 254 and dynamically float via the
> midlayer ->change_queue_depth interface.
>
> No performance regression was observed with this change on the isci
> driver.
>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Mark Salyzyn <msalyzyn@us.xyratex.com>
> Cc: Xiangliang Yu <yuxiangl@marvell.com>
> Cc: Jack Wang <jack_wang@usish.com>
> Cc: Mark Salyzyn <msalyzyn@us.xyratex.com>
> Cc: Luben Tuikov <ltuikov@yahoo.com>
> Tested-by: Dave Jiang <dave.jiang@intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
> drivers/scsi/libsas/sas_scsi_host.c | 39
> ++++++++++++++++-------------------
> 1 files changed, 18 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/scsi/libsas/sas_scsi_host.c
> b/drivers/scsi/libsas/sas_scsi_host.c
> index 4c6923c..fe08ef0 100644
> --- a/drivers/scsi/libsas/sas_scsi_host.c
> +++ b/drivers/scsi/libsas/sas_scsi_host.c
> @@ -784,8 +784,7 @@ int sas_target_alloc(struct scsi_target *starget)
> return 0;
> }
>
> -#define SAS_DEF_QD 32
> -#define SAS_MAX_QD 64
> +#define SAS_DEF_QD 256
>
> int sas_slave_configure(struct scsi_device *scsi_dev)
> {
> @@ -828,31 +827,29 @@ void sas_slave_destroy(struct scsi_device *scsi_dev)
> dev->sata_dev.ap->link.device[0].class = ATA_DEV_NONE;
> }
>
> -int sas_change_queue_depth(struct scsi_device *scsi_dev, int new_depth,
> - int reason)
> +int sas_change_queue_depth(struct scsi_device *sdev, int depth, int
reason)
> {
> - struct domain_device *dev = sdev_to_domain_dev(scsi_dev);
> - int res = min(new_depth, SAS_MAX_QD);
> + struct domain_device *dev = sdev_to_domain_dev(sdev);
>
> if (dev_is_sata(dev))
> - return __ata_change_queue_depth(dev->sata_dev.ap, scsi_dev,
> - new_depth, reason);
> -
> - if (reason != SCSI_QDEPTH_DEFAULT)
> + return __ata_change_queue_depth(dev->sata_dev.ap, sdev,
depth,
> + reason);
> +
> + switch (reason) {
> + case SCSI_QDEPTH_DEFAULT:
> + case SCSI_QDEPTH_RAMP_UP:
> + if (!sdev->tagged_supported)
> + depth = 1;
> + scsi_adjust_queue_depth(sdev, scsi_get_tag_type(sdev),
depth);
> + break;
> + case SCSI_QDEPTH_QFULL:
> + scsi_track_queue_full(sdev, depth);
> + break;
> + default:
> return -EOPNOTSUPP;
> -
> - if (scsi_dev->tagged_supported)
> - scsi_adjust_queue_depth(scsi_dev,
scsi_get_tag_type(scsi_dev),
> - res);
> - else {
> - sas_printk("device %llx LUN %x queue depth changed to 1\n",
> - SAS_ADDR(dev->sas_addr),
> - scsi_dev->lun);
> - scsi_adjust_queue_depth(scsi_dev, 0, 1);
> - res = 1;
> }
>
> - return res;
> + return depth;
> }
>
> int sas_change_queue_type(struct scsi_device *scsi_dev, int qt)
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH 2/5] libsas: fix ->change_queue_{depth|type} for sata devices
2011-09-20 22:10 ` [PATCH 2/5] libsas: fix ->change_queue_{depth|type} for sata devices Dan Williams
2011-09-20 22:16 ` Christoph Hellwig
@ 2011-09-21 12:27 ` Mark Salyzyn
2011-09-22 2:30 ` Jeff Garzik
2 siblings, 0 replies; 21+ messages in thread
From: Mark Salyzyn @ 2011-09-21 12:27 UTC (permalink / raw)
To: Dan Williams, linux-ide, linux-scsi
Cc: jgarzik, JBottomley, Jack Wang, Tejun Heo, Xiangliang Yu
Good Stuff
-----Original Message-----
From: Dan Williams [mailto:dan.j.williams@intel.com]
Sent: Tuesday, September 20, 2011 6:11 PM
To: linux-ide@vger.kernel.org; linux-scsi@vger.kernel.org
Cc: jgarzik@pobox.com; JBottomley@parallels.com; Jack Wang; Tejun Heo;
Xiangliang Yu; Mark Salyzyn
Subject: [PATCH 2/5] libsas: fix ->change_queue_{depth|type} for sata
devices
Pass queue_depth change requests to libata, and prevent queue_type
changes for ATA devices.
Otherwise:
1/ we do not honor the libata specific restrictions on the queue depth
2/ libsas drivers that do not set sdev->tagged_supported are unable to
change the queue_depth of ata devices via sysfs
Cc: Tejun Heo <tj@kernel.org>
Cc: Xiangliang Yu <yuxiangl@marvell.com>
Cc: Jack Wang <jack_wang@usish.com>
Cc: Mark Salyzyn <msalyzyn@us.xyratex.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/ata/libata-core.c | 1 +
drivers/ata/libata-scsi.c | 44
+++++++++++++++++++++++------------
drivers/scsi/libsas/sas_scsi_host.c | 11 ++++++++-
include/linux/libata.h | 2 ++
4 files changed, 42 insertions(+), 16 deletions(-)
______________________________________________________________________
This email may contain privileged or confidential information, which should only be used for the purpose for which it was sent by Xyratex. No further rights or licenses are granted to use such information. If you are not the intended recipient of this message, please notify the sender by return and delete it. You may not use, copy, disclose or rely on the information contained in it.
Internet email is susceptible to data corruption, interception and unauthorised amendment for which Xyratex does not accept liability. While we have taken reasonable precautions to ensure that this email is free of viruses, Xyratex does not accept liability for the presence of any computer viruses in this email, nor for any losses caused as a result of viruses.
Xyratex Technology Limited (03134912), Registered in England & Wales, Registered Office, Langstone Road, Havant, Hampshire, PO9 1SA.
The Xyratex group of companies also includes, Xyratex Ltd, registered in Bermuda, Xyratex International Inc, registered in California, Xyratex (Malaysia) Sdn Bhd registered in Malaysia, Xyratex Technology (Wuxi) Co Ltd registered in The People's Republic of China and Xyratex Japan Limited registered in Japan.
______________________________________________________________________
^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH 1/5] libsas: disable scanning lun > 0 on ata devices
2011-09-20 22:10 ` [PATCH 1/5] libsas: disable scanning lun > 0 on ata devices Dan Williams
2011-09-20 22:15 ` Christoph Hellwig
@ 2011-09-21 12:28 ` Mark Salyzyn
2011-09-22 2:29 ` Jeff Garzik
2 siblings, 0 replies; 21+ messages in thread
From: Mark Salyzyn @ 2011-09-21 12:28 UTC (permalink / raw)
To: Dan Williams, linux-ide, linux-scsi
Cc: Luben Tuikov, jgarzik, JBottomley, Jack Wang, Xiangliang Yu,
Darrick Wong
Good Stuff
-----Original Message-----
From: Dan Williams [mailto:dan.j.williams@intel.com]
Sent: Tuesday, September 20, 2011 6:10 PM
To: linux-ide@vger.kernel.org; linux-scsi@vger.kernel.org
Cc: Luben Tuikov; jgarzik@pobox.com; JBottomley@parallels.com; Jack
Wang; Xiangliang Yu; Mark Salyzyn; Darrick Wong
Subject: [PATCH 1/5] libsas: disable scanning lun > 0 on ata devices
Currently mvsas and pm8001 have custom ->slave_alloc implementations to
achieve this. Uplevel it for all libsas drivers as isci encounters
problems
with atapi devices when scanning past lun0.
Just do what Darrick suggested [1], and limit the scan for ata devices.
[1] http://marc.info/?l=linux-scsi&m=116604101119861&w=2
Cc: Darrick Wong <djwong@us.ibm.com>
Cc: Xiangliang Yu <yuxiangl@marvell.com>
Cc: Jack Wang <jack_wang@usish.com>
Cc: Mark Salyzyn <msalyzyn@us.xyratex.com>
Cc: Luben Tuikov <ltuikov@yahoo.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/scsi/scsi_transport_sas.c | 10 ++++++++--
1 files changed, 8 insertions(+), 2 deletions(-)
______________________________________________________________________
This email may contain privileged or confidential information, which should only be used for the purpose for which it was sent by Xyratex. No further rights or licenses are granted to use such information. If you are not the intended recipient of this message, please notify the sender by return and delete it. You may not use, copy, disclose or rely on the information contained in it.
Internet email is susceptible to data corruption, interception and unauthorised amendment for which Xyratex does not accept liability. While we have taken reasonable precautions to ensure that this email is free of viruses, Xyratex does not accept liability for the presence of any computer viruses in this email, nor for any losses caused as a result of viruses.
Xyratex Technology Limited (03134912), Registered in England & Wales, Registered Office, Langstone Road, Havant, Hampshire, PO9 1SA.
The Xyratex group of companies also includes, Xyratex Ltd, registered in Bermuda, Xyratex International Inc, registered in California, Xyratex (Malaysia) Sdn Bhd registered in Malaysia, Xyratex Technology (Wuxi) Co Ltd registered in The People's Republic of China and Xyratex Japan Limited registered in Japan.
______________________________________________________________________
^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [RFC PATCH 3/5] libsas: dynamic queue depth
2011-09-20 22:10 ` [RFC PATCH 3/5] libsas: dynamic queue depth Dan Williams
2011-09-20 22:18 ` Christoph Hellwig
2011-09-21 1:17 ` Jack Wang
@ 2011-09-21 12:37 ` Mark Salyzyn
2011-09-22 1:38 ` Jack Wang
2 siblings, 1 reply; 21+ messages in thread
From: Mark Salyzyn @ 2011-09-21 12:37 UTC (permalink / raw)
To: Dan Williams, linux-ide, linux-scsi
Cc: Dave Jiang, Luben Tuikov, jgarzik, JBottomley, Jack Wang,
Xiangliang Yu, Christoph Hellwig
Good Stuff, 'cept I am worried about the host max queue depth of 128 on
the pm8001 driver resulting in starvation. I defer to Jack Wang's
judgement on that point (and will explore separately how we can increase
the host queue depth reliably on the pm8001).
Consider this an Ack from me that has an interlock on Jack Wang's Ack
...
Sincerely -- Mark Salyzyn
-----Original Message-----
From: Dan Williams [mailto:dan.j.williams@intel.com]
Sent: Tuesday, September 20, 2011 6:11 PM
To: linux-ide@vger.kernel.org; linux-scsi@vger.kernel.org
Cc: Dave Jiang; Luben Tuikov; jgarzik@pobox.com;
JBottomley@parallels.com; Jack Wang; Xiangliang Yu; Mark Salyzyn;
Christoph Hellwig
Subject: [RFC PATCH 3/5] libsas: dynamic queue depth
The queue-depth for libsas-attached devices initializes to 32 and can
only be increased manually via sysfs to a max of 64, while mpt2sas
attached devices initialize to 254 and dynamically float via the
midlayer ->change_queue_depth interface.
No performance regression was observed with this change on the isci
driver.
Cc: Christoph Hellwig <hch@lst.de>
Cc: Mark Salyzyn <msalyzyn@us.xyratex.com>
Cc: Xiangliang Yu <yuxiangl@marvell.com>
Cc: Jack Wang <jack_wang@usish.com>
Cc: Mark Salyzyn <msalyzyn@us.xyratex.com>
Cc: Luben Tuikov <ltuikov@yahoo.com>
Tested-by: Dave Jiang <dave.jiang@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/scsi/libsas/sas_scsi_host.c | 39
++++++++++++++++-------------------
1 files changed, 18 insertions(+), 21 deletions(-)
diff --git a/drivers/scsi/libsas/sas_scsi_host.c
b/drivers/scsi/libsas/sas_scsi_host.c
index 4c6923c..fe08ef0 100644
--- a/drivers/scsi/libsas/sas_scsi_host.c
+++ b/drivers/scsi/libsas/sas_scsi_host.c
@@ -784,8 +784,7 @@ int sas_target_alloc(struct scsi_target *starget)
return 0;
}
-#define SAS_DEF_QD 32
-#define SAS_MAX_QD 64
+#define SAS_DEF_QD 256
int sas_slave_configure(struct scsi_device *scsi_dev)
{
______________________________________________________________________
This email may contain privileged or confidential information, which should only be used for the purpose for which it was sent by Xyratex. No further rights or licenses are granted to use such information. If you are not the intended recipient of this message, please notify the sender by return and delete it. You may not use, copy, disclose or rely on the information contained in it.
Internet email is susceptible to data corruption, interception and unauthorised amendment for which Xyratex does not accept liability. While we have taken reasonable precautions to ensure that this email is free of viruses, Xyratex does not accept liability for the presence of any computer viruses in this email, nor for any losses caused as a result of viruses.
Xyratex Technology Limited (03134912), Registered in England & Wales, Registered Office, Langstone Road, Havant, Hampshire, PO9 1SA.
The Xyratex group of companies also includes, Xyratex Ltd, registered in Bermuda, Xyratex International Inc, registered in California, Xyratex (Malaysia) Sdn Bhd registered in Malaysia, Xyratex Technology (Wuxi) Co Ltd registered in The People's Republic of China and Xyratex Japan Limited registered in Japan.
______________________________________________________________________
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [RFC PATCH 3/5] libsas: dynamic queue depth
2011-09-21 12:37 ` Mark Salyzyn
@ 2011-09-22 1:38 ` Jack Wang
0 siblings, 0 replies; 21+ messages in thread
From: Jack Wang @ 2011-09-22 1:38 UTC (permalink / raw)
To: 'Mark Salyzyn', 'Dan Williams', linux-ide, linux-scsi
Cc: 'Dave Jiang', 'Luben Tuikov',
jgarzik, JBottomley, 'Xiangliang Yu',
'Christoph Hellwig'
Thanks Mark, I 'll post a patch to raise the default host can queue value,
I'm OK with his change!
--Jack
RE: [RFC PATCH 3/5] libsas: dynamic queue depth
>
> Good Stuff, 'cept I am worried about the host max queue depth of 128 on
> the pm8001 driver resulting in starvation. I defer to Jack Wang's
> judgement on that point (and will explore separately how we can increase
> the host queue depth reliably on the pm8001).
>
> Consider this an Ack from me that has an interlock on Jack Wang's Ack
> ...
>
> Sincerely -- Mark Salyzyn
>
> -----Original Message-----
> From: Dan Williams [mailto:dan.j.williams@intel.com]
> Sent: Tuesday, September 20, 2011 6:11 PM
> To: linux-ide@vger.kernel.org; linux-scsi@vger.kernel.org
> Cc: Dave Jiang; Luben Tuikov; jgarzik@pobox.com;
> JBottomley@parallels.com; Jack Wang; Xiangliang Yu; Mark Salyzyn;
> Christoph Hellwig
> Subject: [RFC PATCH 3/5] libsas: dynamic queue depth
>
> The queue-depth for libsas-attached devices initializes to 32 and can
> only be increased manually via sysfs to a max of 64, while mpt2sas
> attached devices initialize to 254 and dynamically float via the
> midlayer ->change_queue_depth interface.
>
> No performance regression was observed with this change on the isci
> driver.
>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Mark Salyzyn <msalyzyn@us.xyratex.com>
> Cc: Xiangliang Yu <yuxiangl@marvell.com>
> Cc: Jack Wang <jack_wang@usish.com>
> Cc: Mark Salyzyn <msalyzyn@us.xyratex.com>
> Cc: Luben Tuikov <ltuikov@yahoo.com>
> Tested-by: Dave Jiang <dave.jiang@intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
> drivers/scsi/libsas/sas_scsi_host.c | 39
> ++++++++++++++++-------------------
> 1 files changed, 18 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/scsi/libsas/sas_scsi_host.c
> b/drivers/scsi/libsas/sas_scsi_host.c
> index 4c6923c..fe08ef0 100644
> --- a/drivers/scsi/libsas/sas_scsi_host.c
> +++ b/drivers/scsi/libsas/sas_scsi_host.c
> @@ -784,8 +784,7 @@ int sas_target_alloc(struct scsi_target *starget)
> return 0;
> }
>
> -#define SAS_DEF_QD 32
> -#define SAS_MAX_QD 64
> +#define SAS_DEF_QD 256
>
> int sas_slave_configure(struct scsi_device *scsi_dev)
> {
> ______________________________________________________________________
> This email may contain privileged or confidential information, which
should
> only be used for the purpose for which it was sent by Xyratex. No further
rights
> or licenses are granted to use such information. If you are not the
intended
> recipient of this message, please notify the sender by return and delete
it.
> You may not use, copy, disclose or rely on the information contained in
it.
>
> Internet email is susceptible to data corruption, interception and
> unauthorised amendment for which Xyratex does not accept liability. While
we
> have taken reasonable precautions to ensure that this email is free of
viruses,
> Xyratex does not accept liability for the presence of any computer viruses
in
> this email, nor for any losses caused as a result of viruses.
>
> Xyratex Technology Limited (03134912), Registered in England & Wales,
> Registered Office, Langstone Road, Havant, Hampshire, PO9 1SA.
>
> The Xyratex group of companies also includes, Xyratex Ltd, registered in
> Bermuda, Xyratex International Inc, registered in California, Xyratex
> (Malaysia) Sdn Bhd registered in Malaysia, Xyratex Technology (Wuxi) Co
Ltd
> registered in The People's Republic of China and Xyratex Japan Limited
> registered in Japan.
> ______________________________________________________________________
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH 4/5] mvsas: remove mvs_slave_{alloc|configure}
2011-09-20 22:10 ` [PATCH 4/5] mvsas: remove mvs_slave_{alloc|configure} Dan Williams
@ 2011-09-22 2:26 ` Xiangliang Yu
2011-09-22 2:31 ` Jeff Garzik
2011-09-22 3:17 ` Williams, Dan J
0 siblings, 2 replies; 21+ messages in thread
From: Xiangliang Yu @ 2011-09-22 2:26 UTC (permalink / raw)
To: Dan Williams, linux-ide, linux-scsi; +Cc: jgarzik, JBottomley
> Subject: [PATCH 4/5] mvsas: remove mvs_slave_{alloc|configure}
>
> libsas now handles:
> 1/ limiting ata scanning to lun0
> 2/ maximizing the queue_depth of sas devices (up to 256, mvsas only
> supports 64)
> 3/ changes to /sys/block/<sdX>/device/queue_depth for ata devices
>
> Cc: Xiangliang Yu <yuxiangl@marvell.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
> drivers/scsi/mvsas/mv_init.c | 4 ++--
> drivers/scsi/mvsas/mv_sas.c | 30 ------------------------------
> drivers/scsi/mvsas/mv_sas.h | 2 --
> 3 files changed, 2 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/scsi/mvsas/mv_init.c b/drivers/scsi/mvsas/mv_init.c
> index 4e9af66..052861b 100644
> --- a/drivers/scsi/mvsas/mv_init.c
> +++ b/drivers/scsi/mvsas/mv_init.c
> @@ -59,7 +59,7 @@ static struct scsi_host_template mvs_sht = {
> .name = DRV_NAME,
> .queuecommand = sas_queuecommand,
> .target_alloc = sas_target_alloc,
> - .slave_configure = mvs_slave_configure,
> + .slave_configure = sas_slave_configure,
> .slave_destroy = sas_slave_destroy,
> .scan_finished = mvs_scan_finished,
> .scan_start = mvs_scan_start,
> @@ -74,7 +74,7 @@ static struct scsi_host_template mvs_sht = {
> .use_clustering = ENABLE_CLUSTERING,
> .eh_device_reset_handler = sas_eh_device_reset_handler,
> .eh_bus_reset_handler = sas_eh_bus_reset_handler,
> - .slave_alloc = mvs_slave_alloc,
> + .slave_alloc = sas_slave_alloc,
> .target_destroy = sas_target_destroy,
> -int mvs_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)) {
> - sas_change_queue_depth(sdev,
> - MVS_QUEUE_SIZE,
> - SCSI_QDEPTH_DEFAULT);
> - }
> - return 0;
Can you don't remove mvs_slave_configure function? Someday it will be useful, such as fixing some issue.
Thanks!
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/5] libsas: disable scanning lun > 0 on ata devices
2011-09-20 22:10 ` [PATCH 1/5] libsas: disable scanning lun > 0 on ata devices Dan Williams
2011-09-20 22:15 ` Christoph Hellwig
2011-09-21 12:28 ` Mark Salyzyn
@ 2011-09-22 2:29 ` Jeff Garzik
2 siblings, 0 replies; 21+ messages in thread
From: Jeff Garzik @ 2011-09-22 2:29 UTC (permalink / raw)
To: Dan Williams
Cc: linux-ide, linux-scsi, Luben Tuikov, JBottomley, Jack Wang,
Xiangliang Yu, Mark Salyzyn, Darrick Wong
On 09/20/2011 06:10 PM, Dan Williams wrote:
> Currently mvsas and pm8001 have custom ->slave_alloc implementations to
> achieve this. Uplevel it for all libsas drivers as isci encounters problems
> with atapi devices when scanning past lun0.
>
> Just do what Darrick suggested [1], and limit the scan for ata devices.
>
> [1] http://marc.info/?l=linux-scsi&m=116604101119861&w=2
>
> Cc: Darrick Wong<djwong@us.ibm.com>
> Cc: Xiangliang Yu<yuxiangl@marvell.com>
> Cc: Jack Wang<jack_wang@usish.com>
> Cc: Mark Salyzyn<msalyzyn@us.xyratex.com>
> Cc: Luben Tuikov<ltuikov@yahoo.com>
> Signed-off-by: Dan Williams<dan.j.williams@intel.com>
> ---
> drivers/scsi/scsi_transport_sas.c | 10 ++++++++--
> 1 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/scsi_transport_sas.c b/drivers/scsi/scsi_transport_sas.c
> index c6fcf76..9d9330a 100644
> --- a/drivers/scsi/scsi_transport_sas.c
> +++ b/drivers/scsi/scsi_transport_sas.c
> @@ -1545,8 +1545,14 @@ int sas_rphy_add(struct sas_rphy *rphy)
>
> if (identify->device_type == SAS_END_DEVICE&&
> rphy->scsi_target_id != -1) {
> - scsi_scan_target(&rphy->dev, 0,
> - rphy->scsi_target_id, SCAN_WILD_CARD, 0);
> + int lun;
> +
> + if (identify->target_port_protocols& SAS_PROTOCOL_SSP)
> + lun = SCAN_WILD_CARD;
> + else
> + lun = 0;
> +
> + scsi_scan_target(&rphy->dev, 0, rphy->scsi_target_id, lun, 0);
Acked-by: Jeff Garzik <jgarzik@redhat.com>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/5] libsas: fix ->change_queue_{depth|type} for sata devices
2011-09-20 22:10 ` [PATCH 2/5] libsas: fix ->change_queue_{depth|type} for sata devices Dan Williams
2011-09-20 22:16 ` Christoph Hellwig
2011-09-21 12:27 ` Mark Salyzyn
@ 2011-09-22 2:30 ` Jeff Garzik
2 siblings, 0 replies; 21+ messages in thread
From: Jeff Garzik @ 2011-09-22 2:30 UTC (permalink / raw)
To: Dan Williams
Cc: linux-ide, linux-scsi, JBottomley, Jack Wang, Tejun Heo,
Xiangliang Yu, Mark Salyzyn
On 09/20/2011 06:10 PM, Dan Williams wrote:
> Pass queue_depth change requests to libata, and prevent queue_type
> changes for ATA devices.
>
> Otherwise:
> 1/ we do not honor the libata specific restrictions on the queue depth
> 2/ libsas drivers that do not set sdev->tagged_supported are unable to
> change the queue_depth of ata devices via sysfs
>
> Cc: Tejun Heo<tj@kernel.org>
> Cc: Xiangliang Yu<yuxiangl@marvell.com>
> Cc: Jack Wang<jack_wang@usish.com>
> Cc: Mark Salyzyn<msalyzyn@us.xyratex.com>
> Signed-off-by: Dan Williams<dan.j.williams@intel.com>
> ---
> drivers/ata/libata-core.c | 1 +
> drivers/ata/libata-scsi.c | 44 +++++++++++++++++++++++------------
> drivers/scsi/libsas/sas_scsi_host.c | 11 ++++++++-
> include/linux/libata.h | 2 ++
> 4 files changed, 42 insertions(+), 16 deletions(-)
Acked-by: Jeff Garzik <jgarzik@redhat.com>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/5] mvsas: remove mvs_slave_{alloc|configure}
2011-09-22 2:26 ` Xiangliang Yu
@ 2011-09-22 2:31 ` Jeff Garzik
2011-09-22 3:17 ` Williams, Dan J
1 sibling, 0 replies; 21+ messages in thread
From: Jeff Garzik @ 2011-09-22 2:31 UTC (permalink / raw)
To: Xiangliang Yu; +Cc: Dan Williams, linux-ide, linux-scsi, JBottomley
On 09/21/2011 10:26 PM, Xiangliang Yu wrote:
>
>> Subject: [PATCH 4/5] mvsas: remove mvs_slave_{alloc|configure}
>>
>> libsas now handles:
>> 1/ limiting ata scanning to lun0
>> 2/ maximizing the queue_depth of sas devices (up to 256, mvsas only
>> supports 64)
>> 3/ changes to /sys/block/<sdX>/device/queue_depth for ata devices
>>
>> Cc: Xiangliang Yu<yuxiangl@marvell.com>
>> Signed-off-by: Dan Williams<dan.j.williams@intel.com>
>> ---
>> drivers/scsi/mvsas/mv_init.c | 4 ++--
>> drivers/scsi/mvsas/mv_sas.c | 30 ------------------------------
>> drivers/scsi/mvsas/mv_sas.h | 2 --
>> 3 files changed, 2 insertions(+), 34 deletions(-)
>>
>> diff --git a/drivers/scsi/mvsas/mv_init.c b/drivers/scsi/mvsas/mv_init.c
>> index 4e9af66..052861b 100644
>> --- a/drivers/scsi/mvsas/mv_init.c
>> +++ b/drivers/scsi/mvsas/mv_init.c
>> @@ -59,7 +59,7 @@ static struct scsi_host_template mvs_sht = {
>> .name = DRV_NAME,
>> .queuecommand = sas_queuecommand,
>> .target_alloc = sas_target_alloc,
>> - .slave_configure = mvs_slave_configure,
>> + .slave_configure = sas_slave_configure,
>> .slave_destroy = sas_slave_destroy,
>> .scan_finished = mvs_scan_finished,
>> .scan_start = mvs_scan_start,
>> @@ -74,7 +74,7 @@ static struct scsi_host_template mvs_sht = {
>> .use_clustering = ENABLE_CLUSTERING,
>> .eh_device_reset_handler = sas_eh_device_reset_handler,
>> .eh_bus_reset_handler = sas_eh_bus_reset_handler,
>> - .slave_alloc = mvs_slave_alloc,
>> + .slave_alloc = sas_slave_alloc,
>> .target_destroy = sas_target_destroy,
>> -int mvs_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)) {
>> - sas_change_queue_depth(sdev,
>> - MVS_QUEUE_SIZE,
>> - SCSI_QDEPTH_DEFAULT);
>> - }
>> - return 0;
> Can you don't remove mvs_slave_configure function? Someday it will be useful, such as fixing some issue.
> Thanks!
There is no reason to keep a no-op function there.
Jeff
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 4/5] mvsas: remove mvs_slave_{alloc|configure}
2011-09-22 2:26 ` Xiangliang Yu
2011-09-22 2:31 ` Jeff Garzik
@ 2011-09-22 3:17 ` Williams, Dan J
2011-09-22 3:19 ` Xiangliang Yu
1 sibling, 1 reply; 21+ messages in thread
From: Williams, Dan J @ 2011-09-22 3:17 UTC (permalink / raw)
To: Xiangliang Yu; +Cc: linux-ide, linux-scsi, jgarzik, JBottomley
On Wed, Sep 21, 2011 at 7:26 PM, Xiangliang Yu <yuxiangl@marvell.com> wrote:
>
>> Subject: [PATCH 4/5] mvsas: remove mvs_slave_{alloc|configure}
>>
>> libsas now handles:
>> 1/ limiting ata scanning to lun0
>> 2/ maximizing the queue_depth of sas devices (up to 256, mvsas only
>> supports 64)
>> 3/ changes to /sys/block/<sdX>/device/queue_depth for ata devices
>>
>> Cc: Xiangliang Yu <yuxiangl@marvell.com>
>> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>> ---
>> drivers/scsi/mvsas/mv_init.c | 4 ++--
>> drivers/scsi/mvsas/mv_sas.c | 30 ------------------------------
>> drivers/scsi/mvsas/mv_sas.h | 2 --
>> 3 files changed, 2 insertions(+), 34 deletions(-)
>>
>> diff --git a/drivers/scsi/mvsas/mv_init.c b/drivers/scsi/mvsas/mv_init.c
>> index 4e9af66..052861b 100644
>> --- a/drivers/scsi/mvsas/mv_init.c
>> +++ b/drivers/scsi/mvsas/mv_init.c
>> @@ -59,7 +59,7 @@ static struct scsi_host_template mvs_sht = {
>> .name = DRV_NAME,
>> .queuecommand = sas_queuecommand,
>> .target_alloc = sas_target_alloc,
>> - .slave_configure = mvs_slave_configure,
>> + .slave_configure = sas_slave_configure,
>> .slave_destroy = sas_slave_destroy,
>> .scan_finished = mvs_scan_finished,
>> .scan_start = mvs_scan_start,
>> @@ -74,7 +74,7 @@ static struct scsi_host_template mvs_sht = {
>> .use_clustering = ENABLE_CLUSTERING,
>> .eh_device_reset_handler = sas_eh_device_reset_handler,
>> .eh_bus_reset_handler = sas_eh_bus_reset_handler,
>> - .slave_alloc = mvs_slave_alloc,
>> + .slave_alloc = sas_slave_alloc,
>> .target_destroy = sas_target_destroy,
>> -int mvs_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)) {
>> - sas_change_queue_depth(sdev,
>> - MVS_QUEUE_SIZE,
>> - SCSI_QDEPTH_DEFAULT);
>> - }
>> - return 0;
> Can you don't remove mvs_slave_configure function? Someday it will be useful, such as fixing some issue.
That's exactly the point of the patch. The isci driver encountered a
bug that the mvsas driver had fixed in its private mvs_slave_alloc()
routine. If I had taken the same approach and "fixed" it with a
isci_slave_alloc() routine that would still leave aic94xx or any other
future libsas driver to rediscover the fix.
If you later find a problem with sas_slave_configure() there's a good
chance the fix will be applicable to all libsas drivers. But if it is
truly a mvsas specific issue you can always re-introduce
mvs_slave_configure().
--
Dan
^ permalink raw reply [flat|nested] 21+ messages in thread
* RE: [PATCH 4/5] mvsas: remove mvs_slave_{alloc|configure}
2011-09-22 3:17 ` Williams, Dan J
@ 2011-09-22 3:19 ` Xiangliang Yu
0 siblings, 0 replies; 21+ messages in thread
From: Xiangliang Yu @ 2011-09-22 3:19 UTC (permalink / raw)
To: Williams, Dan J; +Cc: linux-ide, linux-scsi, jgarzik, JBottomley
> Subject: Re: [PATCH 4/5] mvsas: remove mvs_slave_{alloc|configure}
>
> On Wed, Sep 21, 2011 at 7:26 PM, Xiangliang Yu <yuxiangl@marvell.com> wrote:
> >
> >> Subject: [PATCH 4/5] mvsas: remove mvs_slave_{alloc|configure}
> >>
> >> libsas now handles:
> >> 1/ limiting ata scanning to lun0
> >> 2/ maximizing the queue_depth of sas devices (up to 256, mvsas only
> >> supports 64)
> >> 3/ changes to /sys/block/<sdX>/device/queue_depth for ata devices
> >>
> >> Cc: Xiangliang Yu <yuxiangl@marvell.com>
> >> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> >> ---
> >> drivers/scsi/mvsas/mv_init.c | 4 ++--
> >> drivers/scsi/mvsas/mv_sas.c | 30 ------------------------------
> >> drivers/scsi/mvsas/mv_sas.h | 2 --
> >> 3 files changed, 2 insertions(+), 34 deletions(-)
> >>
> >> diff --git a/drivers/scsi/mvsas/mv_init.c b/drivers/scsi/mvsas/mv_init.c
> >> index 4e9af66..052861b 100644
> >> --- a/drivers/scsi/mvsas/mv_init.c
> >> +++ b/drivers/scsi/mvsas/mv_init.c
> >> @@ -59,7 +59,7 @@ static struct scsi_host_template mvs_sht = {
> >> .name = DRV_NAME,
> >> .queuecommand = sas_queuecommand,
> >> .target_alloc = sas_target_alloc,
> >> - .slave_configure = mvs_slave_configure,
> >> + .slave_configure = sas_slave_configure,
> >> .slave_destroy = sas_slave_destroy,
> >> .scan_finished = mvs_scan_finished,
> >> .scan_start = mvs_scan_start,
> >> @@ -74,7 +74,7 @@ static struct scsi_host_template mvs_sht = {
> >> .use_clustering = ENABLE_CLUSTERING,
> >> .eh_device_reset_handler = sas_eh_device_reset_handler,
> >> .eh_bus_reset_handler = sas_eh_bus_reset_handler,
> >> - .slave_alloc = mvs_slave_alloc,
> >> + .slave_alloc = sas_slave_alloc,
> >> .target_destroy = sas_target_destroy,
> >> -int mvs_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)) {
> >> - sas_change_queue_depth(sdev,
> >> - MVS_QUEUE_SIZE,
> >> - SCSI_QDEPTH_DEFAULT);
> >> - }
> >> - return 0;
> > Can you don't remove mvs_slave_configure function? Someday it will be useful,
> such as fixing some issue.
>
> That's exactly the point of the patch. The isci driver encountered a
> bug that the mvsas driver had fixed in its private mvs_slave_alloc()
> routine. If I had taken the same approach and "fixed" it with a
> isci_slave_alloc() routine that would still leave aic94xx or any other
> future libsas driver to rediscover the fix.
>
> If you later find a problem with sas_slave_configure() there's a good
> chance the fix will be applicable to all libsas drivers. But if it is
> truly a mvsas specific issue you can always re-introduce
> mvs_slave_configure().
Sound good.
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2011-09-22 3:25 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-09-20 22:10 [PATCH 0/5] libsas fixes discovered via isci-atapi enabling effort Dan Williams
2011-09-20 22:10 ` [PATCH 1/5] libsas: disable scanning lun > 0 on ata devices Dan Williams
2011-09-20 22:15 ` Christoph Hellwig
2011-09-21 12:28 ` Mark Salyzyn
2011-09-22 2:29 ` Jeff Garzik
2011-09-20 22:10 ` [PATCH 2/5] libsas: fix ->change_queue_{depth|type} for sata devices Dan Williams
2011-09-20 22:16 ` Christoph Hellwig
2011-09-21 12:27 ` Mark Salyzyn
2011-09-22 2:30 ` Jeff Garzik
2011-09-20 22:10 ` [RFC PATCH 3/5] libsas: dynamic queue depth Dan Williams
2011-09-20 22:18 ` Christoph Hellwig
2011-09-21 1:17 ` Jack Wang
2011-09-21 12:37 ` Mark Salyzyn
2011-09-22 1:38 ` Jack Wang
2011-09-20 22:10 ` [PATCH 4/5] mvsas: remove mvs_slave_{alloc|configure} Dan Williams
2011-09-22 2:26 ` Xiangliang Yu
2011-09-22 2:31 ` Jeff Garzik
2011-09-22 3:17 ` Williams, Dan J
2011-09-22 3:19 ` Xiangliang Yu
2011-09-20 22:11 ` [PATCH 5/5] pm8001: remove pm8001_slave_{alloc|configure} Dan Williams
2011-09-21 1:13 ` [PATCH 0/5] libsas fixes discovered via isci-atapi enabling effort Jack Wang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.