[0/4] ext4: DAX fixes
diff mbox

Message ID 20160516155950.GF21714@quack2.suse.cz
State New
Headers show

Commit Message

Jan Kara May 16, 2016, 3:59 p.m. UTC
On Mon 16-05-16 08:13:50, Dan Williams wrote:
> On Mon, May 16, 2016 at 8:08 AM, Theodore Ts'o <tytso@mit.edu> wrote:
> > On Mon, May 16, 2016 at 04:26:16PM +0200, Jan Kara wrote:
> >> 1) Just push patches as is and have ext4 dax broken between ext4 merge and
> >> nvdimm merge.
> >>
> >> 2) Split out the one-line change from "dax: Remove dead zeroing code from
> >> fault handlers" in __dax_fault() which fixes the behavior for ext4 and
> >> merge it through ext4 tree. Merge the rest through nvdimm tree.
> >
> > I'm good either way, although I have a slight preference for (2).
> > It's really tiny preference, though, so if you or Dan want to run the
> > fix through the dax branch, that's fine too.
> 
> Would you fold the change and trigger a rebase or just apply it on
> top?  If just applying on top then it seems the same exposure as
> merging it intact through nvdimm.git.

The patch which fixes ext4 behavior is attached. Just that we know what we
are speaking about... Rebasing all the patches on top of this is trivial
(git rebase just handles the conflict automatically).

I've scheduled full ext4 & XFS xfstest run with just this patch and ext4
fixes to make sure it doesn't introduce some intermediate regresion
somewhere.

								Honza

Comments

Dan Williams May 16, 2016, 6:29 p.m. UTC | #1
On Mon, May 16, 2016 at 8:59 AM, Jan Kara <jack@suse.cz> wrote:
> On Mon 16-05-16 08:13:50, Dan Williams wrote:
>> On Mon, May 16, 2016 at 8:08 AM, Theodore Ts'o <tytso@mit.edu> wrote:
>> > On Mon, May 16, 2016 at 04:26:16PM +0200, Jan Kara wrote:
>> >> 1) Just push patches as is and have ext4 dax broken between ext4 merge and
>> >> nvdimm merge.
>> >>
>> >> 2) Split out the one-line change from "dax: Remove dead zeroing code from
>> >> fault handlers" in __dax_fault() which fixes the behavior for ext4 and
>> >> merge it through ext4 tree. Merge the rest through nvdimm tree.
>> >
>> > I'm good either way, although I have a slight preference for (2).
>> > It's really tiny preference, though, so if you or Dan want to run the
>> > fix through the dax branch, that's fine too.
>>
>> Would you fold the change and trigger a rebase or just apply it on
>> top?  If just applying on top then it seems the same exposure as
>> merging it intact through nvdimm.git.
>
> The patch which fixes ext4 behavior is attached. Just that we know what we
> are speaking about... Rebasing all the patches on top of this is trivial
> (git rebase just handles the conflict automatically).
>
> I've scheduled full ext4 & XFS xfstest run with just this patch and ext4
> fixes to make sure it doesn't introduce some intermediate regresion
> somewhere.
>

Ok, sounds good.  It makes the most sense to base the nvdimm.git
branch on top of this fix.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Theodore Y. Ts'o May 16, 2016, 8:47 p.m. UTC | #2
On Mon, May 16, 2016 at 05:59:50PM +0200, Jan Kara wrote:
> The patch which fixes ext4 behavior is attached. Just that we know what we
> are speaking about... Rebasing all the patches on top of this is trivial
> (git rebase just handles the conflict automatically).

OK, I've added this one-liner to the ext4 tree.  I presume that if git
rebase handles the conflict automatically, git merge should be able to
handle automatically it as well.

					- Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

From c8963499eeba112430fc4499c675cca243b8b4a6 Mon Sep 17 00:00:00 2001
From: Jan Kara <jack@suse.cz>
Date: Mon, 16 May 2016 17:42:11 +0200
Subject: [PATCH] dax: Call get_blocks() with create == 1 for write faults to
 unwritten extents

Currently, __dax_fault() does not call get_blocks() callback with create
argument set, when we got back unwritten extent from the initial
get_blocks() call during a write fault. This is because originally
filesystems were supposed to convert unwritten extents to written ones
using complete_unwritten() callback. Later this was abandoned in favor of
using pre-zeroed blocks however the condition whether get_blocks() needs
to be called with create == 1 remained.

Fix the condition so that filesystems are not forced to zero-out and
convert unwritten extents when get_blocks() is called with create == 0
(which introduces unnecessary overhead for read faults and can be
problematic as the filesystem may possibly be read-only).

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/dax.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/dax.c b/fs/dax.c
index 75ba46d82a76..2494255c5785 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -667,7 +667,7 @@  int __dax_fault(struct vm_area_struct *vma, struct vm_fault *vmf,
 	if (error)
 		goto unlock_page;
 
-	if (!buffer_mapped(&bh) && !buffer_unwritten(&bh) && !vmf->cow_page) {
+	if (!buffer_mapped(&bh) && !vmf->cow_page) {
 		if (vmf->flags & FAULT_FLAG_WRITE) {
 			error = get_block(inode, block, &bh, 1);
 			count_vm_event(PGMAJFAULT);
-- 
2.6.6