Re: [PATCH linux-2.6-block:post-2.6.15 09/10] blk: add FUA support to IDE

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/17/05, Tejun Heo <[email protected]> wrote:
> 09_blk_ide-add-fua-support.patch
>
>         Add FUA support to IDE
>
> Signed-off-by: Tejun Heo <[email protected]>
>
>  drivers/ide/ide-disk.c |   57 +++++++++++++++++++++++++++++++++++++++++--------
>  include/linux/hdreg.h  |   16 ++++++++++++-
>  include/linux/ide.h    |    3 ++
>  3 files changed, 65 insertions(+), 11 deletions(-)
>
> Index: work/drivers/ide/ide-disk.c
> ===================================================================
> --- work.orig/drivers/ide/ide-disk.c    2005-11-18 00:35:06.000000000 +0900
> +++ work/drivers/ide/ide-disk.c 2005-11-18 00:35:07.000000000 +0900
> @@ -164,13 +164,14 @@ static ide_startstop_t __ide_do_rw_disk(
>         ide_hwif_t *hwif        = HWIF(drive);
>         unsigned int dma        = drive->using_dma;
>         u8 lba48                = (drive->addressing == 1) ? 1 : 0;
> +       int fua                 = blk_fua_rq(rq);
>         task_ioreg_t command    = WIN_NOP;
>         ata_nsector_t           nsectors;
>
>         nsectors.all            = (u16) rq->nr_sectors;
>
>         if (hwif->no_lba48_dma && lba48 && dma) {
> -               if (block + rq->nr_sectors > 1ULL << 28)
> +               if (block + rq->nr_sectors > 1ULL << 28 || fua)
>                         dma = 0;
>                 else
>                         lba48 = 0;
> @@ -226,6 +227,16 @@ static ide_startstop_t __ide_do_rw_disk(
>                         hwif->OUTB(tasklets[6], IDE_HCYL_REG);
>                         hwif->OUTB(0x00|drive->select.all,IDE_SELECT_REG);
>                 } else {
> +                       if (unlikely(fua)) {
> +                               /*
> +                                * This happens if LBA48 addressing is
> +                                * turned off during operation.
> +                                */
> +                               printk(KERN_ERR "%s: FUA write but LBA48 off\n",
> +                                      drive->name);
> +                               goto fail;
> +                       }
> +
>                         hwif->OUTB(0x00, IDE_FEATURE_REG);
>                         hwif->OUTB(nsectors.b.low, IDE_NSECTOR_REG);
>                         hwif->OUTB(block, IDE_SECTOR_REG);
> @@ -253,9 +264,12 @@ static ide_startstop_t __ide_do_rw_disk(
>         if (dma) {
>                 if (!hwif->dma_setup(drive)) {
>                         if (rq_data_dir(rq)) {
> -                               command = lba48 ? WIN_WRITEDMA_EXT : WIN_WRITEDMA;
> -                               if (drive->vdma)
> -                                       command = lba48 ? WIN_WRITE_EXT: WIN_WRITE;
> +                               if (!fua) {
> +                                       command = lba48 ? WIN_WRITEDMA_EXT : WIN_WRITEDMA;
> +                                       if (drive->vdma)
> +                                               command = lba48 ? WIN_WRITE_EXT: WIN_WRITE;
> +                               } else
> +                                       command = ATA_CMD_WRITE_FUA_EXT;

What does happen for fua && drive->vdma case?

>                         } else {
>                                 command = lba48 ? WIN_READDMA_EXT : WIN_READDMA;
>                                 if (drive->vdma)
> @@ -284,8 +298,20 @@ static ide_startstop_t __ide_do_rw_disk(
>         } else {
>                 if (drive->mult_count) {
>                         hwif->data_phase = TASKFILE_MULTI_OUT;
> -                       command = lba48 ? WIN_MULTWRITE_EXT : WIN_MULTWRITE;
> +                       if (!fua)
> +                               command = lba48 ? WIN_MULTWRITE_EXT : WIN_MULTWRITE;
> +                       else
> +                               command = ATA_CMD_WRITE_MULTI_FUA_EXT;
>                 } else {
> +                       if (unlikely(fua)) {
> +                               /*
> +                                * This happens if multisector PIO is
> +                                * turned off during operation.
> +                                */
> +                               printk(KERN_ERR "%s: FUA write but in single "
> +                                      "sector PIO mode\n", drive->name);
> +                               goto fail;
> +                       }

Wouldn't it be better to do the following check at the beginning
of __ide_do_rw_disk() (after checking for dma vs lba48):

        if (fua) {
                if (!lba48 || ((!dma || drive->vdma) && !drive->mult_count))
                        goto fail_fua;
        }

...

and fail the request if needed *before* actually touching any
hardware registers?

fail_fua:
        printk(KERN_ERR "%s: FUA write unsupported (lba48=%u dma=%u"
                                       " vdma=%u mult_count=%u)\n", drive->name,
                                       lba48, dma, drive->vdma,
drive->mult_count);
        ide_end_request(drive, 0, 0);
        return ide_stopped;

>                         hwif->data_phase = TASKFILE_OUT;
>                         command = lba48 ? WIN_WRITE_EXT : WIN_WRITE;
>                 }
> @@ -295,6 +321,10 @@ static ide_startstop_t __ide_do_rw_disk(
>
>                 return pre_task_out_intr(drive, rq);
>         }
> +
> + fail:
> +       ide_end_request(drive, 0, 0);
> +       return ide_stopped;
>  }
>
>  /*
> @@ -846,7 +876,7 @@ static void idedisk_setup (ide_drive_t *
>  {
>         struct hd_driveid *id = drive->id;
>         unsigned long long capacity;
> -       int barrier;
> +       int barrier, fua;
>
>         idedisk_add_settings(drive);
>
> @@ -967,10 +997,19 @@ static void idedisk_setup (ide_drive_t *
>                         barrier = 0;
>         }
>
> -       printk(KERN_INFO "%s: cache flushes %ssupported\n",
> -               drive->name, barrier ? "" : "not ");
> +       fua = barrier && idedisk_supports_lba48(id) && ide_id_has_fua(id);
> +       /* When using PIO, FUA needs multisector. */
> +       if ((!drive->using_dma || drive->hwif->no_lba48_dma) &&
> +           drive->mult_count == 0)
> +               fua = 0;

Shouldn't this check also for drive->vdma?

> +
> +       printk(KERN_INFO "%s: cache flushes %ssupported%s\n",
> +              drive->name, barrier ? "" : "not ",
> +              fua ? " w/ FUA" : "");
>         if (barrier) {
> -               blk_queue_ordered(drive->queue, QUEUE_ORDERED_DRAIN_FLUSH,
> +               unsigned ordered = fua ? QUEUE_ORDERED_DRAIN_FUA
> +                                      : QUEUE_ORDERED_DRAIN_FLUSH;
> +               blk_queue_ordered(drive->queue, ordered,
>                                   idedisk_prepare_flush, GFP_KERNEL);
>                 blk_queue_issue_flush_fn(drive->queue, idedisk_issue_flush);
>         } else if (!drive->wcache)
> Index: work/include/linux/hdreg.h
> ===================================================================
> --- work.orig/include/linux/hdreg.h     2005-11-18 00:06:46.000000000 +0900
> +++ work/include/linux/hdreg.h  2005-11-18 00:35:07.000000000 +0900
> @@ -550,7 +550,13 @@ struct hd_driveid {
>                                          * cmd set-feature supported extensions
>                                          * 15:  Shall be ZERO
>                                          * 14:  Shall be ONE
> -                                        * 13:6 reserved
> +                                        * 13:  IDLE IMMEDIATE w/ UNLOAD FEATURE
> +                                        * 12:11 reserved for technical report
> +                                        * 10:  URG for WRITE STREAM
> +                                        *  9:  URG for READ STREAM
> +                                        *  8:  64-bit World wide name
> +                                        *  7:  WRITE DMA QUEUED FUA EXT
> +                                        *  6:  WRITE DMA/MULTIPLE FUA EXT
>                                          *  5:  General Purpose Logging
>                                          *  4:  Streaming Feature Set
>                                          *  3:  Media Card Pass Through
> @@ -600,7 +606,13 @@ struct hd_driveid {
>                                          * command set-feature default
>                                          * 15:  Shall be ZERO
>                                          * 14:  Shall be ONE
> -                                        * 13:6 reserved
> +                                        * 13:  IDLE IMMEDIATE w/ UNLOAD FEATURE
> +                                        * 12:11 reserved for technical report
> +                                        * 10:  URG for WRITE STREAM
> +                                        *  9:  URG for READ STREAM
> +                                        *  8:  64-bit World wide name
> +                                        *  7:  WRITE DMA QUEUED FUA EXT
> +                                        *  6:  WRITE DMA/MULTIPLE FUA EXT
>                                          *  5:  General Purpose Logging enabled
>                                          *  4:  Valid CONFIGURE STREAM executed
>                                          *  3:  Media Card Pass Through enabled
> Index: work/include/linux/ide.h
> ===================================================================
> --- work.orig/include/linux/ide.h       2005-11-18 00:14:29.000000000 +0900
> +++ work/include/linux/ide.h    2005-11-18 00:35:07.000000000 +0900
> @@ -1503,6 +1503,9 @@ extern struct bus_type ide_bus_type;
>  /* check if CACHE FLUSH (EXT) command is supported (bits defined in ATA-6) */
>  #define ide_id_has_flush_cache(id)     ((id)->cfs_enable_2 & 0x3000)
>
> +/* check if WRITE DMA FUA EXT command is supported (defined in ATA-8) */
> +#define ide_id_has_fua(id)             ((id)->cfsse & 0x0040)
> +
>  /* some Maxtor disks have bit 13 defined incorrectly so check bit 10 too */
>  #define ide_id_has_flush_cache_ext(id) \
>         (((id)->cfs_enable_2 & 0x2400) == 0x2400)
>
>
-
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/

[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]
  Powered by Linux