Re: [PATCH] USB: storage: add "no SYNCHRONIZE CACHE" quirk

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

 



On Mon, 2015-06-22 at 10:55 -0400, Alan Stern wrote:
> Some USB mass-storage devices claim to have a write-back cache but
> don't support the SYNCHRONIZE CACHE command, which means there is no
> way to tell these devices to flush their caches out to permanent
> storage.  Unfortunately, there is nothing we can do about this.
> 
> Until recently this deficiency did not cause any noticeable problems.
> But following commit 89fb4cd1f717 ("scsi: handle flush errors
> properly"), the errors are propagated to userspace where they get
> reported as failures.
> 
> As a workaround, this patch adds a quirks flag for these devices to
> the usb-storage driver, and a corresponding SCSI device flag, to
> indicate the device can't handle SYNCHRONIZE CACHE.  If the flag is
> set, the sd driver will override the cache settings reported by the
> device, so that the device appears to be write-through.  This will
> prevent the unsupported command from being sent.
> 
> This addresses Bugzilla #89511.

I'm not sure I entirely like this:  we are back again treating data
corruption problems silently.

However, I also believe treating a single flush failure as a critical
filesystem error is also wrong:  The data's all there correctly; all it
does is introduce a potential window were the FS could get corrupted in
the unlikely event the system crashed.

Obviously, for a disk with a writeback cache that can't do flush, that
window is much wider and the real solution should be to try to switch
the cache to write through.
How about something like this patch?  It transforms FS FLUSH into a log
warning from an error but preserves the error on any other path.  You'll
still get a fairly continuous dump of warnings for one of these devices,
though ... do they respond to mode selects turning off the writeback?

James

---

diff --git a/block/blk-flush.c b/block/blk-flush.c
index 20badd7..bb9f9f3 100644
--- a/block/blk-flush.c
+++ b/block/blk-flush.c
@@ -173,10 +173,21 @@ static bool blk_flush_complete_seq(struct request *rq,
 	BUG_ON(rq->flush.seq & seq);
 	rq->flush.seq |= seq;
 
-	if (likely(!error))
+	if (likely(!error)) {
 		seq = blk_flush_cur_seq(rq);
-	else
+	} else {
 		seq = REQ_FSEQ_DONE;
+		printk_ratelimited(KERN_ERR "%s: flush failed: data integrity problem\n",
+				   rq->rq_disk ? rq->rq_disk->disk_name : "?");
+		/*
+		 * returning an error to the FS is wrong: the data is all
+		 * there, it just might not be written out in the expected
+		 * order and thus have a window where the integrity is suspect
+		 * in a crash.  Given the small likelihood of actually
+		 * crashing, we should just log a warning here.
+		 */
+		error = 0;
+	}
 
 	switch (seq) {
 	case REQ_FSEQ_PREFLUSH:


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux