From: Boaz Harrosh <openosd@gmail.com>
To: Dave Chinner <david@fromorbit.com>
Cc: Matthew Wilcox <matthew.r.wilcox@intel.com>,
linux-fsdevel@vger.kernel.org, linux-mm@kvack.org,
linux-kernel@vger.kernel.org,
Ross Zwisler <ross.zwisler@linux.intel.com>,
willy@linux.intel.com
Subject: Re: [PATCH v10 20/21] ext4: Add DAX functionality
Date: Mon, 15 Sep 2014 12:41:27 +0300 [thread overview]
Message-ID: <5416B447.8030606@gmail.com> (raw)
In-Reply-To: <20140915061534.GF4322@dastard>
On 09/15/2014 09:15 AM, Dave Chinner wrote:
> On Sun, Sep 14, 2014 at 03:25:45PM +0300, Boaz Harrosh wrote:
<>
>
> Well, that's one way of working around the immediate issue, but I
> don't think it solves the whole problem. e.g. what do you do with the
> bit of the partial write that failed? We may have allocated space
> for it but not written data to it, so to simply fail exposes stale
> data in the file(*).
>
I'm confused. From what you said and what I read of the dax_do_io
code the only possible error is ENOSPC from getblock.
(since ->direct_access() and memcopy cannot fail.)
Is it possible to fail with ENOSPC and still allocate an unwritten
block?
> Hence it's not clear to me that simply returning the short write is
> a valid solution for DAX-enabled filesystems. I think that the
> above - initially, at least - is much better than falling back to
> buffered IO but filesystems are going to have to be updated to work
> correctly without that fallback.
>
The way I read dax_do_io it will call getblock, write or zero it
and continue to the next one only after that.
If not we should establish an handshake that will at least zero out
any error blocks, and or d-allocates them. But can you see such code
path in dax_do_io?
>> Yes I agree this is a very bad data corruption bug. I also think
>> that the read path should not be allowed to fall back to buffered
>> IO just the same for the same reason. We must not allow any real
>> data in page_cache for a DAX file.
>
> Right, I didn't check the read path for the same issue as XFS won't
> return a short read on direct IO unless the read spans EOF. And in
> that case it won't ever do buffered reads. ;)
>
Right read is less problematic. I guess. But we should not attempt
a buffered read anyway.
> Cheers,
> Dave.
>
> (*) XFS avoids this problem by always using unwritten extents for
> direct IO allocation, but I'm pretty sure that ext4 doesn't do this.
> Using unwritten extents means that we don't expose stale data in the
> event we don't end up writing to the allocated space.
>
If only we had an xfstest for this ?
Thanks
Boaz
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2014-09-15 9:41 UTC|newest]
Thread overview: 52+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-08-27 3:45 [PATCH v10 00/21] Support ext4 on NV-DIMMs Matthew Wilcox
2014-08-27 3:45 ` [PATCH v10 01/21] axonram: Fix bug in direct_access Matthew Wilcox
2014-08-27 3:45 ` [PATCH v10 02/21] Change direct_access calling convention Matthew Wilcox
2014-08-27 3:45 ` [PATCH v10 03/21] Fix XIP fault vs truncate race Matthew Wilcox
2014-08-27 3:45 ` [PATCH v10 04/21] Allow page fault handlers to perform the COW Matthew Wilcox
2014-08-27 3:45 ` [PATCH v10 05/21] Introduce IS_DAX(inode) Matthew Wilcox
2014-08-27 3:45 ` [PATCH v10 06/21] Add copy_to_iter(), copy_from_iter() and iov_iter_zero() Matthew Wilcox
2014-08-27 3:45 ` [PATCH v10 07/21] Replace XIP read and write with DAX I/O Matthew Wilcox
2014-09-14 14:11 ` Boaz Harrosh
2014-08-27 3:45 ` [PATCH v10 08/21] Replace ext2_clear_xip_target with dax_clear_blocks Matthew Wilcox
2014-08-27 3:45 ` [PATCH v10 09/21] Replace the XIP page fault handler with the DAX page fault handler Matthew Wilcox
2014-09-03 7:47 ` Dave Chinner
2014-09-10 15:23 ` Matthew Wilcox
2014-09-11 3:09 ` Dave Chinner
2014-09-24 15:43 ` Matthew Wilcox
2014-09-25 1:01 ` Dave Chinner
2014-08-27 3:45 ` [PATCH v10 10/21] Replace xip_truncate_page with dax_truncate_page Matthew Wilcox
2014-08-27 3:45 ` [PATCH v10 11/21] Replace XIP documentation with DAX documentation Matthew Wilcox
2014-08-27 3:45 ` [PATCH v10 12/21] Remove get_xip_mem Matthew Wilcox
2014-08-27 3:45 ` [PATCH v10 13/21] ext2: Remove ext2_xip_verify_sb() Matthew Wilcox
2014-08-27 3:45 ` [PATCH v10 14/21] ext2: Remove ext2_use_xip Matthew Wilcox
2014-08-27 3:45 ` [PATCH v10 15/21] ext2: Remove xip.c and xip.h Matthew Wilcox
2014-08-27 3:45 ` [PATCH v10 16/21] Remove CONFIG_EXT2_FS_XIP and rename CONFIG_FS_XIP to CONFIG_FS_DAX Matthew Wilcox
2014-08-27 3:45 ` [PATCH v10 17/21] ext2: Remove ext2_aops_xip Matthew Wilcox
2014-08-27 3:45 ` [PATCH v10 18/21] Get rid of most mentions of XIP in ext2 Matthew Wilcox
2014-08-27 3:45 ` [PATCH v10 19/21] xip: Add xip_zero_page_range Matthew Wilcox
2014-09-03 9:21 ` Dave Chinner
2014-09-04 21:08 ` Matthew Wilcox
2014-09-04 21:36 ` Theodore Ts'o
2014-09-08 18:59 ` Matthew Wilcox
2014-08-27 3:45 ` [PATCH v10 20/21] ext4: Add DAX functionality Matthew Wilcox
2014-09-03 11:13 ` Dave Chinner
2014-09-10 16:49 ` Boaz Harrosh
2014-09-11 4:38 ` Dave Chinner
2014-09-14 12:25 ` Boaz Harrosh
2014-09-15 6:15 ` Dave Chinner
2014-09-15 9:41 ` Boaz Harrosh [this message]
2014-08-27 3:45 ` [PATCH v10 21/21] brd: Rename XIP to DAX Matthew Wilcox
2014-08-27 20:06 ` [PATCH v10 00/21] Support ext4 on NV-DIMMs Andrew Morton
2014-08-27 21:12 ` Matthew Wilcox
2014-08-27 21:46 ` Andrew Morton
2014-08-28 1:30 ` Andy Lutomirski
2014-08-28 16:50 ` Matthew Wilcox
2014-08-28 15:45 ` Matthew Wilcox
2014-08-27 21:22 ` Christoph Lameter
2014-08-27 21:30 ` Andrew Morton
2014-08-27 23:04 ` One Thousand Gnomes
2014-08-28 7:17 ` Dave Chinner
2014-08-30 23:11 ` Christian Stroetmann
2014-08-28 8:08 ` Boaz Harrosh
2014-08-28 22:09 ` Zwisler, Ross
2014-09-03 12:05 ` [PATCH 1/1] xfs: add DAX support Dave Chinner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5416B447.8030606@gmail.com \
--to=openosd@gmail.com \
--cc=david@fromorbit.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=matthew.r.wilcox@intel.com \
--cc=ross.zwisler@linux.intel.com \
--cc=willy@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).