public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Alan Cox <alan@redhat.com>
To: torvalds@osdl.org, axboe@suse.dk, linux-kernel@vger.kernel.org
Subject: PATCH: fix the barrier IDE detection logic
Date: Tue, 31 Aug 2004 12:50:46 -0400	[thread overview]
Message-ID: <20040831165046.GA6928@devserv.devel.redhat.com> (raw)

This fixes the logic so we always check for the cache. It also defaults
to safer behaviour for the non cache flush case now we have the right bits
in the right places. I've also played a bit with timings - the worst case
timings I can get for the flush are about 7 seconds (which I'd expect
as the engineering worst cases will include retries)

Probably what should happen is that the barrier logic is enabled providing
the wcache is disabled. I've not meddled with this as I don't know what
the intended semantics and rules are for disabling barrier on a live disk
(eg when a user uses hdparm to turn on the write cache). In the current
code as with Jens original that cannot occur.

I've also fixed the new printk's as per a private request from Matt Domsch.

This look right to you Jens ?

--- drivers/ide/ide-disk.c~	2004-08-31 17:35:01.065916488 +0100
+++ drivers/ide/ide-disk.c	2004-08-31 17:41:42.637868272 +0100
@@ -1298,7 +1298,7 @@
 	return 0;
 }
 
-static int write_cache (ide_drive_t *drive, int arg)
+static int write_cache(ide_drive_t *drive, int arg)
 {
 	ide_task_t args;
 	int err;
@@ -1508,7 +1508,7 @@
 		blk_queue_max_sectors(drive->queue, max_s);
 	}
 
-	printk("%s: max request size: %dKiB\n", drive->name, drive->queue->max_sectors / 2);
+	printk(KERN_INFO "%s: max request size: %dKiB\n", drive->name, drive->queue->max_sectors / 2);
 
 	/* Extract geometry if we did not already have one for the drive */
 	if (!drive->cyl || !drive->head || !drive->sect) {
@@ -1537,7 +1537,7 @@
 
 	/* limit drive capacity to 137GB if LBA48 cannot be used */
 	if (drive->addressing == 0 && drive->capacity64 > 1ULL << 28) {
-		printk("%s: cannot use LBA48 - full capacity "
+		printk(KERN_WARNING "%s: cannot use LBA48 - full capacity "
 		       "%llu sectors (%llu MB)\n",
 		       drive->name, (unsigned long long)drive->capacity64,
 		       sectors_to_MB(drive->capacity64));
@@ -1607,23 +1607,31 @@
 	if ((id->csfo & 1) || (id->cfs_enable_1 & (1 << 5)))
 		drive->wcache = 1;
 
-	write_cache(drive, 1);
-
 	/*
-	 * decide if we can sanely support flushes and barriers on
-	 * this drive. unfortunately not all drives advertise FLUSH_CACHE
-	 * support even if they support it. So assume FLUSH_CACHE is there
-	 * always. LBA48 drives are newer, so expect it to flag support
-	 * properly. We can safely support FLUSH_CACHE on lba48, if capacity
-	 * doesn't exceed lba28
+	 * We must avoid issuing commands a drive does not understand
+	 * or we may crash it. We check flush cache is supported. We also
+	 * check we have the LBA48 flush cache if the drive capacity is
+	 * too large. By this time we have trimmed the drive capacity if
+	 * LBA48 is not available so we don't need to recheck that.
 	 */
-	barrier = 1;
+	barrier = 0;
+	if (ide_id_has_flush_cache(id))
+		barrier = 1;
 	if (drive->addressing == 1) {
+		/* Can't issue the correct flush ? */
 		if (capacity > (1ULL << 28) && !ide_id_has_flush_cache_ext(id))
 			barrier = 0;
 	}
+	/* Now we have barrier awareness we can be properly conservative
+	   by default with other drives. We turn off write caching when
+	   barrier is not available. Users can adjust this at runtime if
+	   they need unsafe but fast filesystems. This will reduce the
+	   performance of non cache flush supporting disks but it means
+	   you get the data order guarantees the journalling fs's require */
+	   
+	write_cache(drive, barrier);
 
-	printk("%s: cache flushes %ssupported\n",
+	printk(KERN_DEBUG "%s: cache flushes %ssupported\n",
 		drive->name, barrier ? "" : "not ");
 	if (barrier) {
 		blk_queue_ordered(drive->queue, 1);

             reply	other threads:[~2004-08-31 16:51 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2004-08-31 16:50 Alan Cox [this message]
2004-09-03 13:54 ` PATCH: fix the barrier IDE detection logic Bartlomiej Zolnierkiewicz
2004-09-03 14:05   ` Alan Cox
2004-09-03 15:10   ` Jens Axboe
2004-09-03 15:08 ` Jens Axboe

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=20040831165046.GA6928@devserv.devel.redhat.com \
    --to=alan@redhat.com \
    --cc=axboe@suse.dk \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@osdl.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