Re: [PATCH Linux 2.6.12-rc5-mm2 07/09] blk: update libata to use the new blk_ordered.

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 <[email protected]>



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-0514: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-0514: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-0514: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-0514:50:11.000000000 +0900+++ blk-fixes/drivers/scsi/sata_promise.c 2005-06-0514: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-0514: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-0514: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-0514: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-0514: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-0514: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-0514: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-0514: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-0514:50:11.000000000 +0900+++ blk-fixes/drivers/scsi/libata-core.c 2005-06-0514: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 opcodesto 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(structdev->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-0514:50:11.000000000 +0900+++ blk-fixes/drivers/scsi/libata-scsi.c 2005-06-0514: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(strif (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 andFUA' not just 'FUA'.

As DPO is sort of optimization flag, it doesn't make user-visibledifferences other than in performance. I think we can add DPO checkwhen translating commands and abort it w/ ILLEGAL_REQUEST (thus we'll belying about DPO part of the flag but not be lying that DPO operationsucceeds.) But it doesn't make any user-visible difference and we'renot using DPO in anywhere right now, so I think it's an overkill.

Any better ideas? Maybe adding another flag to scsi_device structurelike ->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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

References:
- [PATCH Linux 2.6.12-rc5-mm2 00/09] blk: ordered request reimplementation (take 2, for review)
  - From: Tejun Heo <[email protected]>
- Re: [PATCH Linux 2.6.12-rc5-mm2 07/09] blk: update libata to use the new blk_ordered.
  - From: Tejun Heo <[email protected]>
- Re: [PATCH Linux 2.6.12-rc5-mm2 07/09] blk: update libata to use the new blk_ordered.
  - From: Jeff Garzik <[email protected]>

Prev by Date: Re: [PATCH Linux 2.6.12-rc5-mm2 06/09] blk: update SCSI to use the new blk_ordered
Next by Date: Re: [PATCH] pcmcia/ds: handle any error code
Previous by thread: Re: [PATCH Linux 2.6.12-rc5-mm2 07/09] blk: update libata to use the new blk_ordered.
Next by thread: Re: [PATCH Linux 2.6.12-rc5-mm2 08/09] blk: update IDE to use the new blk_ordered.
Index(es):
- Date
- Thread

[Index of Archives] [Kernel Newbies] [Netfilter] [Bugtraq] [Photo] [Stuff] [Gimp] [Yosemite News] [MIPS Linux] [ARM Linux] [Linux Security] [Linux RAID] [Video 4 Linux] [Linux for the blind] [Linux Resources]