+ fat-add-nobarrier-to-workaround-the-strange-behavior-of-device.patch added to -mm tree

+ fat-add-nobarrier-to-workaround-the-strange-behavior-of-device.patch added to -mm tree

[akpm]

The patch titled
     Subject: fat: add nobarrier to workaround the strange behavior of device
has been added to the -mm tree.  Its filename is
     fat-add-nobarrier-to-workaround-the-strange-behavior-of-device.patch

This patch should soon appear at
    http://ozlabs.org/~akpm/mmots/broken-out/fat-add-nobarrier-to-workaround-the-strange-behavior-of-device.patch
and later at
    http://ozlabs.org/~akpm/mmotm/broken-out/fat-add-nobarrier-to-workaround-the-strange-behavior-of-device.patch

Before you just go and hit "reply", please:
   a) Consider who else should be cc'ed
   b) Prefer to cc a suitable mailing list as well
   c) Ideally: find the original patch on the mailing list and do a
      reply-to-all to that, adding suitable additional cc's

*** Remember to use Documentation/process/submit-checklist.rst when testing your code ***

The -mm tree is included into linux-next and is updated
there every 3-4 working days

------------------------------------------------------
From: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
Subject: fat: add nobarrier to workaround the strange behavior of device

There was a report of strange behavior of device with recent
blkdev_issue_flush() position change.

The following is simplified usbmon trace:

 4203   9.160230         host -> 1.25.1       USBMS 95 SCSI: Synchronize Cache(10) LUN: 0x00 (LBA: 0x00000000, Len: 0)
 4206   9.164911       1.25.1 -> host         USBMS 77 SCSI: Response LUN: 0x00 (Synchronize Cache(10)) (Good)
 4207   9.323927         host -> 1.25.1       USBMS 95 SCSI: Read(10) LUN: 0x00 (LBA: 0x00279950, Len: 240)
 4212   9.327138       1.25.1 -> host         USBMS 77 SCSI: Response LUN: 0x00 (Read(10)) (Good)

[...]

 7323  10.202167         host -> 1.25.1       USBMS 95 SCSI: Synchronize Cache(10) LUN: 0x00 (LBA: 0x00000000, Len: 0)
 7326  10.432266       1.25.1 -> host         USBMS 77 SCSI: Response LUN: 0x00 (Synchronize Cache(10)) (Good)
 7327  10.769092         host -> 1.25.1       USBMS 95 SCSI: Test Unit Ready LUN: 0x00 
 7330  10.769192       1.25.1 -> host         USBMS 77 SCSI: Response LUN: 0x00 (Test Unit Ready) (Good)
 7335  12.849093         host -> 1.25.1       USBMS 95 SCSI: Test Unit Ready LUN: 0x00 
 7338  12.849206       1.25.1 -> host         USBMS 77 SCSI: Response LUN: 0x00 (Test Unit Ready) (Check Condition)
 7339  12.849209         host -> 1.25.1       USBMS 95 SCSI: Request Sense LUN: 0x00

If "Synchronize Cache" command issued then there is idle time, the device
stop to process further commands, and behave as like no media.  (it
returns NOT_READY [MEDIUM NOT PRESENT] for SENSE command, and this
happened on Kindle) [just a guess, the device is trying to detect the
"safe-unplug" operation of Windows or such?]

To work around those devices and provide flexibility, this adds
"barrier"/"nobarrier" mount options to fat driver.

Link: http://lkml.kernel.org/r/87woh5pyqh.fsf@mail.parknet.co.jp
Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
Cc: Christoph Hellwig <hch@lst.de>
Cc: <stable@vger.kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 fs/fat/fat.h   |    1 +
 fs/fat/file.c  |    8 ++++++--
 fs/fat/inode.c |   22 +++++++++++++++++-----
 3 files changed, 24 insertions(+), 7 deletions(-)

--- a/fs/fat/fat.h~fat-add-nobarrier-to-workaround-the-strange-behavior-of-device
+++ a/fs/fat/fat.h
@@ -51,6 +51,7 @@ struct fat_mount_options {
 		 tz_set:1,	   /* Filesystem timestamps' offset set */
 		 rodir:1,	   /* allow ATTR_RO for directory */
 		 discard:1,	   /* Issue discard requests on deletions */
+		 barrier:1,	   /* Issue FLUSH command */
 		 dos1xfloppy:1;	   /* Assume default BPB for DOS 1.x floppies */
 };
 
--- a/fs/fat/file.c~fat-add-nobarrier-to-workaround-the-strange-behavior-of-device
+++ a/fs/fat/file.c
@@ -194,17 +194,21 @@ static int fat_file_release(struct inode
 int fat_file_fsync(struct file *filp, loff_t start, loff_t end, int datasync)
 {
 	struct inode *inode = filp->f_mapping->host;
+	struct msdos_sb_info *sbi = MSDOS_SB(inode->i_sb);
 	int err;
 
 	err = __generic_file_fsync(filp, start, end, datasync);
 	if (err)
 		return err;
 
-	err = sync_mapping_buffers(MSDOS_SB(inode->i_sb)->fat_inode->i_mapping);
+	err = sync_mapping_buffers(sbi->fat_inode->i_mapping);
 	if (err)
 		return err;
 
-	return blkdev_issue_flush(inode->i_sb->s_bdev, GFP_KERNEL, NULL);
+	if (sbi->options.barrier)
+		err = blkdev_issue_flush(inode->i_sb->s_bdev, GFP_KERNEL, NULL);
+
+	return err;
 }
 
 
--- a/fs/fat/inode.c~fat-add-nobarrier-to-workaround-the-strange-behavior-of-device
+++ a/fs/fat/inode.c
@@ -1011,6 +1011,8 @@ static int fat_show_options(struct seq_f
 		seq_puts(m, ",nfs=stale_rw");
 	if (opts->discard)
 		seq_puts(m, ",discard");
+	if (!opts->barrier)
+		seq_puts(m, ",nobarrier");
 	if (opts->dos1xfloppy)
 		seq_puts(m, ",dos1xfloppy");
 
@@ -1026,8 +1028,9 @@ enum {
 	Opt_shortname_winnt, Opt_shortname_mixed, Opt_utf8_no, Opt_utf8_yes,
 	Opt_uni_xl_no, Opt_uni_xl_yes, Opt_nonumtail_no, Opt_nonumtail_yes,
 	Opt_obsolete, Opt_flush, Opt_tz_utc, Opt_rodir, Opt_err_cont,
-	Opt_err_panic, Opt_err_ro, Opt_discard, Opt_nfs, Opt_time_offset,
-	Opt_nfs_stale_rw, Opt_nfs_nostale_ro, Opt_err, Opt_dos1xfloppy,
+	Opt_err_panic, Opt_err_ro, Opt_discard, Opt_barrier, Opt_nobarrier,
+	Opt_nfs, Opt_time_offset, Opt_nfs_stale_rw, Opt_nfs_nostale_ro,
+	Opt_err, Opt_dos1xfloppy,
 };
 
 static const match_table_t fat_tokens = {
@@ -1057,6 +1060,8 @@ static const match_table_t fat_tokens =
 	{Opt_err_panic, "errors=panic"},
 	{Opt_err_ro, "errors=remount-ro"},
 	{Opt_discard, "discard"},
+	{Opt_barrier, "barrier"},
+	{Opt_nobarrier, "nobarrier"},
 	{Opt_nfs_stale_rw, "nfs"},
 	{Opt_nfs_stale_rw, "nfs=stale_rw"},
 	{Opt_nfs_nostale_ro, "nfs=nostale_ro"},
@@ -1141,6 +1146,7 @@ static int parse_options(struct super_bl
 	opts->numtail = 1;
 	opts->usefree = opts->nocase = 0;
 	opts->tz_set = 0;
+	opts->barrier = 1;
 	opts->nfs = 0;
 	opts->errors = FAT_ERRORS_RO;
 	*debug = 0;
@@ -1264,6 +1270,15 @@ static int parse_options(struct super_bl
 		case Opt_err_ro:
 			opts->errors = FAT_ERRORS_RO;
 			break;
+		case Opt_discard:
+			opts->discard = 1;
+			break;
+		case Opt_barrier:
+			opts->barrier = 1;
+			break;
+		case Opt_nobarrier:
+			opts->barrier = 0;
+			break;
 		case Opt_nfs_stale_rw:
 			opts->nfs = FAT_NFS_STALE_RW;
 			break;
@@ -1327,9 +1342,6 @@ static int parse_options(struct super_bl
 		case Opt_rodir:
 			opts->rodir = 1;
 			break;
-		case Opt_discard:
-			opts->discard = 1;
-			break;
 
 		/* obsolete mount options */
 		case Opt_obsolete:
_

Patches currently in -mm which might be from hirofumi@mail.parknet.co.jp are

fat-add-nobarrier-to-workaround-the-strange-behavior-of-device.patch

Re: + fat-add-nobarrier-to-workaround-the-strange-behavior-of-device.patch added to -mm tree

[Christoph Hellwig]

I still very fundamentally disagree with this patch.  We did a concerted
effort around the other file systems to move to the device level tweak
and remove the badly misnamed option, so we should not add it now for
fat.

Re: + fat-add-nobarrier-to-workaround-the-strange-behavior-of-device.patch added to -mm tree

[Andrew Morton]

On Wed, 3 Jul 2019 00:50:53 +0200 Christoph Hellwig <hch@lst.de> wrote:

> I still very fundamentally disagree with this patch.  We did a concerted
> effort around the other file systems to move to the device level tweak
> and remove the badly misnamed option, so we should not add it now for
> fat.

OK, thanks, I'll leave it on hold for now.

Re: + fat-add-nobarrier-to-workaround-the-strange-behavior-of-device.patch added to -mm tree

[OGAWA Hirofumi]

Andrew Morton <akpm@linux-foundation.org> writes:

> On Wed, 3 Jul 2019 00:50:53 +0200 Christoph Hellwig <hch@lst.de> wrote:
>
>> I still very fundamentally disagree with this patch.  We did a concerted
>> effort around the other file systems to move to the device level tweak
>> and remove the badly misnamed option, so we should not add it now for
>> fat.
>
> OK, thanks, I'll leave it on hold for now.

I've checked v5.1, you mentioned "cache_type" (scsi specific
unfortunately). It calls blk_queue_write_cache(), so it calls
wbt_set_write_cache() too. So it affects to writeback throttling.

Is this intended one? At least, it was not what I expected. It is why I
asked side effect of it repeatedly in that thread.

True candidate looks like block layer's "write_cache" introduced after
v4.6. It doesn't call wbt_set_write_cache(). However, I worry if this is
intent or bug, behave like "cache_type". Did this get consensus to
replace "nobarrier" (I hope so)?

If not, I even can't recommend "write_cache", instead of "nobarrier" as
documenting, and we need real replacement to just disable flush command.

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>