* [PATCH 2/4] libata: move cdb_len for host to device
2006-02-12 14:32 [PATCHSET] libata: make some configurations per-dev Tejun Heo
@ 2006-02-12 14:32 ` Tejun Heo
2006-02-12 14:32 ` [PATCH 1/4] libata: make ata_dev_knobble() per-device Tejun Heo
` (2 subsequent siblings)
3 siblings, 0 replies; 24+ messages in thread
From: Tejun Heo @ 2006-02-12 14:32 UTC (permalink / raw)
To: jgarzik, albertcc, linux-ide; +Cc: Tejun Heo
cdb_len is per-device property. Sharing cdb_len on ap results in
inaccurate configuration on revalidation and hotplugging. This patch
makes cdb_len per-device.
Signed-off-by: Tejun Heo <htejun@gmail.com>
---
drivers/scsi/ahci.c | 3 ++-
drivers/scsi/libata-core.c | 19 ++++++++++++-------
drivers/scsi/libata-scsi.c | 4 ++--
drivers/scsi/sata_sil24.c | 4 ++--
include/linux/libata.h | 2 +-
5 files changed, 19 insertions(+), 13 deletions(-)
3adaa59ad21ad95f5bad4410b8d4334a3f703185
diff --git a/drivers/scsi/ahci.c b/drivers/scsi/ahci.c
index 24a54a5..23caa0c 100644
--- a/drivers/scsi/ahci.c
+++ b/drivers/scsi/ahci.c
@@ -617,7 +617,8 @@ static void ahci_qc_prep(struct ata_queu
ata_tf_to_fis(&qc->tf, pp->cmd_tbl, 0);
if (is_atapi) {
memset(pp->cmd_tbl + AHCI_CMD_TBL_CDB, 0, 32);
- memcpy(pp->cmd_tbl + AHCI_CMD_TBL_CDB, qc->cdb, ap->cdb_len);
+ memcpy(pp->cmd_tbl + AHCI_CMD_TBL_CDB, qc->cdb,
+ qc->dev->cdb_len);
}
n_elem = 0;
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index 01d0ca8..c62798c 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -931,7 +931,7 @@ static void ata_dev_identify(struct ata_
unsigned int using_edd;
struct ata_taskfile tf;
unsigned int err_mask;
- int rc;
+ int i, rc;
if (!ata_dev_present(dev)) {
DPRINTK("ENTER/EXIT (host %u, dev %u) -- nodev\n",
@@ -1087,7 +1087,7 @@ retry:
}
- ap->host->max_cmd_len = 16;
+ dev->cdb_len = 16;
}
/* ATAPI-specific feature tests */
@@ -1100,8 +1100,7 @@ retry:
printk(KERN_WARNING "ata%u: unsupported CDB len\n", ap->id);
goto err_out_nosup;
}
- ap->cdb_len = (unsigned int) rc;
- ap->host->max_cmd_len = (unsigned char) ap->cdb_len;
+ dev->cdb_len = (unsigned int) rc;
/* print device info to dmesg */
printk(KERN_INFO "ata%u: dev %u ATAPI, max %s\n",
@@ -1109,6 +1108,12 @@ retry:
ata_mode_string(xfer_modes));
}
+ ap->host->max_cmd_len = 0;
+ for (i = 0; i < ATA_MAX_DEVICES; i++)
+ ap->host->max_cmd_len = max_t(unsigned int,
+ ap->host->max_cmd_len,
+ ap->device[i].cdb_len);
+
DPRINTK("EXIT, drv_stat = 0x%x\n", ata_chk_status(ap));
return;
@@ -4200,7 +4205,7 @@ static void atapi_packet_task(void *_dat
/* send SCSI cdb */
DPRINTK("send cdb\n");
- WARN_ON(ap->cdb_len < 12);
+ WARN_ON(qc->dev->cdb_len < 12);
if (qc->tf.protocol == ATA_PROT_ATAPI_DMA ||
qc->tf.protocol == ATA_PROT_ATAPI_NODATA) {
@@ -4214,12 +4219,12 @@ static void atapi_packet_task(void *_dat
*/
spin_lock_irqsave(&ap->host_set->lock, flags);
ap->flags &= ~ATA_FLAG_NOINTR;
- ata_data_xfer(ap, qc->cdb, ap->cdb_len, 1);
+ ata_data_xfer(ap, qc->cdb, qc->dev->cdb_len, 1);
if (qc->tf.protocol == ATA_PROT_ATAPI_DMA)
ap->ops->bmdma_start(qc); /* initiate bmdma */
spin_unlock_irqrestore(&ap->host_set->lock, flags);
} else {
- ata_data_xfer(ap, qc->cdb, ap->cdb_len, 1);
+ ata_data_xfer(ap, qc->cdb, qc->dev->cdb_len, 1);
/* PIO commands are handled by polling */
ap->hsm_task_state = HSM_ST;
diff --git a/drivers/scsi/libata-scsi.c b/drivers/scsi/libata-scsi.c
index ebd7cf4..3628fed 100644
--- a/drivers/scsi/libata-scsi.c
+++ b/drivers/scsi/libata-scsi.c
@@ -2146,7 +2146,7 @@ static void atapi_request_sense(struct a
ata_sg_init_one(qc, cmd->sense_buffer, sizeof(cmd->sense_buffer));
qc->dma_dir = DMA_FROM_DEVICE;
- memset(&qc->cdb, 0, ap->cdb_len);
+ memset(&qc->cdb, 0, qc->dev->cdb_len);
qc->cdb[0] = REQUEST_SENSE;
qc->cdb[4] = SCSI_SENSE_BUFFERSIZE;
@@ -2248,7 +2248,7 @@ static unsigned int atapi_xlat(struct at
if (ata_check_atapi_dma(qc))
using_pio = 1;
- memcpy(&qc->cdb, scsicmd, qc->ap->cdb_len);
+ memcpy(&qc->cdb, scsicmd, dev->cdb_len);
qc->complete_fn = atapi_qc_complete;
diff --git a/drivers/scsi/sata_sil24.c b/drivers/scsi/sata_sil24.c
index 228a7fa..24020c6 100644
--- a/drivers/scsi/sata_sil24.c
+++ b/drivers/scsi/sata_sil24.c
@@ -371,7 +371,7 @@ static void sil24_dev_config(struct ata_
{
void __iomem *port = (void __iomem *)ap->ioaddr.cmd_addr;
- if (ap->cdb_len == 16)
+ if (dev->cdb_len == 16)
writel(PORT_CS_CDB16, port + PORT_CTRL_STAT);
else
writel(PORT_CS_CDB16, port + PORT_CTRL_CLR);
@@ -543,7 +543,7 @@ static void sil24_qc_prep(struct ata_que
prb = &cb->atapi.prb;
sge = cb->atapi.sge;
memset(cb->atapi.cdb, 0, 32);
- memcpy(cb->atapi.cdb, qc->cdb, ap->cdb_len);
+ memcpy(cb->atapi.cdb, qc->cdb, qc->dev->cdb_len);
if (qc->tf.protocol != ATA_PROT_ATAPI_NODATA) {
if (qc->tf.flags & ATA_TFLAG_WRITE)
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 0853032..853c988 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -348,6 +348,7 @@ struct ata_device {
unsigned int multi_count; /* sectors count for
READ/WRITE MULTIPLE */
+ unsigned int cdb_len;
/* for CHS addressing */
u16 cylinders; /* Number of cylinders */
@@ -377,7 +378,6 @@ struct ata_port {
unsigned int mwdma_mask;
unsigned int udma_mask;
unsigned int cbl; /* cable type; ATA_CBL_xxx */
- unsigned int cdb_len;
struct ata_device device[ATA_MAX_DEVICES];
--
1.1.5
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 1/4] libata: make ata_dev_knobble() per-device
2006-02-12 14:32 [PATCHSET] libata: make some configurations per-dev Tejun Heo
2006-02-12 14:32 ` [PATCH 2/4] libata: move cdb_len for host to device Tejun Heo
@ 2006-02-12 14:32 ` Tejun Heo
2006-02-12 19:29 ` Jeff Garzik
2006-02-12 14:32 ` [PATCH 4/4] libata: kill sht->max_sectors Tejun Heo
2006-02-12 14:32 ` [PATCH 3/4] libata: add per-device max_sectors Tejun Heo
3 siblings, 1 reply; 24+ messages in thread
From: Tejun Heo @ 2006-02-12 14:32 UTC (permalink / raw)
To: jgarzik, albertcc, linux-ide; +Cc: Tejun Heo
ata_dev_knobble() unconditionally used the first device of the port to
determine whether a device is bridged or not. This causes bridge
limit to be incorrectly applied or unapplied for hosts with slave
devices (e.g. ata_piix).
Signed-off-by: Tejun Heo <htejun@gmail.com>
---
drivers/scsi/libata-core.c | 9 +++++----
1 files changed, 5 insertions(+), 4 deletions(-)
f6a90d153c45a62fb4fdd3a3c575e8163182d675
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index 7a88e91..01d0ca8 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -1121,9 +1121,10 @@ err_out:
}
-static inline u8 ata_dev_knobble(const struct ata_port *ap)
+static inline u8 ata_dev_knobble(const struct ata_port *ap,
+ struct ata_device *dev)
{
- return ((ap->cbl == ATA_CBL_SATA) && (!ata_id_is_sata(ap->device->id)));
+ return ((ap->cbl == ATA_CBL_SATA) && (!ata_id_is_sata(dev->id)));
}
/**
@@ -1137,9 +1138,9 @@ static inline u8 ata_dev_knobble(const s
void ata_dev_config(struct ata_port *ap, unsigned int i)
{
/* limit bridge transfers to udma5, 200 sectors */
- if (ata_dev_knobble(ap)) {
+ if (ata_dev_knobble(ap, &ap->device[i])) {
printk(KERN_INFO "ata%u(%u): applying bridge limits\n",
- ap->id, ap->device->devno);
+ ap->id, i);
ap->udma_mask &= ATA_UDMA5;
ap->host->max_sectors = ATA_MAX_SECTORS;
ap->host->hostt->max_sectors = ATA_MAX_SECTORS;
--
1.1.5
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH 4/4] libata: kill sht->max_sectors
2006-02-12 14:32 [PATCHSET] libata: make some configurations per-dev Tejun Heo
2006-02-12 14:32 ` [PATCH 2/4] libata: move cdb_len for host to device Tejun Heo
2006-02-12 14:32 ` [PATCH 1/4] libata: make ata_dev_knobble() per-device Tejun Heo
@ 2006-02-12 14:32 ` Tejun Heo
2006-02-12 14:32 ` [PATCH 3/4] libata: add per-device max_sectors Tejun Heo
3 siblings, 0 replies; 24+ messages in thread
From: Tejun Heo @ 2006-02-12 14:32 UTC (permalink / raw)
To: jgarzik, albertcc, linux-ide; +Cc: Tejun Heo
The previous dev->max_sectors patch made sht->max_sectors meaningless.
Kill all initializations of sht->max_sectors.
Signed-off-by: Tejun Heo <htejun@gmail.com>
---
drivers/scsi/ahci.c | 1 -
drivers/scsi/ata_piix.c | 1 -
drivers/scsi/pdc_adma.c | 1 -
drivers/scsi/sata_mv.c | 1 -
drivers/scsi/sata_nv.c | 1 -
drivers/scsi/sata_promise.c | 1 -
drivers/scsi/sata_qstor.c | 1 -
drivers/scsi/sata_sil.c | 1 -
drivers/scsi/sata_sil24.c | 1 -
drivers/scsi/sata_sis.c | 1 -
drivers/scsi/sata_svw.c | 1 -
drivers/scsi/sata_sx4.c | 1 -
drivers/scsi/sata_uli.c | 1 -
drivers/scsi/sata_via.c | 1 -
drivers/scsi/sata_vsc.c | 1 -
15 files changed, 0 insertions(+), 15 deletions(-)
05115b54cfe2458ef077e1168533bc14f102fdb3
diff --git a/drivers/scsi/ahci.c b/drivers/scsi/ahci.c
index 23caa0c..1c2ab3d 100644
--- a/drivers/scsi/ahci.c
+++ b/drivers/scsi/ahci.c
@@ -211,7 +211,6 @@ static struct scsi_host_template ahci_sh
.can_queue = ATA_DEF_QUEUE,
.this_id = ATA_SHT_THIS_ID,
.sg_tablesize = AHCI_MAX_SG,
- .max_sectors = ATA_MAX_SECTORS,
.cmd_per_lun = ATA_SHT_CMD_PER_LUN,
.emulated = ATA_SHT_EMULATED,
.use_clustering = AHCI_USE_CLUSTERING,
diff --git a/drivers/scsi/ata_piix.c b/drivers/scsi/ata_piix.c
index 4933ba2..289be58 100644
--- a/drivers/scsi/ata_piix.c
+++ b/drivers/scsi/ata_piix.c
@@ -185,7 +185,6 @@ static struct scsi_host_template piix_sh
.can_queue = ATA_DEF_QUEUE,
.this_id = ATA_SHT_THIS_ID,
.sg_tablesize = LIBATA_MAX_PRD,
- .max_sectors = ATA_MAX_SECTORS,
.cmd_per_lun = ATA_SHT_CMD_PER_LUN,
.emulated = ATA_SHT_EMULATED,
.use_clustering = ATA_SHT_USE_CLUSTERING,
diff --git a/drivers/scsi/pdc_adma.c b/drivers/scsi/pdc_adma.c
index d0ad3eb..5f33cc9 100644
--- a/drivers/scsi/pdc_adma.c
+++ b/drivers/scsi/pdc_adma.c
@@ -148,7 +148,6 @@ static struct scsi_host_template adma_at
.can_queue = ATA_DEF_QUEUE,
.this_id = ATA_SHT_THIS_ID,
.sg_tablesize = LIBATA_MAX_PRD,
- .max_sectors = ATA_MAX_SECTORS,
.cmd_per_lun = ATA_SHT_CMD_PER_LUN,
.emulated = ATA_SHT_EMULATED,
.use_clustering = ENABLE_CLUSTERING,
diff --git a/drivers/scsi/sata_mv.c b/drivers/scsi/sata_mv.c
index 732cb64..c158de2 100644
--- a/drivers/scsi/sata_mv.c
+++ b/drivers/scsi/sata_mv.c
@@ -383,7 +383,6 @@ static struct scsi_host_template mv_sht
.can_queue = MV_USE_Q_DEPTH,
.this_id = ATA_SHT_THIS_ID,
.sg_tablesize = MV_MAX_SG_CT / 2,
- .max_sectors = ATA_MAX_SECTORS,
.cmd_per_lun = ATA_SHT_CMD_PER_LUN,
.emulated = ATA_SHT_EMULATED,
.use_clustering = ATA_SHT_USE_CLUSTERING,
diff --git a/drivers/scsi/sata_nv.c b/drivers/scsi/sata_nv.c
index cdfeb9a..caffadc 100644
--- a/drivers/scsi/sata_nv.c
+++ b/drivers/scsi/sata_nv.c
@@ -234,7 +234,6 @@ static struct scsi_host_template nv_sht
.can_queue = ATA_DEF_QUEUE,
.this_id = ATA_SHT_THIS_ID,
.sg_tablesize = LIBATA_MAX_PRD,
- .max_sectors = ATA_MAX_SECTORS,
.cmd_per_lun = ATA_SHT_CMD_PER_LUN,
.emulated = ATA_SHT_EMULATED,
.use_clustering = ATA_SHT_USE_CLUSTERING,
diff --git a/drivers/scsi/sata_promise.c b/drivers/scsi/sata_promise.c
index c9dfd93..ba2b7a0 100644
--- a/drivers/scsi/sata_promise.c
+++ b/drivers/scsi/sata_promise.c
@@ -116,7 +116,6 @@ static struct scsi_host_template pdc_ata
.can_queue = ATA_DEF_QUEUE,
.this_id = ATA_SHT_THIS_ID,
.sg_tablesize = LIBATA_MAX_PRD,
- .max_sectors = ATA_MAX_SECTORS,
.cmd_per_lun = ATA_SHT_CMD_PER_LUN,
.emulated = ATA_SHT_EMULATED,
.use_clustering = ATA_SHT_USE_CLUSTERING,
diff --git a/drivers/scsi/sata_qstor.c b/drivers/scsi/sata_qstor.c
index 955131b..2864826 100644
--- a/drivers/scsi/sata_qstor.c
+++ b/drivers/scsi/sata_qstor.c
@@ -137,7 +137,6 @@ static struct scsi_host_template qs_ata_
.can_queue = ATA_DEF_QUEUE,
.this_id = ATA_SHT_THIS_ID,
.sg_tablesize = QS_MAX_PRD,
- .max_sectors = ATA_MAX_SECTORS,
.cmd_per_lun = ATA_SHT_CMD_PER_LUN,
.emulated = ATA_SHT_EMULATED,
//FIXME .use_clustering = ATA_SHT_USE_CLUSTERING,
diff --git a/drivers/scsi/sata_sil.c b/drivers/scsi/sata_sil.c
index 6c482c8..1534688 100644
--- a/drivers/scsi/sata_sil.c
+++ b/drivers/scsi/sata_sil.c
@@ -140,7 +140,6 @@ static struct scsi_host_template sil_sht
.can_queue = ATA_DEF_QUEUE,
.this_id = ATA_SHT_THIS_ID,
.sg_tablesize = LIBATA_MAX_PRD,
- .max_sectors = ATA_MAX_SECTORS,
.cmd_per_lun = ATA_SHT_CMD_PER_LUN,
.emulated = ATA_SHT_EMULATED,
.use_clustering = ATA_SHT_USE_CLUSTERING,
diff --git a/drivers/scsi/sata_sil24.c b/drivers/scsi/sata_sil24.c
index 24020c6..a0e35a2 100644
--- a/drivers/scsi/sata_sil24.c
+++ b/drivers/scsi/sata_sil24.c
@@ -285,7 +285,6 @@ static struct scsi_host_template sil24_s
.can_queue = ATA_DEF_QUEUE,
.this_id = ATA_SHT_THIS_ID,
.sg_tablesize = LIBATA_MAX_PRD,
- .max_sectors = ATA_MAX_SECTORS,
.cmd_per_lun = ATA_SHT_CMD_PER_LUN,
.emulated = ATA_SHT_EMULATED,
.use_clustering = ATA_SHT_USE_CLUSTERING,
diff --git a/drivers/scsi/sata_sis.c b/drivers/scsi/sata_sis.c
index 2f18157..7fd45f8 100644
--- a/drivers/scsi/sata_sis.c
+++ b/drivers/scsi/sata_sis.c
@@ -92,7 +92,6 @@ static struct scsi_host_template sis_sht
.can_queue = ATA_DEF_QUEUE,
.this_id = ATA_SHT_THIS_ID,
.sg_tablesize = ATA_MAX_PRD,
- .max_sectors = ATA_MAX_SECTORS,
.cmd_per_lun = ATA_SHT_CMD_PER_LUN,
.emulated = ATA_SHT_EMULATED,
.use_clustering = ATA_SHT_USE_CLUSTERING,
diff --git a/drivers/scsi/sata_svw.c b/drivers/scsi/sata_svw.c
index f369c30..4aaccd5 100644
--- a/drivers/scsi/sata_svw.c
+++ b/drivers/scsi/sata_svw.c
@@ -293,7 +293,6 @@ static struct scsi_host_template k2_sata
.can_queue = ATA_DEF_QUEUE,
.this_id = ATA_SHT_THIS_ID,
.sg_tablesize = LIBATA_MAX_PRD,
- .max_sectors = ATA_MAX_SECTORS,
.cmd_per_lun = ATA_SHT_CMD_PER_LUN,
.emulated = ATA_SHT_EMULATED,
.use_clustering = ATA_SHT_USE_CLUSTERING,
diff --git a/drivers/scsi/sata_sx4.c b/drivers/scsi/sata_sx4.c
index e158f7a..9f8a768 100644
--- a/drivers/scsi/sata_sx4.c
+++ b/drivers/scsi/sata_sx4.c
@@ -187,7 +187,6 @@ static struct scsi_host_template pdc_sat
.can_queue = ATA_DEF_QUEUE,
.this_id = ATA_SHT_THIS_ID,
.sg_tablesize = LIBATA_MAX_PRD,
- .max_sectors = ATA_MAX_SECTORS,
.cmd_per_lun = ATA_SHT_CMD_PER_LUN,
.emulated = ATA_SHT_EMULATED,
.use_clustering = ATA_SHT_USE_CLUSTERING,
diff --git a/drivers/scsi/sata_uli.c b/drivers/scsi/sata_uli.c
index c500f24..37a487b 100644
--- a/drivers/scsi/sata_uli.c
+++ b/drivers/scsi/sata_uli.c
@@ -80,7 +80,6 @@ static struct scsi_host_template uli_sht
.can_queue = ATA_DEF_QUEUE,
.this_id = ATA_SHT_THIS_ID,
.sg_tablesize = LIBATA_MAX_PRD,
- .max_sectors = ATA_MAX_SECTORS,
.cmd_per_lun = ATA_SHT_CMD_PER_LUN,
.emulated = ATA_SHT_EMULATED,
.use_clustering = ATA_SHT_USE_CLUSTERING,
diff --git a/drivers/scsi/sata_via.c b/drivers/scsi/sata_via.c
index 2e20887..ff65a0b 100644
--- a/drivers/scsi/sata_via.c
+++ b/drivers/scsi/sata_via.c
@@ -99,7 +99,6 @@ static struct scsi_host_template svia_sh
.can_queue = ATA_DEF_QUEUE,
.this_id = ATA_SHT_THIS_ID,
.sg_tablesize = LIBATA_MAX_PRD,
- .max_sectors = ATA_MAX_SECTORS,
.cmd_per_lun = ATA_SHT_CMD_PER_LUN,
.emulated = ATA_SHT_EMULATED,
.use_clustering = ATA_SHT_USE_CLUSTERING,
diff --git a/drivers/scsi/sata_vsc.c b/drivers/scsi/sata_vsc.c
index cf1f8a6..e124db8 100644
--- a/drivers/scsi/sata_vsc.c
+++ b/drivers/scsi/sata_vsc.c
@@ -228,7 +228,6 @@ static struct scsi_host_template vsc_sat
.can_queue = ATA_DEF_QUEUE,
.this_id = ATA_SHT_THIS_ID,
.sg_tablesize = LIBATA_MAX_PRD,
- .max_sectors = ATA_MAX_SECTORS,
.cmd_per_lun = ATA_SHT_CMD_PER_LUN,
.emulated = ATA_SHT_EMULATED,
.use_clustering = ATA_SHT_USE_CLUSTERING,
--
1.1.5
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 3/4] libata: add per-device max_sectors
2006-02-12 14:32 [PATCHSET] libata: make some configurations per-dev Tejun Heo
` (2 preceding siblings ...)
2006-02-12 14:32 ` [PATCH 4/4] libata: kill sht->max_sectors Tejun Heo
@ 2006-02-12 14:32 ` Tejun Heo
2006-02-12 19:37 ` Jeff Garzik
3 siblings, 1 reply; 24+ messages in thread
From: Tejun Heo @ 2006-02-12 14:32 UTC (permalink / raw)
To: jgarzik, albertcc, linux-ide; +Cc: Tejun Heo
If a low level driver wants to control max_sectors, it had to adjust
ap->host->max_sectors and set ATA_DFLAG_LOCK_SECTORS to tell
ata_scsi_slave_config not to override the limit. This is not only
cumbersome but also incorrect for hosts which support more than one
devices per port.
This patch adds per-device ->max_sectors. If the field is unset
(zero), libata core layer will adjust ->max_sectors according to
default rules. If the field is set, libata honors the setting.
Signed-off-by: Tejun Heo <htejun@gmail.com>
---
drivers/scsi/libata-core.c | 4 +---
drivers/scsi/libata-scsi.c | 18 +++++++++---------
drivers/scsi/sata_sil.c | 4 +---
include/linux/libata.h | 4 ++--
4 files changed, 13 insertions(+), 17 deletions(-)
43ec393ac52f72d2a45f93ee2abf5255bed1d8db
diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index c62798c..4db3db0 100644
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -1147,9 +1147,7 @@ void ata_dev_config(struct ata_port *ap,
printk(KERN_INFO "ata%u(%u): applying bridge limits\n",
ap->id, i);
ap->udma_mask &= ATA_UDMA5;
- ap->host->max_sectors = ATA_MAX_SECTORS;
- ap->host->hostt->max_sectors = ATA_MAX_SECTORS;
- ap->device[i].flags |= ATA_DFLAG_LOCK_SECTORS;
+ ap->device[i].max_sectors = ATA_MAX_SECTORS;
}
if (ap->ops->dev_config)
diff --git a/drivers/scsi/libata-scsi.c b/drivers/scsi/libata-scsi.c
index 3628fed..86da465 100644
--- a/drivers/scsi/libata-scsi.c
+++ b/drivers/scsi/libata-scsi.c
@@ -684,23 +684,23 @@ int ata_scsi_slave_config(struct scsi_de
if (sdev->id < ATA_MAX_DEVICES) {
struct ata_port *ap;
struct ata_device *dev;
+ unsigned int max_sectors;
ap = (struct ata_port *) &sdev->host->hostdata[0];
dev = &ap->device[sdev->id];
- /* TODO: 1024 is an arbitrary number, not the
+ /* TODO: 2048 is an arbitrary number, not the
* hardware maximum. This should be increased to
* 65534 when Jens Axboe's patch for dynamically
* determining max_sectors is merged.
*/
- if ((dev->flags & ATA_DFLAG_LBA48) &&
- ((dev->flags & ATA_DFLAG_LOCK_SECTORS) == 0)) {
- /*
- * do not overwrite sdev->host->max_sectors, since
- * other drives on this host may not support LBA48
- */
- blk_queue_max_sectors(sdev->request_queue, 2048);
- }
+ max_sectors = ATA_MAX_SECTORS;
+ if (dev->flags & ATA_DFLAG_LBA48)
+ max_sectors = 2048;
+ if (dev->max_sectors)
+ max_sectors = dev->max_sectors;
+
+ blk_queue_max_sectors(sdev->request_queue, max_sectors);
/*
* SATA DMA transfers must be multiples of 4 byte, so
diff --git a/drivers/scsi/sata_sil.c b/drivers/scsi/sata_sil.c
index 61c4ac7..6c482c8 100644
--- a/drivers/scsi/sata_sil.c
+++ b/drivers/scsi/sata_sil.c
@@ -354,9 +354,7 @@ static void sil_dev_config(struct ata_po
(quirks & SIL_QUIRK_MOD15WRITE))) {
printk(KERN_INFO "ata%u(%u): applying Seagate errata fix (mod15write workaround)\n",
ap->id, dev->devno);
- ap->host->max_sectors = 15;
- ap->host->hostt->max_sectors = 15;
- dev->flags |= ATA_DFLAG_LOCK_SECTORS;
+ dev->max_sectors = 15;
return;
}
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 853c988..afe4645 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -122,8 +122,7 @@ enum {
/* struct ata_device stuff */
ATA_DFLAG_LBA48 = (1 << 0), /* device supports LBA48 */
ATA_DFLAG_PIO = (1 << 1), /* device currently in PIO mode */
- ATA_DFLAG_LOCK_SECTORS = (1 << 2), /* don't adjust max_sectors */
- ATA_DFLAG_LBA = (1 << 3), /* device supports LBA */
+ ATA_DFLAG_LBA = (1 << 2), /* device supports LBA */
ATA_DEV_UNKNOWN = 0, /* unknown device */
ATA_DEV_ATA = 1, /* ATA device */
@@ -348,6 +347,7 @@ struct ata_device {
unsigned int multi_count; /* sectors count for
READ/WRITE MULTIPLE */
+ unsigned int max_sectors; /* per-device max sectors */
unsigned int cdb_len;
/* for CHS addressing */
--
1.1.5
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH 3/4] libata: add per-device max_sectors
2006-02-12 14:32 ` [PATCH 3/4] libata: add per-device max_sectors Tejun Heo
@ 2006-02-12 19:37 ` Jeff Garzik
2006-02-13 0:47 ` Tejun Heo
0 siblings, 1 reply; 24+ messages in thread
From: Jeff Garzik @ 2006-02-12 19:37 UTC (permalink / raw)
To: Tejun Heo; +Cc: albertcc, linux-ide
Tejun Heo wrote:
> If a low level driver wants to control max_sectors, it had to adjust
> ap->host->max_sectors and set ATA_DFLAG_LOCK_SECTORS to tell
> ata_scsi_slave_config not to override the limit. This is not only
> cumbersome but also incorrect for hosts which support more than one
> devices per port.
>
> This patch adds per-device ->max_sectors. If the field is unset
> (zero), libata core layer will adjust ->max_sectors according to
> default rules. If the field is set, libata honors the setting.
>
> Signed-off-by: Tejun Heo <htejun@gmail.com>
>
> ---
>
> drivers/scsi/libata-core.c | 4 +---
> drivers/scsi/libata-scsi.c | 18 +++++++++---------
> drivers/scsi/sata_sil.c | 4 +---
> include/linux/libata.h | 4 ++--
> 4 files changed, 13 insertions(+), 17 deletions(-)
>
> 43ec393ac52f72d2a45f93ee2abf5255bed1d8db
> diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
> index c62798c..4db3db0 100644
> --- a/drivers/scsi/libata-core.c
> +++ b/drivers/scsi/libata-core.c
> @@ -1147,9 +1147,7 @@ void ata_dev_config(struct ata_port *ap,
> printk(KERN_INFO "ata%u(%u): applying bridge limits\n",
> ap->id, i);
> ap->udma_mask &= ATA_UDMA5;
> - ap->host->max_sectors = ATA_MAX_SECTORS;
> - ap->host->hostt->max_sectors = ATA_MAX_SECTORS;
> - ap->device[i].flags |= ATA_DFLAG_LOCK_SECTORS;
> + ap->device[i].max_sectors = ATA_MAX_SECTORS;
> }
>
> if (ap->ops->dev_config)
> diff --git a/drivers/scsi/libata-scsi.c b/drivers/scsi/libata-scsi.c
> index 3628fed..86da465 100644
> --- a/drivers/scsi/libata-scsi.c
> +++ b/drivers/scsi/libata-scsi.c
> @@ -684,23 +684,23 @@ int ata_scsi_slave_config(struct scsi_de
> if (sdev->id < ATA_MAX_DEVICES) {
> struct ata_port *ap;
> struct ata_device *dev;
> + unsigned int max_sectors;
>
> ap = (struct ata_port *) &sdev->host->hostdata[0];
> dev = &ap->device[sdev->id];
>
> - /* TODO: 1024 is an arbitrary number, not the
> + /* TODO: 2048 is an arbitrary number, not the
> * hardware maximum. This should be increased to
> * 65534 when Jens Axboe's patch for dynamically
> * determining max_sectors is merged.
> */
> - if ((dev->flags & ATA_DFLAG_LBA48) &&
> - ((dev->flags & ATA_DFLAG_LOCK_SECTORS) == 0)) {
> - /*
> - * do not overwrite sdev->host->max_sectors, since
> - * other drives on this host may not support LBA48
> - */
> - blk_queue_max_sectors(sdev->request_queue, 2048);
> - }
> + max_sectors = ATA_MAX_SECTORS;
> + if (dev->flags & ATA_DFLAG_LBA48)
> + max_sectors = 2048;
> + if (dev->max_sectors)
> + max_sectors = dev->max_sectors;
applied 3-4, though note that the Axboe code mentioned in the comment is
now in the kernel, so lba48 can be fixed properly.
Jeff
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH 3/4] libata: add per-device max_sectors
2006-02-12 19:37 ` Jeff Garzik
@ 2006-02-13 0:47 ` Tejun Heo
2006-02-13 4:58 ` Jeff Garzik
0 siblings, 1 reply; 24+ messages in thread
From: Tejun Heo @ 2006-02-13 0:47 UTC (permalink / raw)
To: Jeff Garzik; +Cc: albertcc, linux-ide
Jeff Garzik wrote:
>
> applied 3-4, though note that the Axboe code mentioned in the comment is
> now in the kernel, so lba48 can be fixed properly.
>
Care to tell me which commit it was? I don't really understand the comment.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/4] libata: add per-device max_sectors
2006-02-13 0:47 ` Tejun Heo
@ 2006-02-13 4:58 ` Jeff Garzik
2006-02-15 7:24 ` [PATCH] libata: increase LBA48 max sectors to 65535 Tejun Heo
0 siblings, 1 reply; 24+ messages in thread
From: Jeff Garzik @ 2006-02-13 4:58 UTC (permalink / raw)
To: Tejun Heo; +Cc: albertcc, linux-ide
Tejun Heo wrote:
> Jeff Garzik wrote:
>
>>
>> applied 3-4, though note that the Axboe code mentioned in the comment
>> is now in the kernel, so lba48 can be fixed properly.
>>
>
> Care to tell me which commit it was? I don't really understand the
> comment.
max_sectors was artificially clipped by the driver due to a system
inadequacy: if it truly built commands of 65,536 sectors, then for a
full queue, I/O in flight would lock down way too much system memory.
Hence, 1024, 2048, 4096 is an arbitrary figure chosen to balance large
per-command data size (desireable) and pinned memory (minimize).
The solution was to separate max_sectors -- the maximum we should attach
to each command -- with max_hw_sectors, the absolute hardware limit.
The system will tune max_sectors up and down, within the max_hw_sectors
limit.
Search the kernel changelog for max_hw_sectors.
Jeff
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH] libata: increase LBA48 max sectors to 65535
2006-02-13 4:58 ` Jeff Garzik
@ 2006-02-15 7:24 ` Tejun Heo
2006-02-15 13:07 ` Jens Axboe
2006-02-20 21:59 ` Jeff Garzik
0 siblings, 2 replies; 24+ messages in thread
From: Tejun Heo @ 2006-02-15 7:24 UTC (permalink / raw)
To: Jeff Garzik; +Cc: albertcc, linux-ide
max_hw_sectors/max_sectors separation patch made into the tree,
increase max_sectors to its hardware limit.
Signed-off-by: Tejun Heo <htejun@gmail.com>
---
Jeff, first of all, thanks for the pointer. I have two more
questions.
* I make ATA_MAX_SECTORS_LBA48 65535 (the 0000h case is supposed to be
broken, right?); however, the comment says 65534. Should it be
65534?
* ATA_MAX_SECTORS is 200 w/ FIXME. Would bumping it up to 255 be
okay? Is this because some lldd's don't handle sectors above 200
very well?
Thanks.
diff --git a/drivers/scsi/libata-scsi.c b/drivers/scsi/libata-scsi.c
index 86da465..bceadb3 100644
--- a/drivers/scsi/libata-scsi.c
+++ b/drivers/scsi/libata-scsi.c
@@ -689,14 +689,9 @@ int ata_scsi_slave_config(struct scsi_de
ap = (struct ata_port *) &sdev->host->hostdata[0];
dev = &ap->device[sdev->id];
- /* TODO: 2048 is an arbitrary number, not the
- * hardware maximum. This should be increased to
- * 65534 when Jens Axboe's patch for dynamically
- * determining max_sectors is merged.
- */
max_sectors = ATA_MAX_SECTORS;
if (dev->flags & ATA_DFLAG_LBA48)
- max_sectors = 2048;
+ max_sectors = ATA_MAX_SECTORS_LBA48;
if (dev->max_sectors)
max_sectors = dev->max_sectors;
diff --git a/include/linux/libata.h b/include/linux/libata.h
index afe4645..6253650 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -110,6 +110,7 @@ enum {
ATA_DEF_QUEUE = 1,
ATA_MAX_QUEUE = 1,
ATA_MAX_SECTORS = 200, /* FIXME */
+ ATA_MAX_SECTORS_LBA48 = 65535,
ATA_MAX_BUS = 2,
ATA_DEF_BUSY_WAIT = 10000,
ATA_SHORT_PAUSE = (HZ >> 6) + 1,
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH] libata: increase LBA48 max sectors to 65535
2006-02-15 7:24 ` [PATCH] libata: increase LBA48 max sectors to 65535 Tejun Heo
@ 2006-02-15 13:07 ` Jens Axboe
2006-02-15 15:04 ` Tejun Heo
2006-02-20 21:59 ` Jeff Garzik
1 sibling, 1 reply; 24+ messages in thread
From: Jens Axboe @ 2006-02-15 13:07 UTC (permalink / raw)
To: Tejun Heo; +Cc: Jeff Garzik, albertcc, linux-ide
On Wed, Feb 15 2006, Tejun Heo wrote:
> max_hw_sectors/max_sectors separation patch made into the tree,
> increase max_sectors to its hardware limit.
>
> Signed-off-by: Tejun Heo <htejun@gmail.com>
>
> ---
>
> Jeff, first of all, thanks for the pointer. I have two more
> questions.
>
> * I make ATA_MAX_SECTORS_LBA48 65535 (the 0000h case is supposed to be
> broken, right?); however, the comment says 65534. Should it be
> 65534?
Since we never noticed any breakage on !sectors on lba28, I think we can
safely assume that !sectors will work fine for lba48 as well. So why not
just make it full 64k, eg 65536? ->max_hw_sectors is an unsigned int
now, so 64k wont overflow it.
--
Jens Axboe
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] libata: increase LBA48 max sectors to 65535
2006-02-15 13:07 ` Jens Axboe
@ 2006-02-15 15:04 ` Tejun Heo
2006-02-15 15:12 ` Jens Axboe
0 siblings, 1 reply; 24+ messages in thread
From: Tejun Heo @ 2006-02-15 15:04 UTC (permalink / raw)
To: Jens Axboe; +Cc: Jeff Garzik, albertcc, linux-ide
Jens Axboe wrote:
> On Wed, Feb 15 2006, Tejun Heo wrote:
>
>>max_hw_sectors/max_sectors separation patch made into the tree,
>>increase max_sectors to its hardware limit.
>>
>>Signed-off-by: Tejun Heo <htejun@gmail.com>
>>
>>---
>>
>>Jeff, first of all, thanks for the pointer. I have two more
>>questions.
>>
>>* I make ATA_MAX_SECTORS_LBA48 65535 (the 0000h case is supposed to be
>> broken, right?); however, the comment says 65534. Should it be
>> 65534?
>
>
> Since we never noticed any breakage on !sectors on lba28, I think we can
> safely assume that !sectors will work fine for lba48 as well. So why not
> just make it full 64k, eg 65536? ->max_hw_sectors is an unsigned int
> now, so 64k wont overflow it.
>
Hello, Jens.
libata currently sets max_sectors to 200 (ATA_MAX_SECTORS) on lba28.
Are you talking about IDE driver? IDE driver seems to set max_sectors
to 256 on probe and make it 2048 while setting up ide disk if lba48.
Hmmm... Can we really trust all the firmwares? I just feel that some
drive out there ought have screwed up about nsect == 00h/0000h case.
Thanks.
--
tejun
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] libata: increase LBA48 max sectors to 65535
2006-02-15 15:04 ` Tejun Heo
@ 2006-02-15 15:12 ` Jens Axboe
2006-02-15 15:30 ` Tejun Heo
` (2 more replies)
0 siblings, 3 replies; 24+ messages in thread
From: Jens Axboe @ 2006-02-15 15:12 UTC (permalink / raw)
To: Tejun Heo; +Cc: Jeff Garzik, albertcc, linux-ide
On Thu, Feb 16 2006, Tejun Heo wrote:
> Jens Axboe wrote:
> >On Wed, Feb 15 2006, Tejun Heo wrote:
> >
> >>max_hw_sectors/max_sectors separation patch made into the tree,
> >>increase max_sectors to its hardware limit.
> >>
> >>Signed-off-by: Tejun Heo <htejun@gmail.com>
> >>
> >>---
> >>
> >>Jeff, first of all, thanks for the pointer. I have two more
> >>questions.
> >>
> >>* I make ATA_MAX_SECTORS_LBA48 65535 (the 0000h case is supposed to be
> >> broken, right?); however, the comment says 65534. Should it be
> >> 65534?
> >
> >
> >Since we never noticed any breakage on !sectors on lba28, I think we can
> >safely assume that !sectors will work fine for lba48 as well. So why not
> >just make it full 64k, eg 65536? ->max_hw_sectors is an unsigned int
> >now, so 64k wont overflow it.
> >
>
> Hello, Jens.
>
> libata currently sets max_sectors to 200 (ATA_MAX_SECTORS) on lba28.
> Are you talking about IDE driver? IDE driver seems to set max_sectors
> to 256 on probe and make it 2048 while setting up ide disk if lba48.
Yes I mean the IDE driver. The 200 sector value in libata has always
struck me as being extremely odd and a very bad choice. It's somewhere
in between reasonable defaults, which isn't very nice. I'd suggest just
making it 256 as well, unless Jeff has a reason why it's set to 200.
> Hmmm... Can we really trust all the firmwares? I just feel that some
> drive out there ought have screwed up about nsect == 00h/0000h case.
Apparently Windows uses it, so I'd say that should pretty much guarantee
that it will work for us. And as I mentioned, there has never been a
case where the IDE driver value of 256 triggered a drive bug (one time
one was suspected, it turned out to be something else though).
--
Jens Axboe
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH] libata: increase LBA48 max sectors to 65535
2006-02-15 15:12 ` Jens Axboe
@ 2006-02-15 15:30 ` Tejun Heo
2006-02-15 19:03 ` Mark Lord
2006-02-15 20:21 ` Jeff Garzik
2 siblings, 0 replies; 24+ messages in thread
From: Tejun Heo @ 2006-02-15 15:30 UTC (permalink / raw)
To: Jens Axboe; +Cc: Jeff Garzik, albertcc, linux-ide
Jens Axboe wrote:
> On Thu, Feb 16 2006, Tejun Heo wrote:
>>
>>libata currently sets max_sectors to 200 (ATA_MAX_SECTORS) on lba28.
>>Are you talking about IDE driver? IDE driver seems to set max_sectors
>>to 256 on probe and make it 2048 while setting up ide disk if lba48.
>
>
> Yes I mean the IDE driver. The 200 sector value in libata has always
> struck me as being extremely odd and a very bad choice. It's somewhere
> in between reasonable defaults, which isn't very nice. I'd suggest just
> making it 256 as well, unless Jeff has a reason why it's set to 200.
>
>
>>Hmmm... Can we really trust all the firmwares? I just feel that some
>>drive out there ought have screwed up about nsect == 00h/0000h case.
>
>
> Apparently Windows uses it, so I'd say that should pretty much guarantee
> that it will work for us. And as I mentioned, there has never been a
> case where the IDE driver value of 256 triggered a drive bug (one time
> one was suspected, it turned out to be something else though).
>
Well, then it's Jeff's call. Thanks for the explanation. :-)
--
tejun
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] libata: increase LBA48 max sectors to 65535
2006-02-15 15:12 ` Jens Axboe
2006-02-15 15:30 ` Tejun Heo
@ 2006-02-15 19:03 ` Mark Lord
2006-02-15 20:21 ` Jeff Garzik
2 siblings, 0 replies; 24+ messages in thread
From: Mark Lord @ 2006-02-15 19:03 UTC (permalink / raw)
To: Jens Axboe; +Cc: Tejun Heo, Jeff Garzik, albertcc, linux-ide
There were some PATA host controllers (eg. trm290) that choked
on nsects==0, but I've never found a drive that did.
Cheers
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] libata: increase LBA48 max sectors to 65535
2006-02-15 15:12 ` Jens Axboe
2006-02-15 15:30 ` Tejun Heo
2006-02-15 19:03 ` Mark Lord
@ 2006-02-15 20:21 ` Jeff Garzik
2006-02-15 23:05 ` Mark Lord
2006-02-16 7:42 ` Jens Axboe
2 siblings, 2 replies; 24+ messages in thread
From: Jeff Garzik @ 2006-02-15 20:21 UTC (permalink / raw)
To: Jens Axboe; +Cc: Tejun Heo, albertcc, linux-ide, Mark Lord
Jens Axboe wrote:
> On Thu, Feb 16 2006, Tejun Heo wrote:
>
>>Jens Axboe wrote:
>>
>>>On Wed, Feb 15 2006, Tejun Heo wrote:
>>>
>>>
>>>>max_hw_sectors/max_sectors separation patch made into the tree,
>>>>increase max_sectors to its hardware limit.
>>>>
>>>>Signed-off-by: Tejun Heo <htejun@gmail.com>
>>>>
>>>>---
>>>>
>>>>Jeff, first of all, thanks for the pointer. I have two more
>>>>questions.
>>>>
>>>>* I make ATA_MAX_SECTORS_LBA48 65535 (the 0000h case is supposed to be
>>>>broken, right?); however, the comment says 65534. Should it be
>>>>65534?
>>>
>>>
>>>Since we never noticed any breakage on !sectors on lba28, I think we can
>>>safely assume that !sectors will work fine for lba48 as well. So why not
>>>just make it full 64k, eg 65536? ->max_hw_sectors is an unsigned int
>>>now, so 64k wont overflow it.
>>>
>>
>>Hello, Jens.
>>
>>libata currently sets max_sectors to 200 (ATA_MAX_SECTORS) on lba28.
>>Are you talking about IDE driver? IDE driver seems to set max_sectors
>>to 256 on probe and make it 2048 while setting up ide disk if lba48.
>
>
> Yes I mean the IDE driver. The 200 sector value in libata has always
> struck me as being extremely odd and a very bad choice. It's somewhere
> in between reasonable defaults, which isn't very nice. I'd suggest just
> making it 256 as well, unless Jeff has a reason why it's set to 200.
Its set to 200 for a libata-related reason that I now forget :(
It had nothing to do with controllers or devices, hence the FIXME.
As an aside, I wonder if the PRD limit is unnecessarily low. I don't
remember seeing anything that claimed we were limited to 256 s/g entries
in the old IDE docs?
Jeff
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] libata: increase LBA48 max sectors to 65535
2006-02-15 20:21 ` Jeff Garzik
@ 2006-02-15 23:05 ` Mark Lord
2006-02-20 11:23 ` Jeff Garzik
2006-02-16 7:42 ` Jens Axboe
1 sibling, 1 reply; 24+ messages in thread
From: Mark Lord @ 2006-02-15 23:05 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Jens Axboe, Tejun Heo, albertcc, linux-ide
Jeff Garzik wrote:
> Jens Axboe wrote:
>> On Thu, Feb 16 2006, Tejun Heo wrote:
>>
>>> Jens Axboe wrote:
>>>
>>>> On Wed, Feb 15 2006, Tejun Heo wrote:
>>>>
>>>>
>>>>> max_hw_sectors/max_sectors separation patch made into the tree,
>>>>> increase max_sectors to its hardware limit.
..
> Its set to 200 for a libata-related reason that I now forget :(
>
> It had nothing to do with controllers or devices, hence the FIXME.
>
> As an aside, I wonder if the PRD limit is unnecessarily low. I don't
> remember seeing anything that claimed we were limited to 256 s/g entries
> in the old IDE docs?
>
> Jeff
Well, originally I wrote ide-dma.c with a 256 PRD limit,
because that corresponds to the absolute worst case of
only one sector per PRD. The pre-LBA48 protocol cannot
read/write more than that with a single command.
But nowadays..
Cheers
--
Mark Lord
Real-Time Remedies Inc.
mlord@pobox.com
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] libata: increase LBA48 max sectors to 65535
2006-02-15 23:05 ` Mark Lord
@ 2006-02-20 11:23 ` Jeff Garzik
2006-02-20 13:54 ` Mark Lord
0 siblings, 1 reply; 24+ messages in thread
From: Jeff Garzik @ 2006-02-20 11:23 UTC (permalink / raw)
To: Mark Lord; +Cc: Jens Axboe, Tejun Heo, albertcc, linux-ide
Mark Lord wrote:
> Jeff Garzik wrote:
>> As an aside, I wonder if the PRD limit is unnecessarily low. I don't
>> remember seeing anything that claimed we were limited to 256 s/g
>> entries in the old IDE docs?
> Well, originally I wrote ide-dma.c with a 256 PRD limit,
> because that corresponds to the absolute worst case of
> only one sector per PRD. The pre-LBA48 protocol cannot
> read/write more than that with a single command.
>
> But nowadays..
Indeed. I am paranoid about controllers assuming that no more than 256
PRD entries will be submitted. But I have never seen any document that
says PCI IDE BMDMA hardware can -only- do 256 PRD entries. Never tested
it, either...
Jeff
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] libata: increase LBA48 max sectors to 65535
2006-02-20 11:23 ` Jeff Garzik
@ 2006-02-20 13:54 ` Mark Lord
2006-02-20 13:58 ` Jens Axboe
0 siblings, 1 reply; 24+ messages in thread
From: Mark Lord @ 2006-02-20 13:54 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Jens Axboe, Tejun Heo, albertcc, linux-ide
Jeff Garzik wrote:
>
> Indeed. I am paranoid about controllers assuming that no more than 256
> PRD entries will be submitted. But I have never seen any document that
> says PCI IDE BMDMA hardware can -only- do 256 PRD entries. Never tested
> it, either...
The hardware vendors I have worked with have never had any limit
on PRD count -- they'd happily keep reading through an infinite
list of PRDs if we could supply one.
A number of them did have issues with 64KB boundary crossings,
and it is generally VERY unsafe to have a single PRD "wrap"
into a fresh 64KB segment mid-stride.
Cheers
--
Mark Lord
Real-Time Remedies Inc.
mlord@pobox.com
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] libata: increase LBA48 max sectors to 65535
2006-02-20 13:54 ` Mark Lord
@ 2006-02-20 13:58 ` Jens Axboe
0 siblings, 0 replies; 24+ messages in thread
From: Jens Axboe @ 2006-02-20 13:58 UTC (permalink / raw)
To: Mark Lord; +Cc: Jeff Garzik, Tejun Heo, albertcc, linux-ide
On Mon, Feb 20 2006, Mark Lord wrote:
> Jeff Garzik wrote:
> >
> >Indeed. I am paranoid about controllers assuming that no more than 256
> >PRD entries will be submitted. But I have never seen any document that
> >says PCI IDE BMDMA hardware can -only- do 256 PRD entries. Never tested
> >it, either...
>
> The hardware vendors I have worked with have never had any limit
> on PRD count -- they'd happily keep reading through an infinite
> list of PRDs if we could supply one.
>
> A number of them did have issues with 64KB boundary crossings,
> and it is generally VERY unsafe to have a single PRD "wrap"
> into a fresh 64KB segment mid-stride.
Both IDE and libata take care not to make that happen, so we should be
fine there. The block layer also supports this property.
--
Jens Axboe
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] libata: increase LBA48 max sectors to 65535
2006-02-15 20:21 ` Jeff Garzik
2006-02-15 23:05 ` Mark Lord
@ 2006-02-16 7:42 ` Jens Axboe
2006-02-20 11:25 ` Jeff Garzik
1 sibling, 1 reply; 24+ messages in thread
From: Jens Axboe @ 2006-02-16 7:42 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Tejun Heo, albertcc, linux-ide, Mark Lord
On Wed, Feb 15 2006, Jeff Garzik wrote:
> Jens Axboe wrote:
> >On Thu, Feb 16 2006, Tejun Heo wrote:
> >
> >>Jens Axboe wrote:
> >>
> >>>On Wed, Feb 15 2006, Tejun Heo wrote:
> >>>
> >>>
> >>>>max_hw_sectors/max_sectors separation patch made into the tree,
> >>>>increase max_sectors to its hardware limit.
> >>>>
> >>>>Signed-off-by: Tejun Heo <htejun@gmail.com>
> >>>>
> >>>>---
> >>>>
> >>>>Jeff, first of all, thanks for the pointer. I have two more
> >>>>questions.
> >>>>
> >>>>* I make ATA_MAX_SECTORS_LBA48 65535 (the 0000h case is supposed to be
> >>>>broken, right?); however, the comment says 65534. Should it be
> >>>>65534?
> >>>
> >>>
> >>>Since we never noticed any breakage on !sectors on lba28, I think we can
> >>>safely assume that !sectors will work fine for lba48 as well. So why not
> >>>just make it full 64k, eg 65536? ->max_hw_sectors is an unsigned int
> >>>now, so 64k wont overflow it.
> >>>
> >>
> >>Hello, Jens.
> >>
> >>libata currently sets max_sectors to 200 (ATA_MAX_SECTORS) on lba28.
> >>Are you talking about IDE driver? IDE driver seems to set max_sectors
> >>to 256 on probe and make it 2048 while setting up ide disk if lba48.
> >
> >
> >Yes I mean the IDE driver. The 200 sector value in libata has always
> >struck me as being extremely odd and a very bad choice. It's somewhere
> >in between reasonable defaults, which isn't very nice. I'd suggest just
> >making it 256 as well, unless Jeff has a reason why it's set to 200.
>
> Its set to 200 for a libata-related reason that I now forget :(
Tsk tsk :-)
> It had nothing to do with controllers or devices, hence the FIXME.
How about making it 256 at the start of a kernel cycle and see if
anything falls apart? If IDE works, then surely SATA should as well.
> As an aside, I wonder if the PRD limit is unnecessarily low. I don't
> remember seeing anything that claimed we were limited to 256 s/g entries
> in the old IDE docs?
As Mark mentions it doesn't matter for lba28, for lba48 it'll typically
gives us at least 256*4k == 1meg of transfer, which is above/at the soft
limit we'll use anyways.
--
Jens Axboe
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] libata: increase LBA48 max sectors to 65535
2006-02-16 7:42 ` Jens Axboe
@ 2006-02-20 11:25 ` Jeff Garzik
2006-02-20 11:44 ` Jens Axboe
0 siblings, 1 reply; 24+ messages in thread
From: Jeff Garzik @ 2006-02-20 11:25 UTC (permalink / raw)
To: Jens Axboe; +Cc: Tejun Heo, albertcc, linux-ide, Mark Lord
Jens Axboe wrote:
> On Wed, Feb 15 2006, Jeff Garzik wrote:
>
>>Jens Axboe wrote:
>>
>>>On Thu, Feb 16 2006, Tejun Heo wrote:
>>>
>>>
>>>>Jens Axboe wrote:
>>>>
>>>>
>>>>>On Wed, Feb 15 2006, Tejun Heo wrote:
>>>>>
>>>>>
>>>>>
>>>>>>max_hw_sectors/max_sectors separation patch made into the tree,
>>>>>>increase max_sectors to its hardware limit.
>>>>>>
>>>>>>Signed-off-by: Tejun Heo <htejun@gmail.com>
>>>>>>
>>>>>>---
>>>>>>
>>>>>>Jeff, first of all, thanks for the pointer. I have two more
>>>>>>questions.
>>>>>>
>>>>>>* I make ATA_MAX_SECTORS_LBA48 65535 (the 0000h case is supposed to be
>>>>>>broken, right?); however, the comment says 65534. Should it be
>>>>>>65534?
>>>>>
>>>>>
>>>>>Since we never noticed any breakage on !sectors on lba28, I think we can
>>>>>safely assume that !sectors will work fine for lba48 as well. So why not
>>>>>just make it full 64k, eg 65536? ->max_hw_sectors is an unsigned int
>>>>>now, so 64k wont overflow it.
>>>>>
>>>>
>>>>Hello, Jens.
>>>>
>>>>libata currently sets max_sectors to 200 (ATA_MAX_SECTORS) on lba28.
>>>>Are you talking about IDE driver? IDE driver seems to set max_sectors
>>>>to 256 on probe and make it 2048 while setting up ide disk if lba48.
>>>
>>>
>>>Yes I mean the IDE driver. The 200 sector value in libata has always
>>>struck me as being extremely odd and a very bad choice. It's somewhere
>>>in between reasonable defaults, which isn't very nice. I'd suggest just
>>>making it 256 as well, unless Jeff has a reason why it's set to 200.
>>
>>Its set to 200 for a libata-related reason that I now forget :(
>
>
> Tsk tsk :-)
>
>
>>It had nothing to do with controllers or devices, hence the FIXME.
>
>
> How about making it 256 at the start of a kernel cycle and see if
> anything falls apart? If IDE works, then surely SATA should as well.
As I said, it had nothing to do with hardware. The variable in the
equation is the driver code itself, so IDE working implies nothing here,
alas.
Sigh... I'll search through some old emails.
Jeff
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] libata: increase LBA48 max sectors to 65535
2006-02-20 11:25 ` Jeff Garzik
@ 2006-02-20 11:44 ` Jens Axboe
0 siblings, 0 replies; 24+ messages in thread
From: Jens Axboe @ 2006-02-20 11:44 UTC (permalink / raw)
To: Jeff Garzik; +Cc: Tejun Heo, albertcc, linux-ide, Mark Lord
On Mon, Feb 20 2006, Jeff Garzik wrote:
> >How about making it 256 at the start of a kernel cycle and see if
> >anything falls apart? If IDE works, then surely SATA should as well.
>
> As I said, it had nothing to do with hardware. The variable in the
> equation is the driver code itself, so IDE working implies nothing here,
> alas.
>
> Sigh... I'll search through some old emails.
Hmmm, I'm having a hard time imagining what such a bug would look like
that would cause breakage with 256 sectors and work with 200. Especially
since we already do > 256 for lba48. But let me/us know what you dig up
from the email archives!
--
Jens Axboe
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] libata: increase LBA48 max sectors to 65535
2006-02-15 7:24 ` [PATCH] libata: increase LBA48 max sectors to 65535 Tejun Heo
2006-02-15 13:07 ` Jens Axboe
@ 2006-02-20 21:59 ` Jeff Garzik
1 sibling, 0 replies; 24+ messages in thread
From: Jeff Garzik @ 2006-02-20 21:59 UTC (permalink / raw)
To: Tejun Heo; +Cc: albertcc, linux-ide
Tejun Heo wrote:
> max_hw_sectors/max_sectors separation patch made into the tree,
> increase max_sectors to its hardware limit.
>
> Signed-off-by: Tejun Heo <htejun@gmail.com>
applied to libata-dev.git#max-sect, and pulled into #ALL.
I want this to get a bit of testing past 2.6.16 release.
Jeff
^ permalink raw reply [flat|nested] 24+ messages in thread