Hi, Bartlomiej.
Bartlomiej Zolnierkiewicz wrote:
On 11/17/05, Tejun Heo <[email protected]> wrote:
What does happen for fua && drive->vdma case?
Thanks for pointing out, wasn't thinking about that. Hmmm... When using
vdma, single-sector PIO commands are issued instead but there's no
single-sector FUA PIO command. Would issuing
ATA_CMD_WRITE_MULTI_FUA_EXT instead of ATA_CMD_WRITE_FUA_EXT work? Or
should I just disable FUA on 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;
Hmmm... The thing is that those failure cases will happen extremely
rarely if at all. Remember this post?
http://marc.theaimsgroup.com/?l=linux-kernel&m=111798102108338&w=3
It's mostly guaranteed that those failure cases don't occur, so I
thought avoiding IO on failure case wasn't that helpful.
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?
Yes, it does. Thanks for pointing out. One question though. FUA
support should be changed if using_dma/mult_count settings are changed.
As using_dma configuration is handled by IDE midlayer, we might need
to add a callback there. What do you think?
--
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/
[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]
