Re: [PATCH 2/2] dax: fix bdev NULL pointer dereferences

[Dave Chinner]

On Fri, Feb 05, 2016 at 03:25:00PM -0700, Ross Zwisler wrote:
> On Thu, Feb 04, 2016 at 09:29:57PM +0100, Jan Kara wrote:
> > I think changes aren't very intrusive so we can feed them in during RC
> > phase and frankly, you have to move to using ->writepages() anyway to make
> > sync(2) work reliably.
> 
> I've been looking into this a bit more, and I don't think we actually want to
> have DAX flushing in response to sync(2) or syncfs(2).  Here's some text from
> the BUGS section of the sync(2) man page:
> 
> BUGS
> 	According to the standard specification (e.g., POSIX.1-2001), sync()
> 	schedules the writes,  but  may  return before  the  actual  writing
> 	is done.  However, since version 1.3.20 Linux does actually wait.
> 	(This still does not guarantee data integrity: modern disks have large
> 	caches.) 
> 
> Based on this I don't believe that it is a requirement that sync and syncfs
> actually flush the data durably to media before they return - they just need
> to make sure it has been sent to the device, which is always true for all
> writes PMEM via DAX, even without any calls to dax_writeback_mapping_range().

That's an assumption we've already pointed out as being platform
dependent, not to mention also being undesirable from a performance
point of view (writes are 10x slower into pmem than into the page
cache using the same physical memory!).

Further, the ordering constraints of modern filesystems mean that
they guarantee durability of data written back when sync() is run.
i.e.  ext4, XFS, btrfs, etc all ensure that sync guarantees data
integrity is maintained across all the data and metadata written
back during sync().

e.g. for XFS we do file size update transactions at IO completion.
sync() triggers writeback of data, then runs a transaction that
modifies the file size so that the data is valid on disk. We
absolutely need to ensure that this transaction is durable before
sync() returns, otherwise we lose that data if a failure occurs
immediately after sync() returns because the size update is not on
disk.

Users are right to complain when data written before a sync() call
is made does not accessible after a crash/reboot because we failed
to make it durable. That's why ->sync_fs(wait) is called at the end
of the sync() implementation - it enables filesystems to ensure all
data and metadata written during the sync processing is on durable
storage.

IOWs, we can't language-lawyer or weasel-word our way out of
providing durability guarantees for sync().

Cheers,

Dave.
-- 
Dave Chinner
david@xxxxxxxxxxxxx