| Message ID | 1434987827.2237.71.camel@HansenPartnership.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, 22 Jun 2015, James Bottomley wrote: > 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. I agree. Doing the switch manually (by writing to the "cache_type" attribute file) works, but it's a nuisance to do this when you have a portable USB drive that gets moved among a bunch of machines. > 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? I would be very surprised if any of those drives support MODE SELECT at all. Maybe your patch will be acceptable, though. We'll have to hear from Markus and Matt. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
On Mon, 2015-06-22 at 13:30 -0400, Alan Stern wrote: > On Mon, 22 Jun 2015, James Bottomley wrote: > > > 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. > > I agree. Doing the switch manually (by writing to the "cache_type" > attribute file) works, but it's a nuisance to do this when you have a > portable USB drive that gets moved among a bunch of machines. Perhaps it might be wise to do this to every USB device ... for external devices, the small performance gain doesn't really make up for the potential data loss. > > 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? > > I would be very surprised if any of those drives support MODE SELECT at > all. I assume the cache type attribute file you refer to above is just pretending their cache is write through rather than actually setting it to be so? The original IDE device had no way of turning their cache types to write through either, but the manufacturers were eventually convinced of the error of their ways. > Maybe your patch will be acceptable, though. We'll have to hear from > Markus and Matt. We'll probably have to take this to fsdevel as well ... they might have opinions about whether the FS wants to be informed about flush failure. James > Alan Stern > > -- > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in > -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
On Mon, 22 Jun 2015, James Bottomley wrote: > On Mon, 2015-06-22 at 13:30 -0400, Alan Stern wrote: > > On Mon, 22 Jun 2015, James Bottomley wrote: > > > > > 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. > > > > I agree. Doing the switch manually (by writing to the "cache_type" > > attribute file) works, but it's a nuisance to do this when you have a > > portable USB drive that gets moved among a bunch of machines. > > Perhaps it might be wise to do this to every USB device ... for external > devices, the small performance gain doesn't really make up for the > potential data loss. > > > > 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? > > > > I would be very surprised if any of those drives support MODE SELECT at > > all. > > I assume the cache type attribute file you refer to above is just > pretending their cache is write through rather than actually setting it > to be so? Yes; I'm referring to cache_type_store() in sd.c, and writing "temporary write through", which does not issue a MODE SELECT command. It would be easy enough for people to try leaving out the "temporary", but I don't expect it to work. > The original IDE device had no way of turning their cache > types to write through either, but the manufacturers were eventually > convinced of the error of their ways. In this case the stupidity resides in the USB-ATA bridge. You can see the gory details at https://bugzilla.kernel.org/show_bug.cgi?id=89511#c19 > > Maybe your patch will be acceptable, though. We'll have to hear from > > Markus and Matt. > > We'll probably have to take this to fsdevel as well ... they might have > opinions about whether the FS wants to be informed about flush failure. Okay. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
>> > Maybe your patch will be acceptable, though. We'll have to hear from >> > Markus and Matt. >> >> We'll probably have to take this to fsdevel as well ... they might have >> opinions about whether the FS wants to be informed about flush failure. So, it is okay to wait for the end of that discussion before I start further testing or should I test the patch above? -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
On Mon, 22 Jun 2015, Markus Rathgeb wrote: > >> > Maybe your patch will be acceptable, though. We'll have to hear from > >> > Markus and Matt. > >> > >> We'll probably have to take this to fsdevel as well ... they might have > >> opinions about whether the FS wants to be informed about flush failure. > > So, it is okay to wait for the end of that discussion before I start > further testing or should I test the patch above? Please test it now. I'd like to know if it fixes your problem, regardless of how the discussion goes. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
On Mon, 2015-06-22 at 13:48 -0400, Alan Stern wrote: > On Mon, 22 Jun 2015, James Bottomley wrote: > > > On Mon, 2015-06-22 at 13:30 -0400, Alan Stern wrote: > > > On Mon, 22 Jun 2015, James Bottomley wrote: > > > > > > > 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. > > > > > > I agree. Doing the switch manually (by writing to the "cache_type" > > > attribute file) works, but it's a nuisance to do this when you have a > > > portable USB drive that gets moved among a bunch of machines. > > > > Perhaps it might be wise to do this to every USB device ... for external > > devices, the small performance gain doesn't really make up for the > > potential data loss. > > > > > > 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? > > > > > > I would be very surprised if any of those drives support MODE SELECT at > > > all. > > > > I assume the cache type attribute file you refer to above is just > > pretending their cache is write through rather than actually setting it > > to be so? > > Yes; I'm referring to cache_type_store() in sd.c, and writing > "temporary write through", which does not issue a MODE SELECT command. > It would be easy enough for people to try leaving out the "temporary", > but I don't expect it to work. > > > The original IDE device had no way of turning their cache > > types to write through either, but the manufacturers were eventually > > convinced of the error of their ways. > > In this case the stupidity resides in the USB-ATA bridge. You can see > the gory details at > > https://bugzilla.kernel.org/show_bug.cgi?id=89511#c19 OK, so that says the SAT in the bridge doesn't know what to do with MODE_SELECT (probably unsurprising given that it's a usb bridge). the SATA disk should respond to the ATA command SET FEATURES, though. Presuming we can get it through the bridge. You can try it with hdparm -W 0 <dev> optionally with --prefer_ata_12 to do the 12 instead of 16 byte encapsulation and see if that makes a difference. James -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> Please test it now. I'd like to know if it fixes your problem, > regardless of how the discussion goes. Seems to be working, I also attached the kernel log. The "flush failed" will grow with disc activity (but that was to be expected). Jun 22 23:24:50 m3800 kernel: usb 1-3: new high-speed USB device number 6 using xhci_hcd Jun 22 23:24:51 m3800 kernel: usb 1-3: New USB device found, idVendor=04cf, idProduct=8818 Jun 22 23:24:51 m3800 kernel: usb 1-3: New USB device strings: Mfr=1, Product=2, SerialNumber=3 Jun 22 23:24:51 m3800 kernel: usb 1-3: Product: USB Mass Storage Device Jun 22 23:24:51 m3800 kernel: usb 1-3: Manufacturer: Myson Century, Inc. Jun 22 23:24:51 m3800 kernel: usb 1-3: SerialNumber: 100 Jun 22 23:24:51 m3800 kernel: usb-storage 1-3:1.0: USB Mass Storage device detected Jun 22 23:24:51 m3800 kernel: scsi host6: usb-storage 1-3:1.0 Jun 22 23:24:51 m3800 kernel: usbcore: registered new interface driver usb-storage Jun 22 23:24:52 m3800 kernel: scsi 6:0:0:0: Direct-Access SAMSUNG MP0804H UE10 PQ: 0 ANSI: 0 CCS Jun 22 23:24:52 m3800 kernel: sd 6:0:0:0: Attached scsi generic sg1 type 0 Jun 22 23:24:52 m3800 kernel: sd 6:0:0:0: [sdb] 156368016 512-byte logical blocks: (80.0 GB/74.5 GiB) Jun 22 23:24:52 m3800 kernel: sd 6:0:0:0: [sdb] Write Protect is off Jun 22 23:24:52 m3800 kernel: sd 6:0:0:0: [sdb] Mode Sense: 00 14 00 00 Jun 22 23:24:52 m3800 kernel: sd 6:0:0:0: [sdb] Write cache: enabled, read cache: enabled, doesn't support DPO or FUA Jun 22 23:24:52 m3800 kernel: sdb: sdb1 sdb2 Jun 22 23:24:52 m3800 kernel: sd 6:0:0:0: [sdb] Attached SCSI disk Jun 22 23:24:52 m3800 kernel: BTRFS: device label USB_BILDER devid 1 transid 9 /dev/sdb2 Jun 22 23:24:52 m3800 kernel: BTRFS info (device sdb2): disk space caching is enabled Jun 22 23:24:52 m3800 kernel: BTRFS: has skinny extents Jun 22 23:24:52 m3800 kernel: FAT-fs (sdb1): Volume was not properly unmounted. Some data may be corrupt. Please run fsck. Jun 22 23:25:28 m3800 kernel: sdb: flush failed: data integrity problem Jun 22 23:25:28 m3800 kernel: sdb: flush failed: data integrity problem Jun 22 23:26:55 m3800 kernel: sdb: flush failed: data integrity problem Jun 22 23:26:55 m3800 kernel: sdb: flush failed: data integrity problem -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
RnJvbTogT2YgSmFtZXMgQm90dG9tbGV5DQo+IFNlbnQ6IDIyIEp1bmUgMjAxNSAxODozNg0KPiBU bzogQWxhbiBTdGVybg0KLi4uDQo+ID4gPiBPYnZpb3VzbHksIGZvciBhIGRpc2sgd2l0aCBhIHdy aXRlYmFjayBjYWNoZSB0aGF0IGNhbid0IGRvIGZsdXNoLCB0aGF0DQo+ID4gPiB3aW5kb3cgaXMg bXVjaCB3aWRlciBhbmQgdGhlIHJlYWwgc29sdXRpb24gc2hvdWxkIGJlIHRvIHRyeSB0byBzd2l0 Y2gNCj4gPiA+IHRoZSBjYWNoZSB0byB3cml0ZSB0aHJvdWdoLg0KPiA+DQo+ID4gSSBhZ3JlZS4g IERvaW5nIHRoZSBzd2l0Y2ggbWFudWFsbHkgKGJ5IHdyaXRpbmcgdG8gdGhlICJjYWNoZV90eXBl Ig0KPiA+IGF0dHJpYnV0ZSBmaWxlKSB3b3JrcywgYnV0IGl0J3MgYSBudWlzYW5jZSB0byBkbyB0 aGlzIHdoZW4geW91IGhhdmUgYQ0KPiA+IHBvcnRhYmxlIFVTQiBkcml2ZSB0aGF0IGdldHMgbW92 ZWQgYW1vbmcgYSBidW5jaCBvZiBtYWNoaW5lcy4NCj4gDQo+IFBlcmhhcHMgaXQgbWlnaHQgYmUg d2lzZSB0byBkbyB0aGlzIHRvIGV2ZXJ5IFVTQiBkZXZpY2UgLi4uIGZvciBleHRlcm5hbA0KPiBk ZXZpY2VzLCB0aGUgc21hbGwgcGVyZm9ybWFuY2UgZ2FpbiBkb2Vzbid0IHJlYWxseSBtYWtlIHVw IGZvciB0aGUNCj4gcG90ZW50aWFsIGRhdGEgbG9zcy4NCg0KV2hhdCBhYm91dCBzeXN0ZW1zIHJ1 bm5pbmcgZnJvbSBVU0IgbWVtb3J5L2Rpc2sgZGlyZWN0bHkgcGx1Z2dlZCBpbnRvIHRoZQ0KbW90 aGVyYm9hcmQgYW5kIHNodXQgaW5zaWRlIHRoZSBjYXNlPw0KDQpTaW5jZSB0aGV5IHNob3VsZG4n dCBiZSByZW1vdmVkIHlvdSBkb24ndCB3YW50IHRvIGJ5cGFzcyBhbnkgd3JpdGUgY2FjaGUuDQoo QW55IG1vcmUgdGhhbiB5b3UnZCB3YW50IHRvIG9uIGEgcmVhbCBkaXNrLikNCg0KVGhlcmUgaXMg YW4gYWRkaXRpb25hbCBwcm9ibGVtIGNhdXNlZCBieSB2ZXJ5IHRlbXBvcmFyeSBVU0IgZGlzY29u bmVjdHMNCihlYXNpbHkgY2F1c2VkIGJ5IGVsZWN0cmljYWwgbm9pc2UgY2F1c2luZyAocHJvYmFi bHkpIFZidXMgdG8gYm91bmNlKS4NCkluIHRoZXNlIGNvbmRpdGlvbnMgeW91IGRvbid0IHdhbnQg dG8gc2lnbmFsIGEgVVNCIHJlbW92ZSBhdCBhbGwgLSBhdA0KbGVhc3Qgbm90IHRvIHRoZSBmaWxl c3lzdGVtIC0gdW50aWwgdGhlIGRldmljZSBoYXMgcmVtYWluZWQgZGlzY29ubmVjdGVkDQpmb3Ig YSBzaG9ydCB0aW1lLg0KDQoJRGF2aWQNCg0K -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Jun 22 James Bottomley wrote: > On Mon, 2015-06-22 at 13:30 -0400, Alan Stern wrote: > > On Mon, 22 Jun 2015, James Bottomley wrote: > > > 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. > > > > I agree. Doing the switch manually (by writing to the "cache_type" > > attribute file) works, but it's a nuisance to do this when you have a > > portable USB drive that gets moved among a bunch of machines. > > Perhaps it might be wise to do this to every USB device ... for external > devices, the small performance gain doesn't really make up for the > potential data loss. Just a small note on the assumption of externally (and in extension, temporarily) attached devices: Not all USB-attached devices are external, and not all external devices are used as removable devices. (For example, I am using 2 internal CD-ROMs via USB-2 + usb-storage and 2 internal HDDs via USB 3 + uas in a PC with too few SATA ports and no PCIe slot to spare for a controller add-on card, but plenty of USB headers available on the mainboard. Similarly, some NASes have their operating system located on a USB-attached device. Small offices use USB-attached disks for backup and won't detach such a disk until rotation for off-site deposit. Not to mention embedded computers with USB-attached but fixed disks.)
On Fri, 2015-06-26 at 11:43 +0200, Stefan Richter wrote: > On Jun 22 James Bottomley wrote: > > On Mon, 2015-06-22 at 13:30 -0400, Alan Stern wrote: > > > On Mon, 22 Jun 2015, James Bottomley wrote: > > > > 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. > > > > > > I agree. Doing the switch manually (by writing to the "cache_type" > > > attribute file) works, but it's a nuisance to do this when you have a > > > portable USB drive that gets moved among a bunch of machines. > > > > Perhaps it might be wise to do this to every USB device ... for external > > devices, the small performance gain doesn't really make up for the > > potential data loss. > > Just a small note on the assumption of externally (and in extension, > temporarily) attached devices: Not all USB-attached devices are external, > and not all external devices are used as removable devices. The problems don't depend on the connection type: internal devices which have a writeback cache and don't accept flush commands have data integrity problems too. I can't really think of many situations where you'd be willing to sacrifice data integrity for performance. James > (For example, I am using 2 internal CD-ROMs via USB-2 + usb-storage and 2 > internal HDDs via USB 3 + uas in a PC with too few SATA ports and no PCIe > slot to spare for a controller add-on card, but plenty of USB headers > available on the mainboard. Similarly, some NASes have their operating > system located on a USB-attached device. Small offices use USB-attached > disks for backup and won't detach such a disk until rotation for off-site > deposit. Not to mention embedded computers with USB-attached but fixed > disks.) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Jun 26 James Bottomley wrote: > On Fri, 2015-06-26 at 11:43 +0200, Stefan Richter wrote: > > On Jun 22 James Bottomley wrote: [...] > > > Perhaps it might be wise to do this to every USB device ... for external > > > devices, the small performance gain doesn't really make up for the > > > potential data loss. > > > > Just a small note on the assumption of externally (and in extension, > > temporarily) attached devices: Not all USB-attached devices are external, > > and not all external devices are used as removable devices. > > The problems don't depend on the connection type: internal devices which > have a writeback cache and don't accept flush commands have data > integrity problems too. I can't really think of many situations where > you'd be willing to sacrifice data integrity for performance. Sure; writeback caches are prone to more issues besides sudden connection loss. (E.g. sudden power loss is but one of several more potential issues of course.) I merely wanted to remind that the bus type of a device (USB or whatever) does not say much about the risk of sudden connection loss to that device, since such devices may have physical protection against sudden connection loss too. I was not particularly commenting on the topic at hand, i.e. on presence of writeback cache without working flush command.
On Mon, 22 Jun 2015, James Bottomley wrote: > On Mon, 2015-06-22 at 13:48 -0400, Alan Stern wrote: > > On Mon, 22 Jun 2015, James Bottomley wrote: > > > > > On Mon, 2015-06-22 at 13:30 -0400, Alan Stern wrote: > > > > On Mon, 22 Jun 2015, James Bottomley wrote: > > > > > > > > > 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. > > > > > > > > I agree. Doing the switch manually (by writing to the "cache_type" > > > > attribute file) works, but it's a nuisance to do this when you have a > > > > portable USB drive that gets moved among a bunch of machines. > > > > > > Perhaps it might be wise to do this to every USB device ... for external > > > devices, the small performance gain doesn't really make up for the > > > potential data loss. > > > > > > > > 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? > > > > > > > > I would be very surprised if any of those drives support MODE SELECT at > > > > all. > > > > > > I assume the cache type attribute file you refer to above is just > > > pretending their cache is write through rather than actually setting it > > > to be so? > > > > Yes; I'm referring to cache_type_store() in sd.c, and writing > > "temporary write through", which does not issue a MODE SELECT command. > > It would be easy enough for people to try leaving out the "temporary", > > but I don't expect it to work. > > > > > The original IDE device had no way of turning their cache > > > types to write through either, but the manufacturers were eventually > > > convinced of the error of their ways. > > > > In this case the stupidity resides in the USB-ATA bridge. You can see > > the gory details at > > > > https://bugzilla.kernel.org/show_bug.cgi?id=89511#c19 > > OK, so that says the SAT in the bridge doesn't know what to do with > MODE_SELECT (probably unsurprising given that it's a usb bridge). the > SATA disk should respond to the ATA command SET FEATURES, though. > Presuming we can get it through the bridge. > > You can try it with > > hdparm -W 0 <dev> > > optionally with --prefer_ata_12 to do the 12 instead of 16 byte > encapsulation and see if that makes a difference. Dan Williams recently posted the content below to the Bugzilla report: > I have a drive with this problem and I tested 4.0.8 with this hunk > added to the patch above. > > UNUSUAL_DEV( 0x0bc2, 0x0888, 0x0000, 0x0000, > "Seagate", > "USB 2.0 Pocket Hard Drive", > USB_SC_DEVICE, USB_PR_DEVICE, NULL, > US_FL_NO_SYNCHRONIZE_CACHE), > > I don't have the linux-usb thread around so I can reply to > http://www.spinics.net/lists/linux-usb/msg126364.html, but here's the > result of: > > [dcbw@localhost ~]$ sudo hdparm -W 0 --prefer-ata12 /dev/sdb > > /dev/sdb: > setting drive write-caching to 0 (off) > SG_IO: bad/missing sense data, sb[]: 70 00 05 00 00 00 00 0a 00 00 00 > 00 20 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 In English: "Illegal Request: Invalid command operation code". Apparently there's no way to tell the drive to change its caching. > SG_IO: bad/missing sense data, sb[]: 70 00 05 00 00 00 00 0a 00 00 00 > 00 20 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > SG_IO: bad/missing sense data, sb[]: 70 00 05 00 00 00 00 0a 00 00 00 > 00 20 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > SG_IO: bad/missing sense data, sb[]: 70 00 05 00 00 00 00 0a 00 00 00 > 00 20 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > SG_IO: bad/missing sense data, sb[]: 70 00 05 00 00 00 00 0a 00 00 00 > 00 20 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > write-caching = not supported > > What was the upstream resolution on this one? So far there isn't any. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
This is an old problem, but it was never resolved and it still affects people (Bugzilla #89511). In short, there are USB-(S)ATA bridges that claim to be write-back but don't support the SYNCHRONIZE CACHE command. This causes errors when filesystems try to flush data out to the disk. On Mon, 22 Jun 2015, James Bottomley wrote: > On Mon, 2015-06-22 at 13:48 -0400, Alan Stern wrote: > > On Mon, 22 Jun 2015, James Bottomley wrote: > > > > > On Mon, 2015-06-22 at 13:30 -0400, Alan Stern wrote: > > > > On Mon, 22 Jun 2015, James Bottomley wrote: > > > > > > > > > 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. > > > > > > > > I agree. Doing the switch manually (by writing to the "cache_type" > > > > attribute file) works, but it's a nuisance to do this when you have a > > > > portable USB drive that gets moved among a bunch of machines. > > > > > > Perhaps it might be wise to do this to every USB device ... for external > > > devices, the small performance gain doesn't really make up for the > > > potential data loss. > > > > > > > > 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? > > > > > > > > I would be very surprised if any of those drives support MODE SELECT at > > > > all. > > > > > > I assume the cache type attribute file you refer to above is just > > > pretending their cache is write through rather than actually setting it > > > to be so? > > > > Yes; I'm referring to cache_type_store() in sd.c, and writing > > "temporary write through", which does not issue a MODE SELECT command. > > It would be easy enough for people to try leaving out the "temporary", > > but I don't expect it to work. > > > > > The original IDE device had no way of turning their cache > > > types to write through either, but the manufacturers were eventually > > > convinced of the error of their ways. > > > > In this case the stupidity resides in the USB-ATA bridge. You can see > > the gory details at > > > > https://bugzilla.kernel.org/show_bug.cgi?id=89511#c19 > > OK, so that says the SAT in the bridge doesn't know what to do with > MODE_SELECT (probably unsurprising given that it's a usb bridge). the > SATA disk should respond to the ATA command SET FEATURES, though. > Presuming we can get it through the bridge. > > You can try it with > > hdparm -W 0 <dev> > > optionally with --prefer_ata_12 to do the 12 instead of 16 byte > encapsulation and see if that makes a difference. As reported by Dan Williams, the SET FEATURES command results in an "Illegal Request: Invalid command operation code" error response. So the question remains, what should we do about these things? We can have a blacklist flag that overrides the cache setting, as in my original patch. We can leave the setting alone and change the errors to warnings, as in James's patch earlier in this thread (but that will generate a lot of warnings). James, what do you think? Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 2015-12-03 at 13:36 -0500, Alan Stern wrote: > This is an old problem, but it was never resolved and it still affects > people (Bugzilla #89511). In short, there are USB-(S)ATA bridges that > claim to be write-back but don't support the SYNCHRONIZE CACHE > command. > This causes errors when filesystems try to flush data out to the disk. OK, maybe this is a stupid idea, but could we test the command as we enumerate the slave and store the result? Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 15 Dec 2015, Oliver Neukum wrote: > On Thu, 2015-12-03 at 13:36 -0500, Alan Stern wrote: > > This is an old problem, but it was never resolved and it still affects > > people (Bugzilla #89511). In short, there are USB-(S)ATA bridges that > > claim to be write-back but don't support the SYNCHRONIZE CACHE > > command. > > This causes errors when filesystems try to flush data out to the disk. > > OK, maybe this is a stupid idea, but could we test the command as we > enumerate the slave and store the result? Maybe, although doing so within usb-storage would be kind of difficult -- that driver is set up to forward requests from the SCSI layer, not to generate requests of its own. Besides, usb-storage doesn't know anything about write-back vs. write-through; only sd does. I suppose sd could perform that test. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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: