From: Tejun Heo <htejun@gmail.com>
To: Jeff Garzik <jgarzik@pobox.com>
Cc: axboe@suse.de, James.Bottomley@steeleye.com, bzolnier@gmail.com,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH Linux 2.6.12-rc5-mm2 07/09] blk: update libata to use the new blk_ordered.
Date: Tue, 07 Jun 2005 11:11:31 +0900 [thread overview]
Message-ID: <42A50253.9080307@gmail.com> (raw)
In-Reply-To: <42A2A39B.5020103@pobox.com>
Jeff Garzik wrote:
> Tejun Heo wrote:
>
>> 07_blk_update_libata_to_use_new_ordered.patch
>>
>> Reflect changes in SCSI midlayer and updated to use the new
>> ordered request implementation.
>>
>> Signed-off-by: Tejun Heo <htejun@gmail.com>
>
>
> I would prefer separate patches for:
>
> * implement support for FUA bit in libata SCSI simulator
>
> * update libata for your ordered flush changes
>
Sure.
>
>
>> Index: blk-fixes/drivers/scsi/ahci.c
>> ===================================================================
>> --- blk-fixes.orig/drivers/scsi/ahci.c 2005-06-05
>> 14:50:11.000000000 +0900
>> +++ blk-fixes/drivers/scsi/ahci.c 2005-06-05 14:53:35.000000000 +0900
>> @@ -203,7 +203,6 @@ static Scsi_Host_Template ahci_sht = {
>> .dma_boundary = AHCI_DMA_BOUNDARY,
>> .slave_configure = ata_scsi_slave_config,
>> .bios_param = ata_std_bios_param,
>> - .ordered_flush = 1,
>> };
>>
>> static struct ata_port_operations ahci_ops = {
>> Index: blk-fixes/drivers/scsi/ata_piix.c
>> ===================================================================
>> --- blk-fixes.orig/drivers/scsi/ata_piix.c 2005-06-05
>> 14:50:11.000000000 +0900
>> +++ blk-fixes/drivers/scsi/ata_piix.c 2005-06-05 14:53:35.000000000
>> +0900
>> @@ -123,7 +123,6 @@ static Scsi_Host_Template piix_sht = {
>> .dma_boundary = ATA_DMA_BOUNDARY,
>> .slave_configure = ata_scsi_slave_config,
>> .bios_param = ata_std_bios_param,
>> - .ordered_flush = 1,
>> };
>>
>> static struct ata_port_operations piix_pata_ops = {
>> Index: blk-fixes/drivers/scsi/sata_nv.c
>> ===================================================================
>> --- blk-fixes.orig/drivers/scsi/sata_nv.c 2005-06-05
>> 14:50:11.000000000 +0900
>> +++ blk-fixes/drivers/scsi/sata_nv.c 2005-06-05 14:53:35.000000000
>> +0900
>> @@ -206,7 +206,6 @@ static Scsi_Host_Template nv_sht = {
>> .dma_boundary = ATA_DMA_BOUNDARY,
>> .slave_configure = ata_scsi_slave_config,
>> .bios_param = ata_std_bios_param,
>> - .ordered_flush = 1,
>> };
>>
>> static struct ata_port_operations nv_ops = {
>> Index: blk-fixes/drivers/scsi/sata_promise.c
>> ===================================================================
>> --- blk-fixes.orig/drivers/scsi/sata_promise.c 2005-06-05
>> 14:50:11.000000000 +0900
>> +++ blk-fixes/drivers/scsi/sata_promise.c 2005-06-05
>> 14:53:35.000000000 +0900
>> @@ -104,7 +104,6 @@ static Scsi_Host_Template pdc_ata_sht =
>> .dma_boundary = ATA_DMA_BOUNDARY,
>> .slave_configure = ata_scsi_slave_config,
>> .bios_param = ata_std_bios_param,
>> - .ordered_flush = 1,
>> };
>>
>> static struct ata_port_operations pdc_ata_ops = {
>> Index: blk-fixes/drivers/scsi/sata_sil.c
>> ===================================================================
>> --- blk-fixes.orig/drivers/scsi/sata_sil.c 2005-06-05
>> 14:50:11.000000000 +0900
>> +++ blk-fixes/drivers/scsi/sata_sil.c 2005-06-05 14:53:35.000000000
>> +0900
>> @@ -135,7 +135,6 @@ static Scsi_Host_Template sil_sht = {
>> .dma_boundary = ATA_DMA_BOUNDARY,
>> .slave_configure = ata_scsi_slave_config,
>> .bios_param = ata_std_bios_param,
>> - .ordered_flush = 1,
>> };
>>
>> static struct ata_port_operations sil_ops = {
>> Index: blk-fixes/drivers/scsi/sata_sis.c
>> ===================================================================
>> --- blk-fixes.orig/drivers/scsi/sata_sis.c 2005-06-05
>> 14:50:11.000000000 +0900
>> +++ blk-fixes/drivers/scsi/sata_sis.c 2005-06-05 14:53:35.000000000
>> +0900
>> @@ -90,7 +90,6 @@ static Scsi_Host_Template sis_sht = {
>> .dma_boundary = ATA_DMA_BOUNDARY,
>> .slave_configure = ata_scsi_slave_config,
>> .bios_param = ata_std_bios_param,
>> - .ordered_flush = 1,
>> };
>>
>> static struct ata_port_operations sis_ops = {
>> Index: blk-fixes/drivers/scsi/sata_svw.c
>> ===================================================================
>> --- blk-fixes.orig/drivers/scsi/sata_svw.c 2005-06-05
>> 14:50:11.000000000 +0900
>> +++ blk-fixes/drivers/scsi/sata_svw.c 2005-06-05 14:53:35.000000000
>> +0900
>> @@ -288,7 +288,6 @@ static Scsi_Host_Template k2_sata_sht =
>> .proc_info = k2_sata_proc_info,
>> #endif
>> .bios_param = ata_std_bios_param,
>> - .ordered_flush = 1,
>> };
>>
>>
>> Index: blk-fixes/drivers/scsi/sata_sx4.c
>> ===================================================================
>> --- blk-fixes.orig/drivers/scsi/sata_sx4.c 2005-06-05
>> 14:50:11.000000000 +0900
>> +++ blk-fixes/drivers/scsi/sata_sx4.c 2005-06-05 14:53:35.000000000
>> +0900
>> @@ -188,7 +188,6 @@ static Scsi_Host_Template pdc_sata_sht =
>> .dma_boundary = ATA_DMA_BOUNDARY,
>> .slave_configure = ata_scsi_slave_config,
>> .bios_param = ata_std_bios_param,
>> - .ordered_flush = 1,
>> };
>>
>> static struct ata_port_operations pdc_20621_ops = {
>> Index: blk-fixes/drivers/scsi/sata_uli.c
>> ===================================================================
>> --- blk-fixes.orig/drivers/scsi/sata_uli.c 2005-06-05
>> 14:50:11.000000000 +0900
>> +++ blk-fixes/drivers/scsi/sata_uli.c 2005-06-05 14:53:35.000000000
>> +0900
>> @@ -82,7 +82,6 @@ static Scsi_Host_Template uli_sht = {
>> .dma_boundary = ATA_DMA_BOUNDARY,
>> .slave_configure = ata_scsi_slave_config,
>> .bios_param = ata_std_bios_param,
>> - .ordered_flush = 1,
>> };
>>
>> static struct ata_port_operations uli_ops = {
>> Index: blk-fixes/drivers/scsi/sata_via.c
>> ===================================================================
>> --- blk-fixes.orig/drivers/scsi/sata_via.c 2005-06-05
>> 14:50:11.000000000 +0900
>> +++ blk-fixes/drivers/scsi/sata_via.c 2005-06-05 14:53:35.000000000
>> +0900
>> @@ -102,7 +102,6 @@ static Scsi_Host_Template svia_sht = {
>> .dma_boundary = ATA_DMA_BOUNDARY,
>> .slave_configure = ata_scsi_slave_config,
>> .bios_param = ata_std_bios_param,
>> - .ordered_flush = 1,
>> };
>>
>> static struct ata_port_operations svia_sata_ops = {
>> Index: blk-fixes/drivers/scsi/sata_vsc.c
>> ===================================================================
>> --- blk-fixes.orig/drivers/scsi/sata_vsc.c 2005-06-05
>> 14:50:11.000000000 +0900
>> +++ blk-fixes/drivers/scsi/sata_vsc.c 2005-06-05 14:53:35.000000000
>> +0900
>> @@ -206,7 +206,6 @@ static Scsi_Host_Template vsc_sata_sht =
>> .dma_boundary = ATA_DMA_BOUNDARY,
>> .slave_configure = ata_scsi_slave_config,
>> .bios_param = ata_std_bios_param,
>> - .ordered_flush = 1,
>> };
>>
>>
>> Index: blk-fixes/drivers/scsi/libata-core.c
>> ===================================================================
>> --- blk-fixes.orig/drivers/scsi/libata-core.c 2005-06-05
>> 14:50:11.000000000 +0900
>> +++ blk-fixes/drivers/scsi/libata-core.c 2005-06-05
>> 14:53:35.000000000 +0900
>> @@ -510,19 +510,21 @@ void ata_tf_from_fis(u8 *fis, struct ata
>> }
>>
>> /**
>> - * ata_prot_to_cmd - determine which read/write opcodes to use
>> + * ata_prot_to_cmd - determine which read/write/fua-write opcodes
>> to use
>> * @protocol: ATA_PROT_xxx taskfile protocol
>> * @lba48: true is lba48 is present
>> *
>> - * Given necessary input, determine which read/write commands
>> - * to use to transfer data.
>> + * Given necessary input, determine which read/write/fua-write
>> + * commands to use to transfer data. Note that we only support
>> + * fua-writes on DMA LBA48 protocol. In other cases, we simply
>> + * return 0 which is NOP.
>> *
>> * LOCKING:
>> * None.
>> */
>> static int ata_prot_to_cmd(int protocol, int lba48)
>> {
>> - int rcmd = 0, wcmd = 0;
>> + int rcmd = 0, wcmd = 0, wfua = 0;
>>
>> switch (protocol) {
>> case ATA_PROT_PIO:
>> @@ -539,6 +541,7 @@ static int ata_prot_to_cmd(int protocol,
>> if (lba48) {
>> rcmd = ATA_CMD_READ_EXT;
>> wcmd = ATA_CMD_WRITE_EXT;
>> + wfua = ATA_CMD_WRITE_FUA_EXT;
>> } else {
>> rcmd = ATA_CMD_READ;
>> wcmd = ATA_CMD_WRITE;
>> @@ -549,7 +552,7 @@ static int ata_prot_to_cmd(int protocol,
>> return -1;
>> }
>>
>> - return rcmd | (wcmd << 8);
>> + return rcmd | (wcmd << 8) | (wfua << 16);
>> }
>>
>> /**
>> @@ -582,6 +585,7 @@ static void ata_dev_set_protocol(struct
>> dev->read_cmd = cmd & 0xff;
>> dev->write_cmd = (cmd >> 8) & 0xff;
>> + dev->write_fua_cmd = (cmd >> 16) & 0xff;
>> }
>>
>> static const char * xfer_mode_str[] = {
>> Index: blk-fixes/drivers/scsi/libata-scsi.c
>> ===================================================================
>> --- blk-fixes.orig/drivers/scsi/libata-scsi.c 2005-06-05
>> 14:50:11.000000000 +0900
>> +++ blk-fixes/drivers/scsi/libata-scsi.c 2005-06-05
>> 14:53:35.000000000 +0900
>> @@ -569,6 +569,7 @@ static unsigned int ata_scsi_rw_xlat(str
>> struct ata_device *dev = qc->dev;
>> unsigned int lba = tf->flags & ATA_TFLAG_LBA;
>> unsigned int lba48 = tf->flags & ATA_TFLAG_LBA48;
>> + int fua = scsicmd[1] & 0x8;
>> u64 block = 0;
>> u32 n_block = 0;
>>
>> @@ -577,9 +578,26 @@ static unsigned int ata_scsi_rw_xlat(str
>>
>> if (scsicmd[0] == READ_10 || scsicmd[0] == READ_6 ||
>> scsicmd[0] == READ_16) {
>> + if (fua) {
>> + printk(KERN_WARNING
>> + "ata%u(%u): WARNING: FUA READ unsupported\n",
>> + qc->ap->id, qc->dev->devno);
>> + return 1;
>> + }
>> tf->command = qc->dev->read_cmd;
>> } else {
>> - tf->command = qc->dev->write_cmd;
>> + if (fua) {
>> + if (qc->dev->write_fua_cmd == 0 || !lba48) {
>> + printk(KERN_WARNING
>> + "ata%u(%u): WARNING: FUA WRITE "
>> + "unsupported with the current "
>> + "protocol/addressing\n",
>> + qc->ap->id, qc->dev->devno);
>> + return 1;
>> + }
>> + tf->command = qc->dev->write_fua_cmd;
>> + } else
>> + tf->command = qc->dev->write_cmd;
>> tf->flags |= ATA_TFLAG_WRITE;
>> }
>>
>
>
> this all seems fine.
>
>
>> @@ -1205,10 +1223,12 @@ unsigned int ata_scsiop_mode_sense(struc
>> if (six_byte) {
>> output_len--;
>> rbuf[0] = output_len;
>> + rbuf[2] |= ata_id_has_fua(args->id) ? 0x10 : 0;
>> } else {
>> output_len -= 2;
>> rbuf[0] = output_len >> 8;
>> rbuf[1] = output_len;
>> + rbuf[3] |= ata_id_has_fua(args->id) ? 0x10 : 0;
>> }
>
>
> I wonder what a SCSI person thinks about this. Its defined as 'DPO and
> FUA' not just 'FUA'.
>
As DPO is sort of optimization flag, it doesn't make user-visible
differences other than in performance. I think we can add DPO check
when translating commands and abort it w/ ILLEGAL_REQUEST (thus we'll be
lying about DPO part of the flag but not be lying that DPO operation
succeeds.) But it doesn't make any user-visible difference and we're
not using DPO in anywhere right now, so I think it's an overkill.
Any better ideas? Maybe adding another flag to scsi_device structure
like ->fua_supported which can be adjusted by slave_config?
> Also, a bit of style: please use "1 << n" for bit constants in libata.
>
> Jeff
>
Sure.
Thank you.
--
tejun
next prev parent reply other threads:[~2005-06-07 2:11 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2005-06-05 5:57 [PATCH Linux 2.6.12-rc5-mm2 00/09] blk: ordered request reimplementation (take 2, for review) Tejun Heo
2005-06-05 5:57 ` [PATCH Linux 2.6.12-rc5-mm2 01/09] blk: add @uptodate to end_that_request_last() and @error to rq_end_io_fn() Tejun Heo
2005-06-05 5:57 ` [PATCH Linux 2.6.12-rc5-mm2 02/09] blk: make scsi use -EOPNOTSUPP instead of -EIO on ILLEGAL_REQUEST Tejun Heo
2005-06-05 7:10 ` Jeff Garzik
2005-06-07 1:34 ` Tejun Heo
2005-06-05 5:57 ` [PATCH Linux 2.6.12-rc5-mm2 03/09] blk: make ide use -EOPNOTSUPP instead of -EIO on ABRT_ERR Tejun Heo
2005-06-05 5:57 ` [PATCH Linux 2.6.12-rc5-mm2 04/09] blk: separate out bio init part from __make_request Tejun Heo
2005-06-05 5:57 ` [PATCH Linux 2.6.12-rc5-mm2 05/09] blk: reimplement handling of barrier request Tejun Heo
2005-06-05 5:57 ` [PATCH Linux 2.6.12-rc5-mm2 06/09] blk: update SCSI to use the new blk_ordered Tejun Heo
2005-06-05 7:08 ` Jeff Garzik
2005-06-07 1:58 ` Tejun Heo
2005-06-05 5:57 ` [PATCH Linux 2.6.12-rc5-mm2 07/09] blk: update libata " Tejun Heo
2005-06-05 7:02 ` Jeff Garzik
2005-06-07 2:11 ` Tejun Heo [this message]
2005-06-05 5:57 ` [PATCH Linux 2.6.12-rc5-mm2 08/09] blk: update IDE " Tejun Heo
2005-06-05 6:47 ` Jeff Garzik
2005-06-05 14:14 ` Bartlomiej Zolnierkiewicz
2005-06-07 2:26 ` Tejun Heo
2005-06-05 5:57 ` [PATCH Linux 2.6.12-rc5-mm2 09/09] blk: debug messages Tejun Heo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=42A50253.9080307@gmail.com \
--to=htejun@gmail.com \
--cc=James.Bottomley@steeleye.com \
--cc=axboe@suse.de \
--cc=bzolnier@gmail.com \
--cc=jgarzik@pobox.com \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox