Re: vfs: Add MS_FLUSHONFSYNC mount flag

From: Dave Chinner <david@fromorbit.com>
To: Fernando Luis Vazquez Cao <fernando@kic.ac.jp>
Cc: "Eric Sandeen" <sandeen@redhat.com>,
	"Fernando Luis Vázquez Cao" <fernando@oss.ntt.co.jp>,
	"Jan Kara" <jack@suse.cz>, "Theodore Tso" <tytso@MIT.EDU>,
	"Alan Cox" <alan@lxorguk.ukuu.org.uk>,
	"Pavel Machek" <pavel@suse.cz>,
	"kernel list" <linux-kernel@vger.kernel.org>,
	"Jens Axboe" <jens.axboe@oracle.com>,
	"Ric Wheeler" <rwheeler@redhat.com>
Subject: Re: vfs: Add MS_FLUSHONFSYNC mount flag
Date: Sat, 14 Feb 2009 22:24:43 +1100	[thread overview]
Message-ID: <20090214112443.GY8830@disturbed> (raw)
In-Reply-To: <1234542568.9916.183.camel@bladerunner>

On Sat, Feb 14, 2009 at 01:29:28AM +0900, Fernando Luis Vazquez Cao wrote:
> On Fri, 2009-02-13 at 23:20 +1100, Dave Chinner wrote:
> > On Fri, Feb 13, 2009 at 12:20:17AM -0600, Eric Sandeen wrote:
> > > I'm just a little leery of the "dangerous" mount option proliferation, I
> > > guess.
> > 
> > You're not the only one, Eric. It's bad enough having to explain to
> > users what barriers do once they have lost data after a power loss,
> > let alone confusing them further by adding more mount options they
> > will get wrong by accident....
> 
> That is precisely the reason why we should use sensible defaults, which
> in this case means enabling barriers and flushing disk caches on
> fsync()/fdatasync() by default.
> 
> Adding either a new mount option (as you yourself suggest below) or a
> sysfs tunable is desirable for those cases when we really do not need to
> flush the disk write cache to guarantee integrity (battery-backed block
> devices come to mind), or we want to be fast at the cost of potentially
> losing some data.

Mount options are the wrong place for this. if you want to change
the behaviour of the block device, then it should be at that level.

> > Quite frankly, the VFS should do stuff that is slow and safe
> > and filesystems can choose to ignore the VFS (via filesystem
> > specific mount options) if they want to be fast and potentially
> > unsafe.
> 
> To avoid unnecessary flushes and allow for filesystem-specific
> optimizations I was considering the following approach:
> 
> 1- Add flushonfsync mount option (as an aside, I am of the opinion that
> it should be set by default).

No mount option - too confusing for someone to work out what
combination of barriers and flushing for things to work correctly.
Just make filesystems issue the necessary flush calls or barrier
IOs and allow the block devices to ignore flushes.

> 2- Modify file_fsync() so that it checks whether FLUSHONFSYNC is set and
> flushes the underlying device accordingly. With this we would cover all
> filesystems that use the vfs-provided file_fsync() as their fsync method
> (commonly used filesystems such as fat fall in this group).

Just make it flush the block device.

> 3- Advanced filesystems (ext3/4, XFS, btrfs, etc) which provide their
> own fsync implementations are allowed to perform filesystem-specific
> optimizations there to minimize the number of flushes and maximize
> throughput.

Um, you are describing what we already have in place.  Almost every
filesystem provides it's own ->fsync method, not just the "advanced"
ones. It is those methods that need to be fixed to issue flushes, not just
file_fsync().

> In this patch set I implemented (1) and (3) for ext3/4 to have some code
> to comment on.

I don't think we want (1) at all, and I thought that if ext3/4 are using
barriers then the barrier I/O issued by the journal does the flush
already. Hence (3) is redundant, right?

FWIW, block device flushes are implemented by barrier IOs, so if
the underlying block device doesn't support barriers then you can't
flush the cache, either...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com